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.

Sign up for GitHub

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

Jump to bottom

Support customisation of docker images per-keyspace #610

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Support customisation of docker images per-keyspace #610

merged 4 commits into from
Oct 11, 2024

Conversation

frouioui
Copy link
Member

@frouioui frouioui commented Oct 8, 2024 •
edited
Loading

Description

This PR builds on top of #524 and adds full support for cusomisation of all Docker Images per keyspace. We can now define what Docker Image we want to use at the keyspace level. the containers for which we can customise the image we want at the keyspace level are: vttablet, vtbackup, mysql, mysqldExporter, vtorc.

This per-keyspace definition takes precedence over the top-level definition in spec.images.

This PR is part of #608. It fixes the Issue 1 part.

Tests

apiVersion: planetscale.com/v2
kind: VitessCluster
metadata:
  name: example
spec:
  ...
  images:
    vtctld: vitess/lite:latest
    vtgate: vitess/lite:latest
    vttablet: vitess/lite:latest
    vtorc: vitess/lite:latest
    vtbackup: vitess/lite:latest
    mysqld:
      mysql80Compatible: mysql:8.0.30
    mysqldExporter: prom/mysqld-exporter:v0.11.0
...
...
  keyspaces:
  - name: commerce
    images:
      vtbackup: vitess/lite:v20.0.2
      vtorc: vitess/lite:v20.0.2
      vttablet: vitess/lite:v20.0.2
      mysqld:
        mysql80Compatible: mysql:8.0.31
      mysqldExporter: prom/mysqld-exporter:v0.11.0

$ kubectl describe pod example-vttablet-zone1-2548885007-46a852d0 | grep "Image:"

    Image:         vitess/lite:v20.0.2
    Image:         vitess/lite:v20.0.2
    Image:         vitess/lite:v20.0.2
    Image:         mysql:8.0.31
    Image:         prom/mysqld-exporter:v0.11.0

$ kubectl describe pod example-commerce-x-x-zone1-vtorc-c13ef6ff-8647487fd9-kjmm9 | grep "Image:" 

   Image:         vitess/lite:v20.0.2

Backports

As #524 was made on v2.12, let's backport this all the way to v2.12 plus v2.11too as explained in #609.

@frouioui frouioui mentioned this pull request Oct 8, 2024
Closed
frouioui
frouioui commented Oct 8, 2024
View reviewed changes
pkg/apis/planetscale/v2/vitesskeyspace_types.go Outdated
Comment on lines 190 to 212
// VitessKeyspaceTemplateImages specifies user-definable container images to
// use for this keyspace.
// use for this keyspace. The images defined here by the user will override
// those defined at the top-level in VitessCluster.spec.images
//
// Note: this structure is a copy of VitessKeyspaceImages, once we have gotten
// rid of MysqldImage and replaced it by MysqldImageNew (planned for v2.15), we
// should be able to remove VitessKeyspaceTemplateImages entirely and just use
// VitessKeyspaceImages instead as it contains exactly the same fields.
type VitessKeyspaceTemplateImages struct {
// Vttablet is the container image (including version tag) to use for Vitess Tablet instances.
Vttablet string `json:"vttablet,omitempty"`
// Vtorc is the container image (including version tag) to use for Vitess Orchestrator instances.
Vtorc string `json:"vtorc,omitempty"`
// Vtbackup is the container image (including version tag) to use for Vitess Backup jobs.
Vtbackup string `json:"vtbackup,omitempty"`
// Mysqld specifies the container image to use for mysqld, as well as
// declaring which MySQL flavor setting in Vitess the image is
// compatible with. Only one flavor image may be provided at a time.
// mysqld running alongside each tablet.
Mysqld *MysqldImageNew `json:"mysqld,omitempty"`
// MysqldExporter specifies the container image for mysqld-exporter.
MysqldExporter string `json:"mysqldExporter,omitempty"`
}
Copy link
Member Author

@frouioui frouioui Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This situation is not ideal to me, but will be cleaned up in a subsequent PR in v2.15 soon.

@mattlord mattlord added the enhancement New feature or request label Oct 11, 2024
mattlord
mattlord approved these changes Oct 11, 2024
View reviewed changes
Copy link
Collaborator

@mattlord mattlord left a comment

Choose a reason for hiding this comment

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

Nice work on this! Just had the one suggestion about the text (which would have to be modified in the source rather than the generated HTML, sorry).

docs/api/index.html Outdated
Comment on lines 6556 to 6559
<p>While this field allows you to set a different set of Vitess version for
some components of Vitess than the version defined at the top level, it
is important to note that Vitess only ensure compatibility between one
version and the next and previous one. For instance: N is only compatible
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p>While this field allows you to set a different set of Vitess version for
some components of Vitess than the version defined at the top level, it
is important to note that Vitess only ensure compatibility between one
version and the next and previous one. For instance: N is only compatible
<p>While this field allows you to set a different Vitess version for some
components than the version defined at the top level, it is important to
note that Vitess only ensures compatibility between one version and the
next and previous one. For instance: N is only guaranteed to be compatible

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed via 91eb524

docs/api/index.html
with N+1 and N-1. Do be careful when specifying multiple versions across
your cluster so that they respect this compatibility rule.</p>
<p>Note: this structure is a copy of VitessKeyspaceImages, once we have gotten
rid of MysqldImage and replaced it by MysqldImageNew (planned for v2.15), we
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought that that new was only temporary? This makes it seem like it's permanent. Maybe I'm misunderstanding.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is only temporary, we will fix it within this release cycle on main

@frouioui
Review suggestion - doc
426dd7b 
Signed-off-by: Florent Poinsard <[email protected]>
@frouioui frouioui force-pushed the custom-images-per-keyspace branch from 91eb524 to 426dd7b Compare October 11, 2024 22:26
@frouioui
trigger ci
cf88796 
Signed-off-by: Florent Poinsard <[email protected]>
@frouioui frouioui merged commit 658de0a into main Oct 11, 2024
11 checks passed
@frouioui frouioui deleted the custom-images-per-keyspace branch October 11, 2024 23:55
This was referenced Oct 12, 2024
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattlord mattlord mattlord approved these changes

@maxenglander maxenglander maxenglander approved these changes

Assignees
No one assigned
Labels
Backport to: release-2.11 Backport to: release-2.12 Backport to: release-2.13 Backport to: release-2.14 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frouioui @maxenglander @mattlord