kubernetes-sigs · k8s-ci-robot · Mar 12, 2025 · Mar 12, 2025 · Mar 12, 2025
diff --git a/controllers/vmware/test/controllers_test.go b/controllers/vmware/test/controllers_test.go
@@ -75,6 +75,9 @@ func newInfraCluster(namespace string, cluster *clusterv1.Cluster) client.Object
 func newAnonInfraCluster(namespace string) client.Object {
 	return &vmwarev1.VSphereCluster{
 		ObjectMeta: metav1.ObjectMeta{
+			Finalizers: []string{
+				vmwarev1.ClusterFinalizer,
+			},
 			GenerateName: "test-",
 			Namespace:    namespace,
 		},

diff --git a/controllers/vmware/vspherecluster_reconciler.go b/controllers/vmware/vspherecluster_reconciler.go
@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -82,6 +83,11 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return reconcile.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vsphereCluster, vmwarev1.ClusterFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Fetch the Cluster.
 	cluster, err := clusterutilv1.GetOwnerCluster(ctx, r.Client, vsphereCluster.ObjectMeta)
 	if err != nil {
@@ -131,13 +137,6 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return reconcile.Result{}, nil
 	}
 
-	// If the VSphereCluster doesn't have our finalizer, add it.
-	// Requeue immediately after adding finalizer to avoid the race condition between init and delete
-	if !controllerutil.ContainsFinalizer(vsphereCluster, vmwarev1.ClusterFinalizer) {
-		controllerutil.AddFinalizer(vsphereCluster, vmwarev1.ClusterFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	// Handle non-deleted clusters
 	return ctrl.Result{}, r.reconcileNormal(ctx, clusterContext)
 }

diff --git a/controllers/vspherecluster_reconciler.go b/controllers/vspherecluster_reconciler.go
@@ -35,6 +35,7 @@ import (
 	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -71,6 +72,11 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return reconcile.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vsphereCluster, infrav1.ClusterFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Fetch the CAPI Cluster.
 	cluster, err := clusterutilv1.GetOwnerCluster(ctx, r.Client, vsphereCluster.ObjectMeta)
 	if err != nil {
@@ -114,13 +120,6 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return r.reconcileDelete(ctx, clusterContext)
 	}
 
-	// If the VSphereCluster doesn't have our finalizer, add it.
-	// Requeue immediately after adding finalizer to avoid the race condition between init and delete
-	if !ctrlutil.ContainsFinalizer(vsphereCluster, infrav1.ClusterFinalizer) {
-		ctrlutil.AddFinalizer(vsphereCluster, infrav1.ClusterFinalizer)
-		return reconcile.Result{}, nil
-	}
-
 	// Handle non-deleted clusters
 	return r.reconcileNormal(ctx, clusterContext)
 }

diff --git a/controllers/vsphereclusteridentity_controller.go b/controllers/vsphereclusteridentity_controller.go
@@ -31,6 +31,7 @@ import (
 	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -83,6 +84,11 @@ func (r clusterIdentityReconciler) Reconcile(ctx context.Context, req reconcile.
 		return reconcile.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, identity, infrav1.VSphereClusterIdentityFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	if annotations.HasPaused(identity) {
 		log.Info("Reconciliation is paused for this object")
 		return reconcile.Result{}, nil
@@ -106,12 +112,6 @@ func (r clusterIdentityReconciler) Reconcile(ctx context.Context, req reconcile.
 		return ctrl.Result{}, r.reconcileDelete(ctx, identity)
 	}
 
-	// Add a finalizer and requeue to ensure that the secret is deleted when the identity is deleted.
-	if !ctrlutil.ContainsFinalizer(identity, infrav1.VSphereClusterIdentityFinalizer) {
-		ctrlutil.AddFinalizer(identity, infrav1.VSphereClusterIdentityFinalizer)
-		return reconcile.Result{}, nil
-	}
-
 	// fetch secret
 	secret := &corev1.Secret{}
 	secretKey := client.ObjectKey{

diff --git a/controllers/vspheredeploymentzone_controller.go b/controllers/vspheredeploymentzone_controller.go
@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -101,6 +102,11 @@ func (r vsphereDeploymentZoneReconciler) Reconcile(ctx context.Context, request
 	log = log.WithValues("VSphereFailureDomain", klog.KRef("", vsphereDeploymentZone.Spec.FailureDomain))
 	ctx = ctrl.LoggerInto(ctx, log)
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vsphereDeploymentZone, infrav1.DeploymentZoneFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	if annotations.HasPaused(vsphereDeploymentZone) {
 		log.Info("Reconciliation is paused for this object")
 		return reconcile.Result{}, nil
@@ -126,13 +132,6 @@ func (r vsphereDeploymentZoneReconciler) Reconcile(ctx context.Context, request
 		return ctrl.Result{}, r.reconcileDelete(ctx, vsphereDeploymentZoneContext)
 	}
 
-	// If the VSphereDeploymentZone doesn't have our finalizer, add it.
-	// Requeue immediately after adding finalizer to avoid the race condition between init and delete
-	if !ctrlutil.ContainsFinalizer(vsphereDeploymentZone, infrav1.DeploymentZoneFinalizer) {
-		ctrlutil.AddFinalizer(vsphereDeploymentZone, infrav1.DeploymentZoneFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	return ctrl.Result{}, r.reconcileNormal(ctx, vsphereDeploymentZoneContext)
 }
 

diff --git a/controllers/vspheremachine_controller.go b/controllers/vspheremachine_controller.go
@@ -36,6 +36,7 @@ import (
 	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	clog "sigs.k8s.io/cluster-api/util/log"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
@@ -194,6 +195,11 @@ func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return reconcile.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, machineContext.GetVSphereMachine(), infrav1.MachineFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Fetch the CAPI Machine.
 	machine, err := clusterutilv1.GetOwnerMachine(ctx, r.Client, machineContext.GetObjectMeta())
 	if err != nil {
@@ -294,13 +300,6 @@ func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return reconcile.Result{}, errors.Wrapf(err, "failed to get VSphereCluster")
 	}
 
-	// If the VSphereMachine doesn't have our finalizer, add it.
-	// Requeue immediately after adding finalizer to avoid the race condition between init and delete
-	if !ctrlutil.ContainsFinalizer(machineContext.GetVSphereMachine(), infrav1.MachineFinalizer) {
-		ctrlutil.AddFinalizer(machineContext.GetVSphereMachine(), infrav1.MachineFinalizer)
-		return reconcile.Result{}, nil
-	}
-
 	// Handle non-deleted machines
 	return r.reconcileNormal(ctx, machineContext)
 }

diff --git a/controllers/vspherevm_controller.go b/controllers/vspherevm_controller.go
@@ -37,6 +37,7 @@ import (
 	clusterutilv1 "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	clog "sigs.k8s.io/cluster-api/util/log"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
@@ -156,6 +157,11 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R
 		return reconcile.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vsphereVM, infrav1.VMFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	cluster, err := clusterutilv1.GetClusterFromMetadata(ctx, r.Client, vsphereVM.ObjectMeta)
 	if err != nil {
 		log.Error(err, "Failed to get Cluster from VSphereVM: Machine is missing cluster label or cluster does not exist")
@@ -294,15 +300,6 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R
 		}
 	}()
 
-	if vsphereVM.ObjectMeta.DeletionTimestamp.IsZero() {
-		// If the VSphereVM doesn't have our finalizer, add it.
-		// Requeue immediately to avoid the race condition between init and delete
-		if !ctrlutil.ContainsFinalizer(vsphereVM, infrav1.VMFinalizer) {
-			ctrlutil.AddFinalizer(vsphereVM, infrav1.VMFinalizer)
-			return reconcile.Result{}, nil
-		}
-	}
-
 	return r.reconcile(ctx, vmContext, fetchClusterModuleInput{
 		VSphereCluster: vsphereCluster,
 		Machine:        machine,

diff --git a/controllers/vspherevm_controller_test.go b/controllers/vspherevm_controller_test.go
@@ -433,6 +433,9 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "foo",
 			Namespace: "test",
+			Finalizers: []string{
+				infrav1.VMFinalizer,
+			},
 			Labels: map[string]string{
 				clusterv1.ClusterNameLabel: "valid-cluster",
 			},

diff --git a/test/infrastructure/vcsim/controllers/controlplaneendpoint_controller.go b/test/infrastructure/vcsim/controllers/controlplaneendpoint_controller.go
@@ -25,6 +25,7 @@ import (
 	"k8s.io/klog/v2"
 	inmemoryruntime "sigs.k8s.io/cluster-api/test/infrastructure/inmemory/pkg/runtime"
 	inmemoryserver "sigs.k8s.io/cluster-api/test/infrastructure/inmemory/pkg/server"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -59,6 +60,11 @@ func (r *ControlPlaneEndpointReconciler) Reconcile(ctx context.Context, req ctrl
 		return ctrl.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, controlPlaneEndpoint, vcsimv1.ControlPlaneEndpointFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Initialize the patch helper
 	patchHelper, err := patch.NewHelper(controlPlaneEndpoint, r.Client)
 	if err != nil {
@@ -77,13 +83,6 @@ func (r *ControlPlaneEndpointReconciler) Reconcile(ctx context.Context, req ctrl
 		return ctrl.Result{}, r.reconcileDelete(ctx, controlPlaneEndpoint)
 	}
 
-	// Add finalizer first if not set to avoid the race condition between init and delete.
-	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
-	if !controllerutil.ContainsFinalizer(controlPlaneEndpoint, vcsimv1.ControlPlaneEndpointFinalizer) {
-		controllerutil.AddFinalizer(controlPlaneEndpoint, vcsimv1.ControlPlaneEndpointFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	// Handle non-deleted machines
 	return ctrl.Result{}, r.reconcileNormal(ctx, controlPlaneEndpoint)
 }

diff --git a/test/infrastructure/vcsim/controllers/vcsim_controller.go b/test/infrastructure/vcsim/controllers/vcsim_controller.go
@@ -44,6 +44,7 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -93,6 +94,11 @@ func (r *VCenterSimulatorReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vCenterSimulator, vcsimv1.VCenterFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Initialize the patch helper
 	patchHelper, err := patch.NewHelper(vCenterSimulator, r.Client)
 	if err != nil {
@@ -112,13 +118,6 @@ func (r *VCenterSimulatorReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return ctrl.Result{}, nil
 	}
 
-	// Add finalizer first if not set to avoid the race condition between init and delete.
-	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
-	if !controllerutil.ContainsFinalizer(vCenterSimulator, vcsimv1.VCenterFinalizer) {
-		controllerutil.AddFinalizer(vCenterSimulator, vcsimv1.VCenterFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	// Handle non-deleted machines
 	return ctrl.Result{}, r.reconcileNormal(ctx, vCenterSimulator)
 }

diff --git a/test/infrastructure/vcsim/controllers/virtualmachine_controller.go b/test/infrastructure/vcsim/controllers/virtualmachine_controller.go
@@ -33,6 +33,7 @@ import (
 	inmemoryserver "sigs.k8s.io/cluster-api/test/infrastructure/inmemory/pkg/server"
 	capiutil "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -78,6 +79,11 @@ func (r *VirtualMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, virtualMachine, vcsimv1.VMFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Fetch the owner VSphereMachine.
 	vSphereMachine, err := util.GetOwnerVMWareMachine(ctx, r.Client, virtualMachine.ObjectMeta)
 	// vsphereMachine can be nil in cases where custom mover other than clusterctl
@@ -236,13 +242,6 @@ func (r *VirtualMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return r.reconcileDelete(ctx, cluster, machine, virtualMachine, conditionsTracker)
 	}
 
-	// Add finalizer first if not set to avoid the race condition between init and delete.
-	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
-	if !controllerutil.ContainsFinalizer(virtualMachine, vcsimv1.VMFinalizer) {
-		controllerutil.AddFinalizer(virtualMachine, vcsimv1.VMFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	// Handle non-deleted machines
 	return r.reconcileNormal(ctx, cluster, machine, virtualMachine, conditionsTracker)
 }

diff --git a/test/infrastructure/vcsim/controllers/vspherevm_controller.go b/test/infrastructure/vcsim/controllers/vspherevm_controller.go
@@ -32,6 +32,7 @@ import (
 	capiutil "sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util/finalizers"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -80,6 +81,11 @@ func (r *VSphereVMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, err
 	}
 
+	// Add finalizer first if not set to avoid the race condition between init and delete.
+	if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.Client, vSphereVM, vcsimv1.VMFinalizer); err != nil || finalizerAdded {
+		return ctrl.Result{}, err
+	}
+
 	// Fetch the owner VSphereMachine.
 	vSphereMachine, err := util.GetOwnerVSphereMachine(ctx, r.Client, vSphereVM.ObjectMeta)
 	// vsphereMachine can be nil in cases where custom mover other than clusterctl
@@ -237,13 +243,6 @@ func (r *VSphereVMReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return r.reconcileDelete(ctx, cluster, vSphereCluster, machine, vSphereVM, conditionsTracker)
 	}
 
-	// Add finalizer first if not set to avoid the race condition between init and delete.
-	// Note: Finalizers in general can only be added when the deletionTimestamp is not set.
-	if !controllerutil.ContainsFinalizer(vSphereVM, vcsimv1.VMFinalizer) {
-		controllerutil.AddFinalizer(vSphereVM, vcsimv1.VMFinalizer)
-		return ctrl.Result{}, nil
-	}
-
 	// Handle non-deleted machines
 	return r.reconcileNormal(ctx, cluster, vSphereCluster, machine, vSphereVM, conditionsTracker)
 }