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

Add support for user-defined per-keyspace images #524

Merged
merged 3 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions deploy/crds/planetscale.com_vitessclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1172,6 +1172,28 @@ spec:
type: string
durabilityPolicy:
type: string
images:
properties:
mysqld:
properties:
mariadb103Compatible:
type: string
mariadbCompatible:
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that we can remove these from all of the CRDs on main now, but for now I think we can NOT propagate them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done

mysql56Compatible:
type: string
mysql80Compatible:
type: string
type: object
mysqldExporter:
type: string
vtbackup:
type: string
vtorc:
type: string
vttablet:
type: string
type: object
name:
maxLength: 63
minLength: 1
Expand Down
37 changes: 29 additions & 8 deletions docs/api/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4770,10 +4770,11 @@ <h3 id="planetscale.com/v2.VitessKeyspace">VitessKeyspace
</em>
</td>
<td>
<p>Images are not customizable by users at the keyspace level because version
skew across the cluster is discouraged except during rolling updates,
in which case this field is automatically managed by the VitessCluster
controller that owns this VitessKeyspace.</p>
<p>Images are inherited from the VitessCluster spec, unless the user has
specified keyspace-level overrides. Version skew across the cluster is
discouraged except during rolling updates, in which case this field is
automatically managed by the VitessCluster controller that owns this
VitessKeyspace.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -5078,6 +5079,7 @@ <h3 id="planetscale.com/v2.VitessKeyspaceImages">VitessKeyspaceImages
<p>
(<em>Appears on:</em>
<a href="#planetscale.com/v2.VitessKeyspaceSpec">VitessKeyspaceSpec</a>,
<a href="#planetscale.com/v2.VitessKeyspaceTemplate">VitessKeyspaceTemplate</a>,
<a href="#planetscale.com/v2.VitessShardSpec">VitessShardSpec</a>)
</p>
<p>
Expand Down Expand Up @@ -5557,10 +5559,11 @@ <h3 id="planetscale.com/v2.VitessKeyspaceSpec">VitessKeyspaceSpec
</em>
</td>
<td>
<p>Images are not customizable by users at the keyspace level because version
skew across the cluster is discouraged except during rolling updates,
in which case this field is automatically managed by the VitessCluster
controller that owns this VitessKeyspace.</p>
<p>Images are inherited from the VitessCluster spec, unless the user has
specified keyspace-level overrides. Version skew across the cluster is
discouraged except during rolling updates, in which case this field is
automatically managed by the VitessCluster controller that owns this
VitessKeyspace.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the version here was more about Vitess than MySQL. I think that we're now focusing on MySQL version rather than Vitess version, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Yes indeed, just focusing on MySQL.

Hm..with that observation in mind, do you think it would make sense to scope this PR to only allow MySQL image overrides?

Copy link
Contributor Author

@maxenglander maxenglander Jan 24, 2024

Choose a reason for hiding this comment

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

I went ahead an removed support for user-definable per-keyspace images for vttablet, vtorc, myqsld exporter along w/ removing support for setting mariadb images.

</td>
</tr>
<tr>
Expand Down Expand Up @@ -5936,6 +5939,24 @@ <h3 id="planetscale.com/v2.VitessKeyspaceTemplate">VitessKeyspaceTemplate
<p>Annotations can optionally be used to attach custom annotations to the VitessKeyspace object.</p>
</td>
</tr>
<tr>
<td>
<code>images</code></br>
<em>
<a href="#planetscale.com/v2.VitessKeyspaceImages">
VitessKeyspaceImages
</a>
</em>
</td>
<td>
<p>Users are encouraged to let the VitessCluster controller automatically
propagate image changes from the VitessCluster to the VitessKeyspace
via rolling updates.</p>
<p>For special cases, users may specify per-VitessKeyspace images. An
example: migrating from MySQL 5.7 to MySQL 8.0 via a <code>MoveTables</code>
operation, after which the source keyspace is destroyed.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="planetscale.com/v2.VitessKeyspaceTurndownPolicy">VitessKeyspaceTurndownPolicy
Expand Down
20 changes: 20 additions & 0 deletions pkg/apis/planetscale/v2/vitesskeyspace_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,23 @@ func DefaultVitessKeyspaceImages(dst *VitessKeyspaceImages, clusterDefaults *Vit
dst.MysqldExporter = clusterDefaults.MysqldExporter
}
}

// MergeVitessKeyspaceImages takes non-empty image values from a non-nil src
// and sets them on dst.
func MergeVitessKeyspaceImages(dst *VitessKeyspaceImages, src *VitessKeyspaceImages) {
if src.Vttablet != "" {
dst.Vttablet = src.Vttablet
}
if src.Vtorc != "" {
dst.Vtorc = src.Vtorc
}
if src.Vtbackup != "" {
dst.Vtbackup = src.Vtbackup
}
if src.Mysqld != nil {
dst.Mysqld = src.Mysqld
}
if src.MysqldExporter != "" {
dst.MysqldExporter = src.MysqldExporter
}
}
18 changes: 14 additions & 4 deletions pkg/apis/planetscale/v2/vitesskeyspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ type VitessKeyspaceSpec struct {
// GlobalLockserver are the params to connect to the global lockserver.
GlobalLockserver VitessLockserverParams `json:"globalLockserver"`

// Images are not customizable by users at the keyspace level because version
// skew across the cluster is discouraged except during rolling updates,
// in which case this field is automatically managed by the VitessCluster
// controller that owns this VitessKeyspace.
// Images are inherited from the VitessCluster spec, unless the user has
// specified keyspace-level overrides. Version skew across the cluster is
// discouraged except during rolling updates, in which case this field is
// automatically managed by the VitessCluster controller that owns this
// VitessKeyspace.
Images VitessKeyspaceImages `json:"images,omitempty"`

// ImagePullPolicies are inherited from the VitessCluster spec.
Expand Down Expand Up @@ -178,6 +179,15 @@ type VitessKeyspaceTemplate struct {

// Annotations can optionally be used to attach custom annotations to the VitessKeyspace object.
Annotations map[string]string `json:"annotations,omitempty"`

// Users are encouraged to let the VitessCluster controller automatically
// propagate image changes from the VitessCluster to the VitessKeyspace
// via rolling updates.
//
// For special cases, users may specify per-VitessKeyspace images. An
// example: migrating from MySQL 5.7 to MySQL 8.0 via a `MoveTables`
// operation, after which the source keyspace is destroyed.
Images VitessKeyspaceImages `json:"images,omitempty"`
}

// VitessOrchestratorSpec specifies deployment parameters for vtorc.
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/planetscale/v2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/controller/vitesscluster/reconcile_keyspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ func newVitessKeyspace(key client.ObjectKey, vt *planetscalev2.VitessCluster, pa
images := planetscalev2.VitessKeyspaceImages{}
planetscalev2.DefaultVitessKeyspaceImages(&images, &vt.Spec.Images)

// Apply user-defined overrides for images.
planetscalev2.MergeVitessKeyspaceImages(&images, &keyspace.Images)

// Copy parent labels map and add keyspace-specific label.
labels := make(map[string]string, len(parentLabels)+1)
for k, v := range parentLabels {
Expand Down
Loading