Fix finalizer calculation in MD/MS controller

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

internal/controllers/machinedeployment/machinedeployment_sync.go

@@ -257,14 +257,13 @@ func (r *Reconciler) computeDesiredMachineSet(ctx context.Context, deployment *c
 		name = existingMS.Name
 		uid = existingMS.UID
 
-		// Keep foregroundDeletion finalizer if the existingMS has it.
-		// Note: This case is a little different from the create case. In the update case we preserve
-		// the finalizer on the MachineSet if it already exists. Because of SSA we should not build
-		// the finalizer information from the MachineDeployment when updating a MachineSet because that could lead
-		// to dropping the finalizer from the MachineSet if it is dropped from the MachineDeployment.
-		// We should not drop the finalizer on the MachineSet if the finalizer is dropped from the MachineDeployment.
-		if sets.New[string](existingMS.Finalizers...).Has(metav1.FinalizerDeleteDependents) {
-			finalizers = []string{metav1.FinalizerDeleteDependents}
+		// Preserve all existing finalizers (including foregroundDeletion finalizer).
+		// Ensure MachineSet has foregroundDeletion finalizer if the MachineDeployment has it.
+		// We should not drop the foregroundDeletion finalizer on the MachineSet if the finalizer is dropped from the MachineDeployment.
+		finalizers = existingMS.Finalizers
+		if sets.New[string](deployment.Finalizers...).Has(metav1.FinalizerDeleteDependents) &&
+			!sets.New[string](finalizers...).Has(metav1.FinalizerDeleteDependents) {
+			finalizers = append(finalizers, metav1.FinalizerDeleteDependents)
 		}
 
 		replicas = *existingMS.Spec.Replicas

internal/controllers/machineset/machineset_controller.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/client-go/tools/record"
@@ -613,7 +614,6 @@ func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, exi
 			OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(machineSet, machineSetKind)},
 			Labels:          map[string]string{},
 			Annotations:     map[string]string{},
-			Finalizers:      []string{clusterv1.MachineFinalizer},
 		},
 		Spec: *machineSet.Spec.Template.Spec.DeepCopy(),
 	}
@@ -635,10 +635,34 @@ func (r *Reconciler) computeDesiredMachine(machineSet *clusterv1.MachineSet, exi
 	// Note: we use UID to force SSA to update the existing Machine and to not accidentally create a new Machine.
 	// infrastructureRef and bootstrap.configRef remain the same for an existing Machine.
 	if existingMachine != nil {
+		// Update case.
 		desiredMachine.SetName(existingMachine.Name)
 		desiredMachine.SetUID(existingMachine.UID)
+
+		// Preserve all existing finalizers (including foregroundDeletion finalizer).
+		// Ensure MachineFinalizer is set.
+		// Ensure Machine has foregroundDeletion finalizer if the MachineSet has it.
+		// We should not drop the foregroundDeletion finalizer on the Machine if the finalizer is dropped from the MachineSet.
+		finalizers := existingMachine.Finalizers
+		if !sets.New[string](finalizers...).Has(clusterv1.MachineFinalizer) {
+			finalizers = append(finalizers, clusterv1.MachineFinalizer)
+		}
+		if sets.New[string](machineSet.Finalizers...).Has(metav1.FinalizerDeleteDependents) &&
+			!sets.New[string](finalizers...).Has(metav1.FinalizerDeleteDependents) {
+			finalizers = append(finalizers, metav1.FinalizerDeleteDependents)
+		}
+
 		desiredMachine.Spec.Bootstrap.ConfigRef = existingMachine.Spec.Bootstrap.ConfigRef
 		desiredMachine.Spec.InfrastructureRef = existingMachine.Spec.InfrastructureRef
+	} else {
+		// Create case.
+		// Ensure MachineFinalizer is set.
+		finalizers := []string{clusterv1.MachineFinalizer}
+		// Add foregroundDeletion finalizer to Machine if the MachineSet has it.
+		if sets.New[string](machineSet.Finalizers...).Has(metav1.FinalizerDeleteDependents) {
+			finalizers = append(finalizers, metav1.FinalizerDeleteDependents)
+		}
+		desiredMachine.Finalizers = finalizers
 	}
 
 	// Set the in-place mutable fields.