Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect mysql_server_version from the mysqld Docker Image #521

Merged
merged 9 commits into from
Jan 22, 2024
6 changes: 3 additions & 3 deletions pkg/controller/vitesscell/reconcile_vtgate.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (m *secretCellsMapper) Map(ctx context.Context, obj client.Object) []reconc
return requests
}

func (r *ReconcileVitessCell) reconcileVtgate(ctx context.Context, vtc *planetscalev2.VitessCell) (reconcile.Result, error) {
func (r *ReconcileVitessCell) reconcileVtgate(ctx context.Context, vtc *planetscalev2.VitessCell, mysqldImage string) (reconcile.Result, error) {
clusterName := vtc.Labels[planetscalev2.ClusterLabel]

key := client.ObjectKey{Namespace: vtc.Namespace, Name: vtgate.ServiceName(clusterName, vtc.Spec.Name)}
Expand Down Expand Up @@ -151,11 +151,11 @@ func (r *ReconcileVitessCell) reconcileVtgate(ctx context.Context, vtc *planetsc
Kind: &appsv1.Deployment{},

New: func(key client.ObjectKey) runtime.Object {
return vtgate.NewDeployment(key, spec)
return vtgate.NewDeployment(key, spec, mysqldImage)
},
UpdateInPlace: func(key client.ObjectKey, obj runtime.Object) {
newObj := obj.(*appsv1.Deployment)
vtgate.UpdateDeployment(newObj, spec)
vtgate.UpdateDeployment(newObj, spec, mysqldImage)
},
Status: func(key client.ObjectKey, obj runtime.Object) {
curObj := obj.(*appsv1.Deployment)
Expand Down
23 changes: 22 additions & 1 deletion pkg/controller/vitesscell/vitesscell_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/sirupsen/logrus"
apilabels "k8s.io/apimachinery/pkg/labels"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -195,8 +196,28 @@ func (r *ReconcileVitessCell) Reconcile(cctx context.Context, request reconcile.
resultBuilder.Error(err)
}

// List all VitessShard in the same cluster, we do this to determine what is the image used for the mysqld container
// Note that this is cheap because it comes from the local cache.
labels := map[string]string{
planetscalev2.ClusterLabel: vtc.Labels[planetscalev2.ClusterLabel],
}
opts := &client.ListOptions{
Namespace: vtc.Namespace,
LabelSelector: apilabels.SelectorFromSet(labels),
}
vts := &planetscalev2.VitessShardList{}
if err := r.client.List(ctx, vts, opts); err != nil {
r.recorder.Eventf(vtc, corev1.EventTypeWarning, "ListFailed", "failed to list VitessShard objects: %v", err)
return resultBuilder.Error(err)
}

var mysqldImage string
if len(vts.Items) > 0 {
mysqldImage = vts.Items[0].Spec.Images.Mysqld.Image()
}

Comment on lines +214 to +218
Copy link
Member Author

@frouioui frouioui Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving mysqldImage empty and not erroring out if no shard was found is okay, it will just result in using the default mysql server version

// Create/update vtgate deployments.
vtgateResult, err := r.reconcileVtgate(ctx, vtc)
vtgateResult, err := r.reconcileVtgate(ctx, vtc, mysqldImage)
resultBuilder.Merge(vtgateResult, err)

// Check which VitessKeyspaces are deployed to this cell.
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/vitesscluster/reconcile_vtctld.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,19 @@ func (r *ReconcileVitessCluster) reconcileVtctld(ctx context.Context, vt *planet
Kind: &appsv1.Deployment{},

New: func(key client.ObjectKey) runtime.Object {
return vtctld.NewDeployment(key, specMap[key])
return vtctld.NewDeployment(key, specMap[key], vt.Spec.Images.Mysqld.Image())
},
UpdateInPlace: func(key client.ObjectKey, obj runtime.Object) {
newObj := obj.(*appsv1.Deployment)
if *vt.Spec.UpdateStrategy.Type == planetscalev2.ImmediateVitessClusterUpdateStrategyType {
vtctld.UpdateDeployment(newObj, specMap[key])
vtctld.UpdateDeployment(newObj, specMap[key], vt.Spec.Images.Mysqld.Image())
return
}
vtctld.UpdateDeploymentImmediate(newObj, specMap[key])
},
UpdateRollingInPlace: func(key client.ObjectKey, obj runtime.Object) {
newObj := obj.(*appsv1.Deployment)
vtctld.UpdateDeployment(newObj, specMap[key])
vtctld.UpdateDeployment(newObj, specMap[key], vt.Spec.Images.Mysqld.Image())
},
Status: func(key client.ObjectKey, obj runtime.Object) {
// This function will get called once for each Deployment.
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/vitessshard/reconcile_backup_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (r *ReconcileVitessShard) reconcileBackupJob(ctx context.Context, vts *plan
Kind: &corev1.Pod{},

New: func(key client.ObjectKey) runtime.Object {
return vttablet.NewBackupPod(key, specMap[key])
return vttablet.NewBackupPod(key, specMap[key], vts.Spec.Images.Mysqld.Image())
},
Status: func(key client.ObjectKey, obj runtime.Object) {
pod := obj.(*corev1.Pod)
Expand Down
99 changes: 2 additions & 97 deletions pkg/controller/vitessshardreplication/reconcile_drain.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ package vitessshardreplication
import (
"context"
"fmt"
"regexp"
"slices"
"strconv"
"strings"
"time"

"github.com/sirupsen/logrus"
"planetscale.dev/vitess-operator/pkg/operator/mysql"
"vitess.io/vitess/go/mysql/replication"
"vitess.io/vitess/go/vt/proto/tabletmanagerdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
Expand Down Expand Up @@ -508,7 +505,7 @@ func (r *ReconcileVitessShard) disableFastShutdown(
}
}

needsSafe, err := safeMysqldUpgrade(current, desiredImage)
needsSafe, err := mysql.DockerImageSafeUpgrade(current, desiredImage)
if err != nil {
return err
}
Expand All @@ -527,98 +524,6 @@ func (r *ReconcileVitessShard) disableFastShutdown(
return nil
}

var mysqlImageVersion = regexp.MustCompile(`^(\d+)\.(\d+)\.(\d+)`)

func safeMysqldUpgrade(currentImage, desiredImage string) (bool, error) {
if currentImage == "" || desiredImage == "" {
// No action if we have unknown versions.
return false, nil
}

// Quick check so no regexp matching is needed for the most common
// case where nothing changes.
if desiredImage == currentImage {
return false, nil
}

currentParts := strings.SplitN(currentImage, ":", 2)
if len(currentParts) != 2 {
return false, nil
}

desiredParts := strings.SplitN(desiredImage, ":", 2)
if len(desiredParts) != 2 {
return false, nil
}

current := currentParts[1]
desired := desiredParts[1]

curStrParts := mysqlImageVersion.FindStringSubmatch(current)
if len(curStrParts) != 4 {
// Invalid version, assume that we need to do a safe upgrade.
return true, nil
}
dstStrParts := mysqlImageVersion.FindStringSubmatch(desired)
if len(dstStrParts) != 4 {
// Invalid version, assume that we need to do a safe upgrade.
return true, nil
}
if slices.Equal(curStrParts, dstStrParts) {
return false, nil
}
dstParts := make([]int, len(dstStrParts)-1)
curParts := make([]int, len(curStrParts)-1)
for i, part := range dstStrParts[1:] {
// We already matched with `\d_` so there's no
// way this can trigger an error.
dstParts[i], _ = strconv.Atoi(part)
}

for i, part := range curStrParts[1:] {
// We already matched with `\d_` so there's no
// way this can trigger an error.
curParts[i], _ = strconv.Atoi(part)
}

if dstParts[0] < curParts[0] {
return false, fmt.Errorf("cannot downgrade major version from %s to %s", current, desired)
}
if dstParts[0] == curParts[1] && dstParts[1] < curParts[1] {
return false, fmt.Errorf("cannot downgrade minor version from %s to %s", current, desired)
}

// Alright, here it gets more tricky. MySQL has had a complicated release history. For the 8.0 series,
// up to 8.0.34 at least (known at this point), it was not supported to downgrade patch releases
// as patch release could also include on-disk data format changes. This happened a number of times
// in practice as well, so this concern is real.
//
// MySQL though has announced a new release strategy, see:
// https://dev.mysql.com/blog-archive/introducing-mysql-innovation-and-long-term-support-lts-versions/
//
// With that release strategy, it will become possible that patch releases will be safe to downgrade
// as well and since the data format doesn't change on-disk anymore, it's also safe to upgrade with
// fast shutdown enabled.
// Specifically, it calls out that "MySQL 8.0.34+ will become bugfix only release (red)". This means
// that we use that version as a cut-off point here for when we need to disable fast shutdown or not.
if dstParts[0] == 8 && dstParts[1] == 0 && curParts[0] == 8 && curParts[1] == 0 {
// Our upgrade process stays within the 8.0.x version range.
if dstParts[2] >= 34 && curParts[2] >= 34 {
// No need for safe upgrade if both versions are 8.0.34 or higher.
return false, nil
}
// We can't downgrade within the 8.0.x series before 8.0.34.
if dstParts[2] < curParts[2] {
return false, fmt.Errorf("cannot downgrade patch version from %s to %s", current, desired)
}
// Always need safe upgrade if we change the patch release for 8.0.x before 8.0.34.
return dstParts[2] != curParts[2], nil
}

// For any major or minor version change we always need safe upgrade.
return dstParts[0] != curParts[0] || dstParts[1] != curParts[1], nil
}

type candidateInfo struct {
tablet *topo.TabletInfo
position replication.Position
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (

var (
reconcileTimeout time.Duration
mySQLServerVersion = "8.0.30-Vitess"
MySQLServerVersion = "8.0.30-Vitess"
// truncateUILen truncate queries in debug UIs to the given length. 0 means unlimited.
truncateUILen = 512
// truncateErrLen truncate queries in error logs to the given length. 0 means unlimited.
Expand Down Expand Up @@ -68,7 +68,7 @@ func ReconcileTimeout() time.Duration {
// VtEnvironment gets the vitess environment to be used in the operator.
func VtEnvironment() (*vtenv.Environment, error) {
return vtenv.New(vtenv.Options{
MySQLServerVersion: mySQLServerVersion,
MySQLServerVersion: MySQLServerVersion,
TruncateUILen: truncateUILen,
TruncateErrLen: truncateErrLen,
})
Expand Down
144 changes: 144 additions & 0 deletions pkg/operator/mysql/mysql.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
Copyright 2024 PlanetScale Inc.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package mysql

import (
"fmt"
"regexp"
"slices"
"strconv"
"strings"

"planetscale.dev/vitess-operator/pkg/operator/environment"
"planetscale.dev/vitess-operator/pkg/operator/vitess"
"vitess.io/vitess/go/vt/sqlparser"
)

var imageVersionRegExp = regexp.MustCompile(`^(\d+)\.(\d+)\.(\d+)`)

func UpdateMySQLServerVersion(flags vitess.Flags, mysqldImage string) {
value, err := dockerImageGetVersionToString(mysqldImage)
if err != nil {
return
}
flags["mysql_server_version"] = value
environment.MySQLServerVersion = value
}

func dockerImageGetVersionToString(currentVersionImage string) (string, error) {
currentVersionSlice := strings.SplitN(currentVersionImage, ":", 2)
if len(currentVersionSlice) != 2 {
return "", fmt.Errorf("could not parse the image name and image label, got: %s, but expected xx:xx", currentVersionImage)
}

label := currentVersionSlice[1]
_, err := sqlparser.ConvertMySQLVersionToCommentVersion(label)
if err != nil {
return "", err
}
return fmt.Sprintf("%s-Vitess", label), nil
}

func DockerImageSafeUpgrade(currentVersionImage, desiredVersionImage string) (bool, error) {
if currentVersionImage == "" || desiredVersionImage == "" {
// No action if we have unknown versions.
Comment on lines +56 to +58
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to move this from pkg/controller/vitessshardreplication/reconcile_drain.go to this new package to ease readability, no change were applied to DockerImageSafeUpgrade besides renaming the function from safeMysqldUpgrade to this

return false, nil
}

// Quick check so no regexp matching is needed for the most common
// case where nothing changes.
if desiredVersionImage == currentVersionImage {
return false, nil
}

currentParts := strings.SplitN(currentVersionImage, ":", 2)
if len(currentParts) != 2 {
return false, nil
}

desiredParts := strings.SplitN(desiredVersionImage, ":", 2)
if len(desiredParts) != 2 {
return false, nil
}

current := currentParts[1]
desired := desiredParts[1]

curStrParts := imageVersionRegExp.FindStringSubmatch(current)
if len(curStrParts) != 4 {
// Invalid version, assume that we need to do a safe upgrade.
return true, nil
}
dstStrParts := imageVersionRegExp.FindStringSubmatch(desired)
if len(dstStrParts) != 4 {
// Invalid version, assume that we need to do a safe upgrade.
return true, nil
}
if slices.Equal(curStrParts, dstStrParts) {
return false, nil
}
dstParts := make([]int, len(dstStrParts)-1)
curParts := make([]int, len(curStrParts)-1)
for i, part := range dstStrParts[1:] {
// We already matched with `\d_` so there's no
// way this can trigger an error.
dstParts[i], _ = strconv.Atoi(part)
}

for i, part := range curStrParts[1:] {
// We already matched with `\d_` so there's no
// way this can trigger an error.
curParts[i], _ = strconv.Atoi(part)
}

if dstParts[0] < curParts[0] {
return false, fmt.Errorf("cannot downgrade major version from %s to %s", current, desired)
}
if dstParts[0] == curParts[1] && dstParts[1] < curParts[1] {
return false, fmt.Errorf("cannot downgrade minor version from %s to %s", current, desired)
}

// Alright, here it gets more tricky. MySQL has had a complicated release history. For the 8.0 series,
// up to 8.0.34 at least (known at this point), it was not supported to downgrade patch releases
// as patch release could also include on-disk data format changes. This happened a number of times
// in practice as well, so this concern is real.
//
// MySQL though has announced a new release strategy, see:
// https://dev.mysql.com/blog-archive/introducing-mysql-innovation-and-long-term-support-lts-versions/
//
// With that release strategy, it will become possible that patch releases will be safe to downgrade
// as well and since the data format doesn't change on-disk anymore, it's also safe to upgrade with
// fast shutdown enabled.
// Specifically, it calls out that "MySQL 8.0.34+ will become bugfix only release (red)". This means
// that we use that version as a cut-off point here for when we need to disable fast shutdown or not.
if dstParts[0] == 8 && dstParts[1] == 0 && curParts[0] == 8 && curParts[1] == 0 {
// Our upgrade process stays within the 8.0.x version range.
if dstParts[2] >= 34 && curParts[2] >= 34 {
// No need for safe upgrade if both versions are 8.0.34 or higher.
return false, nil
}
// We can't downgrade within the 8.0.x series before 8.0.34.
if dstParts[2] < curParts[2] {
return false, fmt.Errorf("cannot downgrade patch version from %s to %s", current, desired)
}
// Always need safe upgrade if we change the patch release for 8.0.x before 8.0.34.
return dstParts[2] != curParts[2], nil
}

// For any major or minor version change we always need safe upgrade.
return dstParts[0] != curParts[0] || dstParts[1] != curParts[1], nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package vitessshardreplication
package mysql

import (
"testing"
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestSafeMysqldUpgrade(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
needsSafe, err := safeMysqldUpgrade(tt.current, tt.desired)
needsSafe, err := DockerImageSafeUpgrade(tt.current, tt.desired)
if tt.err != "" {
assert.EqualError(t, err, tt.err)
} else {
Expand Down
Loading
Loading