-
Notifications
You must be signed in to change notification settings - Fork 2
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
Use in-place upgrade release annotation if present #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work, left one comment. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work @berkayoz! Left some minor comments.
pkg/cloudinit/common.go
Outdated
type InstallOption string | ||
|
||
const ( | ||
InstallOptionChannel InstallOption = "track" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused about this part. I can see channel
instead of track
and localPath
instead of local-path
.
https://github.com/canonical/cluster-api-k8s/blob/main/pkg/ck8s/workload_cluster.go#L279-L288
If these are the same, maybe we can centralize them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the track
mentions to channel
. For localPath
vs local-path
, I've kept local-path
to fit the separate with -
file naming convention we had. I'll adjust the k8s-snap-api
to fit this form with another (cleanup) card I've this pulse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work and thanks a lot @berkayoz! LGTM
This PR adjusts the ck8s config reconciler to use the snap install option from an in-place upgrade release annotation if it exists. This can be utilized by adding the release annotation under the
CK8sControlPlane
orMachineDeployment
specs, which will be propagated down to the machine.For control-plane (CK8sControlPlane)
.spec.machineTemplate.metadata.annotations => Machine.annotations
Adopted something similar to propagation logic from KCP
For workers (MachineDeployment)
.spec.template.metadata.annotations => MachineSet.spec.template.metadata.annotations => Machine.annotations