Add Taint during rolling update

api/v1beta1/common_types.go

@@ -217,6 +217,14 @@ const (
 	MachineSetPreflightCheckControlPlaneIsStable MachineSetPreflightCheck = "ControlPlaneIsStable"
 )
 
+// NodeOutdatedRevisionTaint can be added to Nodes at rolling updates in general triggered by updating MachineDeployment
+// This taint is used to prevent unnecessary pod churn, i.e., as the first node is drained, pods previously running on
+// that node are scheduled onto nodes who have yet to be replaced, but will be torn down soon.
+var NodeOutdatedRevisionTaint = corev1.Taint{
+	Key:    "node.cluster.x-k8s.io/outdated-revision",
+	Effect: corev1.TaintEffectPreferNoSchedule,
+}
+
 // NodeUninitializedTaint can be added to Nodes at creation by the bootstrap provider, e.g. the
 // KubeadmBootstrap provider will add the taint.
 // This taint is used to prevent workloads to be scheduled on Nodes before the node is initialized by Cluster API.

internal/controllers/machine/machine_controller.go

@@ -100,6 +100,10 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	if err != nil {
 		return err
 	}
+	msToMachines, err := util.MachineSetToObjectsMapper(mgr.GetClient(), &clusterv1.MachineList{}, mgr.GetScheme())
+	if err != nil {
+		return err
+	}
 
 	if r.nodeDeletionRetryTimeout.Nanoseconds() == 0 {
 		r.nodeDeletionRetryTimeout = 10 * time.Second
@@ -122,6 +126,10 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 					predicates.ResourceHasFilterLabel(ctrl.LoggerFrom(ctx), r.WatchFilterValue),
 				),
 			)).
+		Watches(
+			&clusterv1.MachineSet{},
+			handler.EnqueueRequestsFromMapFunc(msToMachines),
+		).
 		Build(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")

internal/controllers/machine/machine_controller_noderef.go

@@ -31,6 +31,7 @@ import (
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/api/v1beta1/index"
+	"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
 	"sigs.k8s.io/cluster-api/internal/util/taints"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
@@ -133,7 +134,7 @@ func (r *Reconciler) reconcileNode(ctx context.Context, s *scope) (ctrl.Result,
 	_, nodeHadInterruptibleLabel := node.Labels[clusterv1.InterruptibleLabel]
 
 	// Reconcile node taints
-	if err := r.patchNode(ctx, remoteClient, node, nodeLabels, nodeAnnotations); err != nil {
+	if err := r.patchNode(ctx, remoteClient, node, nodeLabels, nodeAnnotations, machine); err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile Node %s", klog.KObj(node))
 	}
 	if !nodeHadInterruptibleLabel && interruptible {
@@ -255,7 +256,9 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID st
 
 // PatchNode is required to workaround an issue on Node.Status.Address which is incorrectly annotated as patchStrategy=merge
 // and this causes SSA patch to fail in case there are two addresses with the same key https://github.com/kubernetes-sigs/cluster-api/issues/8417
-func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string) error {
+func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client, node *corev1.Node, newLabels, newAnnotations map[string]string, m *clusterv1.Machine) error {
+	log := ctrl.LoggerFrom(ctx)
+
 	newNode := node.DeepCopy()
 
 	// Adds the annotations CAPI sets on the node.
@@ -292,9 +295,53 @@ func (r *Reconciler) patchNode(ctx context.Context, remoteClient client.Client,
 	// Drop the NodeUninitializedTaint taint on the node given that we are reconciling labels.
 	hasTaintChanges := taints.RemoveNodeTaint(newNode, clusterv1.NodeUninitializedTaint)
 
+	// Set Taint to a node in an old machineDeployment and unset Taint from a node in a new machineDeployment
+	// Ignore errors as it's not critical for the reconcile.
+	// To avoid an unnecessary Taint remaining due to the error remove Taint when errors occur.
+	isOutdated, err := isNodeOutdated(ctx, r.Client, m)
+	if err != nil {
+		log.V(2).Info("Failed to check if Node %s is outdated", "err", err, "node name", node.Name)
+	}
+	if isOutdated {
+		hasTaintChanges = taints.EnsureNodeTaint(newNode, clusterv1.NodeOutdatedRevisionTaint) || hasTaintChanges
+	} else {
+		hasTaintChanges = taints.RemoveNodeTaint(newNode, clusterv1.NodeOutdatedRevisionTaint) || hasTaintChanges
+	}
+
 	if !hasAnnotationChanges && !hasLabelChanges && !hasTaintChanges {
 		return nil
 	}
 
 	return remoteClient.Patch(ctx, newNode, client.StrategicMergeFrom(node))
 }
+
+func isNodeOutdated(ctx context.Context, c client.Client, m *clusterv1.Machine) (bool, error) {
+	ms := &clusterv1.MachineSet{}
+	objKey := client.ObjectKey{
+		Namespace: m.ObjectMeta.Namespace,
+		Name:      m.Labels[clusterv1.MachineSetNameLabel],
+	}
+	if err := c.Get(ctx, objKey, ms); err != nil {
+		return false, err
+	}
+	md := &clusterv1.MachineDeployment{}
+	objKey = client.ObjectKey{
+		Namespace: m.ObjectMeta.Namespace,
+		Name:      m.Labels[clusterv1.MachineDeploymentNameLabel],
+	}
+	if err := c.Get(ctx, objKey, md); err != nil {
+		return false, err
+	}
+	msRev, err := mdutil.Revision(ms)
+	if err != nil {
+		return false, err
+	}
+	mdRev, err := mdutil.Revision(md)
+	if err != nil {
+		return false, err
+	}
+	if msRev < mdRev {
+		return true, nil
+	}
+	return false, nil
+}