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

Conversation

frouioui
Copy link
Member

@frouioui frouioui commented Jan 18, 2024

Description

This PR adds logic that detects what the wanted MySQL server version is according to the image the end user has applied on the mysqld field of their yaml file.

The tag (for example: mysql:8.0.34) of the image will be parsed and will give us the mysql_server_version, this value will then be used to initiate/update all of our pods with the proper --mysql_server_version flag. If the tag cannot be parsed (if for instance we use mysql:latest) then the default mysql_server_version will be used internally in the operator (as it is today), and no --mysql_server_version flag will be set on the Vitess components, resulting in using the default value defined in the Vitess codebase.

Testing

We apply a yaml file that uses an official MySQL image, or even our own Vitess' mysql image like so:

    mysqld:
      mysql80Compatible: vitess/mysql:8.0.34

Or:

    mysqld:
      mysql80Compatible: mysql:8.0.34

When applying the yaml file, the vitess-operator will detect the mysql server version using the image's tag and will eventually add the --mysql_server_version flag with the proper value to all components that are using it. After applying the yaml file described above, here is what we get when doing kubectl describe pod:

The output have been truncated

  vttablet:
...
    Args:
...
      --mysql_server_version=8.0.34-Vitess
  mysqld:
...
    Args:
...
      --mysql_server_version=8.0.34-Vitess
  vtgate:
...
    Args:
...
      --mysql_server_version=8.0.34-Vitess
  vtctld:
...
    Args:
...
      --mysql_server_version=8.0.34-Vitess

If the label of the docker image was not readable or was simply missing, then no flag is applied and the flag disappears from the list of arguments for each binaries. For instance using mysql:latest will result in vitess-operator not knowing what the actual mysql server version is and falling back on using Vitess' default value.

@frouioui frouioui changed the title mysql_server_version Detect mysql_server_version from the mysqld Docker Image Jan 18, 2024
@frouioui frouioui marked this pull request as ready for review January 18, 2024 22:25
go.mod Outdated
@@ -21,11 +21,11 @@ require (
sigs.k8s.io/controller-runtime v0.16.3
sigs.k8s.io/controller-tools v0.11.3
sigs.k8s.io/kustomize v2.0.3+incompatible
vitess.io/vitess v0.10.3-0.20240112195828-c53420174d46
vitess.io/vitess v0.10.3-0.20240118194106-4c36f2aa6469
Copy link
Member Author

Choose a reason for hiding this comment

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

I upgraded the vitess dependency in order to benefit from the latest changes brought by vitessio/vitess#14988

Comment on lines +214 to +218
var mysqldImage string
if len(vts.Items) > 0 {
mysqldImage = vts.Items[0].Spec.Images.Mysqld.Image()
}

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

Comment on lines +56 to +58
func DockerImageSafeUpgrade(currentVersionImage, desiredVersionImage string) (bool, error) {
if currentVersionImage == "" || desiredVersionImage == "" {
// No action if we have unknown versions.
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

@@ -81,7 +81,7 @@ func (r *reconcileHandler) tsInit(ctx context.Context) error {
}
// Wrangler wraps the necessary clients and implements
// multi-step Vitess cluster management workflows.
wr := wrangler.New(logutil.NewConsoleLogger(), r.ts.Server, r.tmc, collationEnv, parser)
wr := wrangler.New(logutil.NewConsoleLogger(), r.ts.Server, r.tmc, collationEnv, parser, environment.MySQLServerVersion)
Copy link
Member Author

Choose a reason for hiding this comment

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

This change and other changes made to the few wrangler.New function calls we have is not directly related to this PR's enhancement. It was a result of the recent vitess golang dependency upgrade made in this PR.

Copy link
Collaborator

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

LGTM!

@frouioui frouioui merged commit e4addd1 into main Jan 22, 2024
9 checks passed
@frouioui frouioui deleted the default-mysql-server-version branch January 22, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants