✨ Add in-place to rollout planner (#12865)

* Add in-place to rollout planner

* Address comments

* More comments

* More comments

Author: fabriziopandini

api/core/v1beta2/machine_types.go

@@ -87,6 +87,17 @@ const (
 
 	// ManagedNodeAnnotationDomain is one of the CAPI managed Node annotation domains.
 	ManagedNodeAnnotationDomain = "node.cluster.x-k8s.io"
+
+	// PendingAcknowledgeMoveAnnotation is an internal annotation added by the MS controller to a machine when being
+	// moved from the oldMS to the newMS. The annotation is removed as soon as the MS controller get the acknowledgment about the
+	// replica being accounted from the corresponding MD.
+	// Note: The annotation is added when reconciling the oldMS, and it is removed when reconciling the newMS.
+	// Note: This annotation is used in pair with AcknowledgedMoveAnnotation on MachineSets.
+	PendingAcknowledgeMoveAnnotation = "in-place-updates.internal.cluster.x-k8s.io/pending-acknowledge-move"
+
+	// UpdateInProgressAnnotation is an internal annotation added to machines by the controller owning the Machine when in-place update
+	// is started, e.g. by the MachineSet controller; the annotation will be removed by the Machine controller when in-place update is completed.
+	UpdateInProgressAnnotation = "in-place-updates.internal.cluster.x-k8s.io/update-in-progress"
 )
 
 // Machine's Available condition and corresponding reasons.

api/core/v1beta2/machineset_types.go

@@ -33,6 +33,29 @@ const (
 	// MachineSetFinalizer is the finalizer used by the MachineSet controller to
 	// ensure ordered cleanup of corresponding Machines when a Machineset is being deleted.
 	MachineSetFinalizer = "cluster.x-k8s.io/machineset"
+
+	// MachineSetMoveMachinesToMachineSetAnnotation is an internal annotation added by the MD controller to the oldMS
+	// when it should scale down by moving machines that can be updated in-place to the newMS instead of deleting them.
+	// The annotation value is the newMS name.
+	// Note: This annotation is used in pair with MachineSetReceiveMachinesFromMachineSetsAnnotation to perform a two-ways check before moving a machine from oldMS to newMS:
+	//
+	//	"oldMS must have: move to newMS" and "newMS must have: receive replicas from oldMS"
+	MachineSetMoveMachinesToMachineSetAnnotation = "in-place-updates.internal.cluster.x-k8s.io/move-machines-to-machineset"
+
+	// MachineSetReceiveMachinesFromMachineSetsAnnotation is an internal annotation added by the MD controller to the newMS
+	// when it should receive replicas from oldMSs as a first step of an in-place upgrade operation
+	// The annotation value is a comma separated list of oldMSs.
+	// Note: This annotation is used in pair with MachineSetMoveMachinesToMachineSetAnnotation to perform a two-ways check before moving a machine from oldMS to newMS:
+	//
+	//	"oldMS must have: move to newMS" and "newMS must have: receive replicas from oldMS"
+	MachineSetReceiveMachinesFromMachineSetsAnnotation = "in-place-updates.internal.cluster.x-k8s.io/receive-machines-from-machinesets"
+
+	// AcknowledgedMoveAnnotation is an internal annotation with a list of machines added by the MD controller
+	// to a MachineSet when it acknowledges a machine pending acknowledge after being moved from an oldMS.
+	// The annotation value is a comma separated list of Machines already acknowledged; a machine is dropped
+	// from this annotation as soon as pending-acknowledge-move is removed from the machine; the annotation is dropped when empty.
+	// Note: This annotation is used in pair with PendingAcknowledgeMoveAnnotation on Machines.
+	AcknowledgedMoveAnnotation = "in-place-updates.internal.cluster.x-k8s.io/acknowledged-move"
 )
 
 // MachineSetSpec defines the desired state of MachineSet.

docs/book/src/reference/api/labels-and-annotations.md

@@ -58,6 +58,29 @@
 | topology.cluster.x-k8s.io/dry-run                                | It is an annotation that gets set on objects by the topology controller only during a server side dry run apply operation. It is used for validating update webhooks for objects which get updated by template rotation (e.g. InfrastructureMachineTemplate). When the annotation is set and the admission request is a dry run, the webhook should deny validation due to immutability. By that the request will succeed (without any changes to the actual object because it is a dry run) and the topology controller will receive the resulting object. | Cluster API              | Template rotation objects                      |
 | topology.cluster.x-k8s.io/hold-upgrade-sequence                  | It can be used to hold the entire MachineDeployment upgrade sequence. If the annotation is set on a MachineDeployment topology in Cluster.spec.topology.workers, the Kubernetes upgrade for this MachineDeployment topology and all subsequent ones is deferred.                                                                                                                                                                                                                                                                                            | Cluster API              | MachineDeployments in Cluster.topology         |
 | topology.cluster.x-k8s.io/upgrade-concurrency                    | It can be used to configure the maximum concurrency while upgrading MachineDeployments of a classy Cluster. It is set as a top level annotation on the Cluster object. The value should be >= 1. If unspecified the upgrade concurrency will default to 1.                                                                                                                                                                                                                                                                                                  | Cluster API              | Clusters                                       |
-| topology.internal.cluster.x-k8s.io/upgrade-step                  | This is an annotation used by CAPI internally to track upgrade steps. Name, meaning and semantic of the annotation can change anytime and it should not be used outside of CAPI controllers.                                                                                                                                                                                                                                                                                                                                                                | Cluster API              | Clusters                                       |
 | unsafe.topology.cluster.x-k8s.io/disable-update-class-name-check | It can be used to disable the webhook check on update that disallows a pre-existing Cluster to be populated with Topology information and Class.                                                                                                                                                                                                                                                                                                                                                                                                            | User                     | Clusters                                       |
 | unsafe.topology.cluster.x-k8s.io/disable-update-version-check    | It can be used to disable the webhook checks on update that disallows updating the .topology.spec.version on certain conditions.                                                                                                                                                                                                                                                                                                                                                                                                                            | User                     | Clusters                                       |
+
+
+# Internal Annotations
+
+Following annotation are used by CAPI internally. 
+
+<aside class="note warning">
+
+<h1>Internal annotations should not be used outside CAPI controllers</h1>
+
+Name, meaning and semantic of internal annotations can change anytime. 
+
+Users must not change or remove internal annotation on CAPI resources, because this can lead to issues or unexpected behaviour of the system.
+
+</aside>
+
+| Annotation                                                                   | Note                                                                                                                                                                                                                                                      | Applies to |
+|------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------|
+| in-place-updates.internal.cluster.x-k8s.io/acknowledge-move                  | This annotation is added by the MD controller to a MachineSet when it acknowledges a machine pending acknowledge after being moved from an oldMS                                                                                                          | MachineSet |
+| in-place-updates.internal.cluster.x-k8s.io/move-machines-to-machineset       | This annotation is added by the MD controller to the oldMS when it should scale down by moving machines that can be updated in-place to the newMS instead of deleting them.                                                                               | MachineSet |
+| in-place-updates.internal.cluster.x-k8s.io/pending-acknowledge-move          | This annotation is by the MS controller to a machine when being moved from the oldMS to the newMS                                                                                                                                                         | Machine    |
+| in-place-updates.internal.cluster.x-k8s.io/receive-machines-from-machinesets | This annotation is added by the MD controller to the newMS when it should receive replicas from an oldMS                                                                                                                                                  | MachineSet |
+| in-place-updates.internal.cluster.x-k8s.io/update-in-progress                | This annotation is added to machines by the controller owning the Machine when in-place update is started                                                                                                                                                 | Machine    |
+| topology.internal.cluster.x-k8s.io/upgrade-step                              | This is an annotation used by the topology controller to a cluster to track upgrade steps.                                                                                                                                                                | Clusters   |

internal/controllers/machinedeployment/machinedeployment_controller.go

@@ -43,6 +43,7 @@ import (
 	"sigs.k8s.io/cluster-api/controllers/external"
 	"sigs.k8s.io/cluster-api/internal/util/ssa"
 	"sigs.k8s.io/cluster-api/util"
+	"sigs.k8s.io/cluster-api/util/collections"
 	v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1"
 	"sigs.k8s.io/cluster-api/util/finalizers"
 	clog "sigs.k8s.io/cluster-api/util/log"
@@ -163,6 +164,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (retres ct
 		cluster:           cluster,
 	}
 
+	// Get machines.
+	selectorMap, err := metav1.LabelSelectorAsMap(&s.machineDeployment.Spec.Selector)
+	if err != nil {
+		return ctrl.Result{}, errors.Wrap(err, "failed to convert label selector to a map")
+	}
+	machineList := &clusterv1.MachineList{}
+	if err := r.Client.List(ctx, machineList, client.InNamespace(s.machineDeployment.Namespace), client.MatchingLabels(selectorMap)); err != nil {
+		return ctrl.Result{}, errors.Wrap(err, "failed to list Machines")
+	}
+	s.machines = collections.FromMachineList(machineList)
+
 	defer func() {
 		if err := r.updateStatus(ctx, s); err != nil {
 			reterr = kerrors.NewAggregate([]error{reterr, err})
@@ -195,6 +207,7 @@ type scope struct {
 	machineDeployment                            *clusterv1.MachineDeployment
 	cluster                                      *clusterv1.Cluster
 	machineSets                                  []*clusterv1.MachineSet
+	machines                                     collections.Machines
 	bootstrapTemplateNotFound                    bool
 	bootstrapTemplateExists                      bool
 	infrastructureTemplateNotFound               bool
@@ -291,15 +304,15 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error {
 	templateExists := s.infrastructureTemplateExists && (!md.Spec.Template.Spec.Bootstrap.ConfigRef.IsDefined() || s.bootstrapTemplateExists)
 
 	if ptr.Deref(md.Spec.Paused, false) {
-		return r.sync(ctx, md, s.machineSets, templateExists)
+		return r.sync(ctx, md, s.machineSets, s.machines, templateExists)
 	}
 
 	if md.Spec.Rollout.Strategy.Type == clusterv1.RollingUpdateMachineDeploymentStrategyType {
-		return r.rolloutRollingUpdate(ctx, md, s.machineSets, templateExists)
+		return r.rolloutRollingUpdate(ctx, md, s.machineSets, s.machines, templateExists)
 	}
 
 	if md.Spec.Rollout.Strategy.Type == clusterv1.OnDeleteMachineDeploymentStrategyType {
-		return r.rolloutOnDelete(ctx, md, s.machineSets, templateExists)
+		return r.rolloutOnDelete(ctx, md, s.machineSets, s.machines, templateExists)
 	}
 
 	return errors.Errorf("unexpected deployment strategy type: %s", md.Spec.Rollout.Strategy.Type)
@@ -320,7 +333,6 @@ func (r *Reconciler) createOrUpdateMachineSetsAndSyncMachineDeploymentRevision(c
 		log = log.WithValues("MachineSet", klog.KObj(ms))
 		ctx = ctrl.LoggerInto(ctx, log)
 
-		originalReplicas := ptr.Deref(ms.Spec.Replicas, 0)
 		if scaleIntent, ok := p.scaleIntents[ms.Name]; ok {
 			ms.Spec.Replicas = &scaleIntent
 		}
@@ -363,22 +375,30 @@ func (r *Reconciler) createOrUpdateMachineSetsAndSyncMachineDeploymentRevision(c
 		if !ok {
 			return errors.Errorf("failed to update MachineSet %s, original MS is missing", klog.KObj(ms))
 		}
+		originalReplicas := ptr.Deref(originalMS.Spec.Replicas, 0)
 
 		err := ssa.Patch(ctx, r.Client, machineDeploymentManagerName, ms, ssa.WithCachingProxy{Cache: r.ssaCache, Original: originalMS})
 		if err != nil {
 			r.recorder.Eventf(p.md, corev1.EventTypeWarning, "FailedUpdate", "Failed to update MachineSet %s: %v", klog.KObj(ms), err)
 			return errors.Wrapf(err, "failed to update MachineSet %s", klog.KObj(ms))
 		}
 
+		changes := getAnnotationChanges(originalMS, ms)
+
 		newReplicas := ptr.Deref(ms.Spec.Replicas, 0)
 		if newReplicas < originalReplicas {
-			log.Info(fmt.Sprintf("Scaled down MachineSet %s to %d replicas (-%d)", ms.Name, newReplicas, originalReplicas-newReplicas))
+			changes = append(changes, "replicas", newReplicas)
+			log.Info(fmt.Sprintf("Scaled down MachineSet %s to %d replicas (-%d)", ms.Name, newReplicas, originalReplicas-newReplicas), changes...)
 			r.recorder.Eventf(p.md, corev1.EventTypeNormal, "SuccessfulScale", "Scaled down MachineSet %v: %d -> %d", ms.Name, originalReplicas, newReplicas)
 		}
 		if newReplicas > originalReplicas {
-			log.Info(fmt.Sprintf("Scaled up MachineSet %s to %d replicas (+%d)", ms.Name, newReplicas, newReplicas-originalReplicas))
+			changes = append(changes, "replicas", newReplicas)
+			log.Info(fmt.Sprintf("Scaled up MachineSet %s to %d replicas (+%d)", ms.Name, newReplicas, newReplicas-originalReplicas), changes...)
 			r.recorder.Eventf(p.md, corev1.EventTypeNormal, "SuccessfulScale", "Scaled up MachineSet %v: %d -> %d", ms.Name, originalReplicas, newReplicas)
 		}
+		if newReplicas == originalReplicas && len(changes) > 0 {
+			log.Info(fmt.Sprintf("MachineSet %s updated", ms.Name), changes...)
+		}
 	}
 
 	// Surface the revision annotation on the MD level
@@ -392,6 +412,42 @@ func (r *Reconciler) createOrUpdateMachineSetsAndSyncMachineDeploymentRevision(c
 	return nil
 }
 
+func getAnnotationChanges(originalMS *clusterv1.MachineSet, ms *clusterv1.MachineSet) []any {
+	changes := []any{}
+	if originalMS.Annotations[clusterv1.MachineSetMoveMachinesToMachineSetAnnotation] != ms.Annotations[clusterv1.MachineSetMoveMachinesToMachineSetAnnotation] {
+		if value, ok := ms.Annotations[clusterv1.MachineSetMoveMachinesToMachineSetAnnotation]; ok {
+			changes = append(changes, clusterv1.MachineSetMoveMachinesToMachineSetAnnotation, value)
+		} else {
+			changes = append(changes, clusterv1.MachineSetMoveMachinesToMachineSetAnnotation, "(annotation removed)")
+		}
+	}
+
+	if originalMS.Annotations[clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation] != ms.Annotations[clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation] {
+		if value, ok := ms.Annotations[clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation]; ok {
+			changes = append(changes, clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation, value)
+		} else {
+			changes = append(changes, clusterv1.MachineSetReceiveMachinesFromMachineSetsAnnotation, "(annotation removed)")
+		}
+	}
+
+	if originalMS.Annotations[clusterv1.AcknowledgedMoveAnnotation] != ms.Annotations[clusterv1.AcknowledgedMoveAnnotation] {
+		if value, ok := ms.Annotations[clusterv1.AcknowledgedMoveAnnotation]; ok {
+			changes = append(changes, clusterv1.AcknowledgedMoveAnnotation, value)
+		} else {
+			changes = append(changes, clusterv1.AcknowledgedMoveAnnotation, "(annotation removed)")
+		}
+	}
+
+	if originalMS.Annotations[clusterv1.DisableMachineCreateAnnotation] != ms.Annotations[clusterv1.DisableMachineCreateAnnotation] {
+		if value, ok := ms.Annotations[clusterv1.DisableMachineCreateAnnotation]; ok {
+			changes = append(changes, clusterv1.DisableMachineCreateAnnotation, value)
+		} else {
+			changes = append(changes, clusterv1.DisableMachineCreateAnnotation, "(annotation removed)")
+		}
+	}
+	return changes
+}
+
 func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error {
 	log := ctrl.LoggerFrom(ctx)
 	if err := r.getAndAdoptMachineSetsForDeployment(ctx, s); err != nil {

internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go

@@ -27,12 +27,13 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
+	"sigs.k8s.io/cluster-api/util/collections"
 )
 
 // rolloutOnDelete reconcile machine sets controlled by a MachineDeployment that is using the OnDelete strategy.
-func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error {
+func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, machines collections.Machines, templateExists bool) error {
 	planner := newRolloutPlanner()
-	if err := planner.init(ctx, md, msList, nil, true, templateExists); err != nil {
+	if err := planner.init(ctx, md, msList, machines.UnsortedList(), true, templateExists); err != nil {
 		return err
 	}