Skip to content

🌱 [WIP] make topology upgrade sequential #6652

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

Closed
Show file tree
Hide file tree
Changes from all 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
16 changes: 16 additions & 0 deletions internal/controllers/topology/cluster/desired_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,11 @@ func computeControlPlaneVersion(s *scope.Scope) (string, error) {
if s.Current.MachineDeployments.IsAnyRollingOut() {
return *currentVersion, nil
}
// If any of the MachineDeployments are not at the desired version, then do not pick
// up the desiredVersion yet. We will pick up the new version after all MachineDeployments are at the desired version.
if !s.Current.MachineDeployments.AreAtVersion(*currentVersion) {
return *currentVersion, nil
}

// Control plane and machine deployments are stable.
// Ready to pick up the topology version.
Expand Down Expand Up @@ -549,6 +554,16 @@ func computeMachineDeployment(_ context.Context, s *scope.Scope, desiredControlP
// than the number of allowed concurrent upgrades.
func computeMachineDeploymentVersion(s *scope.Scope, desiredControlPlaneState *scope.ControlPlaneState, currentMDState *scope.MachineDeploymentState) (string, error) {
desiredVersion := s.Blueprint.Topology.Version
// If there already exists a control plane then the MachineDeployment should always target to match the current version
// of the control plane.
if s.Current.ControlPlane.Object != nil {
currentCPVersion, err := contract.ControlPlane().Version().Get(s.Current.ControlPlane.Object)
if err != nil {
return "", errors.Wrap(err, "failed to get version of current control plane")
}
desiredVersion = *currentCPVersion
}

// If creating a new machine deployment, we can pick up the desired version
// Note: We are not blocking the creation of new machine deployments when
// the control plane or any of the machine deployments are upgrading/scaling.
Expand Down Expand Up @@ -611,6 +626,7 @@ func computeMachineDeploymentVersion(s *scope.Scope, desiredControlPlaneState *s
// Check if we are about to upgrade the control plane. In that case, do not upgrade the machine deployment yet.
// Wait for the new upgrade operation on the control plane to finish before picking up the new version for the
// machine deployment.
// TODO: We probably don't need this check anymore.
Copy link
Member

Choose a reason for hiding this comment

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

I think you might be right. But because it's very easy to miss an edge case I would prefer keeping this logic as a fail safe so that we never under any circumstances trigger a MD rollout while the CP is still upgrading.

I think with the current code it's also easier to assess that vs. if we have to infer that based on the code above for all edge cases that we can think of.

currentCPVersion, err := contract.ControlPlane().Version().Get(s.Current.ControlPlane.Object)
if err != nil {
return "", errors.Wrap(err, "failed to get version of current control plane")
Expand Down
19 changes: 19 additions & 0 deletions internal/controllers/topology/cluster/scope/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ func (mds MachineDeploymentsStateMap) RollingOut() []string {
return names
}

// AreAtVersion return true if all the machine deployments are the desired version.
func (mds MachineDeploymentsStateMap) AreAtVersion(version string) bool {
for _, md := range mds {
if !md.IsAtVersion(version) {
return false
}
}
return true
}

// IsAnyRollingOut returns true if at least one of the
// machine deployments is rolling out. False, otherwise.
func (mds MachineDeploymentsStateMap) IsAnyRollingOut() bool {
Expand Down Expand Up @@ -96,3 +106,12 @@ type MachineDeploymentState struct {
func (md *MachineDeploymentState) IsRollingOut() bool {
return !mdutil.DeploymentComplete(md.Object, &md.Object.Status) || *md.Object.Spec.Replicas != md.Object.Status.ReadyReplicas
}

// IsAtVersion return true if the MachineDeployment spec version matches the version.
// If the MachineDeployment does not support version then it returns false.
func (md *MachineDeploymentState) IsAtVersion(version string) bool {
if md.Object.Spec.Template.Spec.Version != nil {
return version == *md.Object.Spec.Template.Spec.Version
}
return false
}