-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: Changes to controlplane spec should trigger a rollout #114
feat: Changes to controlplane spec should trigger a rollout #114
Conversation
44be031
to
4a4e8d2
Compare
…d, mirroring the approach used by kubeadm Signed-off-by: Richard Draycott <[email protected]>
…ntrolplanespec Signed-off-by: Richard Draycott <[email protected]>
4a4e8d2
to
9b1f81b
Compare
Signed-off-by: Richard Draycott <[email protected]>
bf39bea
to
81d4839
Compare
// If the annotation is not present (machine is either old or adopted), we won't roll out on any possible changes | ||
// made in KCP's KThreesServerConfig given that we don't have enough information to make a decision. | ||
// Users should use KCP.Spec.RolloutAfter field to force a rollout in this case. | ||
func matchKThreesServerConfig(kcp *controlplanev1.KThreesControlPlane, machine *clusterv1.Machine) bool { |
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.
at least for controlplane, init and join cp node has same bootstrap config, this is different from how kubeadm works
cluster-api-k3s/pkg/k3s/control_plane.go
Lines 191 to 201 in 2a14b6e
// InitialControlPlaneConfig returns a new KThreesConfigSpec that is to be used for an initializing control plane. | |
func (c *ControlPlane) InitialControlPlaneConfig() *bootstrapv1.KThreesConfigSpec { | |
bootstrapSpec := c.KCP.Spec.KThreesConfigSpec.DeepCopy() | |
return bootstrapSpec | |
} | |
// JoinControlPlaneConfig returns a new KThreesConfigSpec that is to be used for joining control planes. | |
func (c *ControlPlane) JoinControlPlaneConfig() *bootstrapv1.KThreesConfigSpec { | |
bootstrapSpec := c.KCP.Spec.KThreesConfigSpec.DeepCopy() | |
return bootstrapSpec | |
} |
I'm not very sure about worker node, you may provision a k3s cluster and check the bootstrap config.
Let's try if we can simplify this code
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.
Ah, I see what you mean. The init and join config is just the kcp KThreesConfigSpec, we do still compare this here, how do you suggest we should simplify the code?
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.
we can simply compare kcp config and machine bootstrap config directly, no need to compare serverConfig separately
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.
Addressed in the latest commit
Signed-off-by: Richard Draycott <[email protected]>
…tion Signed-off-by: Richard Draycott <[email protected]>
998fb1d
to
9caad96
Compare
This PR addresses #83, changes to the controlplane spec should trigger a rollout of controlplane machines.
I've tried to mirror the approach used in kubeadm as much as possible while adapting for k3s