Fix the finalizer code (#1513)

* Fix the finalizer code * Fix the unit tests
kubernetes-sigs · Nov 24, 2023 · bd65766 · bd65766
1 parent a37a749
commit bd65766
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 9 deletions.
diff --git a/controllers/ibmpowervscluster_controller.go b/controllers/ibmpowervscluster_controller.go
@@ -108,8 +108,7 @@ func (r *IBMPowerVSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Re
 }
 
 func (r *IBMPowerVSClusterReconciler) reconcile(clusterScope *scope.PowerVSClusterScope) ctrl.Result { //nolint:unparam
-	if !controllerutil.ContainsFinalizer(clusterScope.IBMPowerVSCluster, infrav1beta2.IBMPowerVSClusterFinalizer) {
-		controllerutil.AddFinalizer(clusterScope.IBMPowerVSCluster, infrav1beta2.IBMPowerVSClusterFinalizer)
+	if controllerutil.AddFinalizer(clusterScope.IBMPowerVSCluster, infrav1beta2.IBMPowerVSClusterFinalizer) {
 		return ctrl.Result{}
 	}
 

diff --git a/controllers/ibmpowervsimage_controller.go b/controllers/ibmpowervsimage_controller.go
@@ -100,7 +100,9 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 }
 
 func (r *IBMPowerVSImageReconciler) reconcile(cluster *infrav1beta2.IBMPowerVSCluster, imageScope *scope.PowerVSImageScope) (ctrl.Result, error) {
-	controllerutil.AddFinalizer(imageScope.IBMPowerVSImage, infrav1beta2.IBMPowerVSImageFinalizer)
+	if controllerutil.AddFinalizer(imageScope.IBMPowerVSImage, infrav1beta2.IBMPowerVSImageFinalizer) {
+		return ctrl.Result{}, nil
+	}
 
 	// Create new labels section for IBMPowerVSImage metadata if nil.
 	if imageScope.IBMPowerVSImage.Labels == nil {

diff --git a/controllers/ibmpowervsimage_controller_test.go b/controllers/ibmpowervsimage_controller_test.go
@@ -192,6 +192,7 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {
 							UID:        "1",
 						},
 					},
+					Finalizers: []string{infrav1beta2.IBMPowerVSImageFinalizer},
 				},
 				Spec: infrav1beta2.IBMPowerVSImageSpec{
 					ClusterName: "capi-powervs-cluster",

diff --git a/controllers/ibmpowervsmachine_controller.go b/controllers/ibmpowervsmachine_controller.go
@@ -218,7 +218,9 @@ func (r *IBMPowerVSMachineReconciler) reconcileNormal(machineScope *scope.PowerV
 		return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
 	}
 
-	controllerutil.AddFinalizer(machineScope.IBMPowerVSMachine, infrav1beta2.IBMPowerVSMachineFinalizer)
+	if controllerutil.AddFinalizer(machineScope.IBMPowerVSMachine, infrav1beta2.IBMPowerVSMachineFinalizer) {
+		return ctrl.Result{}, nil
+	}
 
 	ins, err := r.getOrCreate(machineScope)
 	if err != nil {

diff --git a/controllers/ibmpowervsmachine_controller_test.go b/controllers/ibmpowervsmachine_controller_test.go
@@ -447,7 +447,8 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) {
 				},
 				IBMPowerVSMachine: &infrav1beta2.IBMPowerVSMachine{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: *pointer.String("capi-test-machine"),
+						Name:       *pointer.String("capi-test-machine"),
+						Finalizers: []string{infrav1beta2.IBMPowerVSMachineFinalizer},
 					},
 				},
 				IBMPowerVSImage: &infrav1beta2.IBMPowerVSImage{
@@ -484,7 +485,8 @@ func TestIBMPowerVSMachineReconciler_ReconcileOperations(t *testing.T) {
 
 			pvsmachine := &infrav1beta2.IBMPowerVSMachine{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: *pointer.String("capi-test-machine"),
+					Name:       *pointer.String("capi-test-machine"),
+					Finalizers: []string{infrav1beta2.IBMPowerVSMachineFinalizer},
 				},
 				Spec: infrav1beta2.IBMPowerVSMachineSpec{
 					MemoryGiB:  8,

diff --git a/controllers/ibmvpccluster_controller.go b/controllers/ibmvpccluster_controller.go
@@ -110,8 +110,8 @@ func (r *IBMVPCClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 }
 
 func (r *IBMVPCClusterReconciler) reconcile(clusterScope *scope.ClusterScope) (ctrl.Result, error) {
-	if !controllerutil.ContainsFinalizer(clusterScope.IBMVPCCluster, infrav1beta2.ClusterFinalizer) {
-		controllerutil.AddFinalizer(clusterScope.IBMVPCCluster, infrav1beta2.ClusterFinalizer)
+	// If the IBMVPCCluster doesn't have our finalizer, add it.
+	if controllerutil.AddFinalizer(clusterScope.IBMVPCCluster, infrav1beta2.ClusterFinalizer) {
 		return ctrl.Result{}, nil
 	}
 

diff --git a/controllers/ibmvpcmachine_controller.go b/controllers/ibmvpcmachine_controller.go
@@ -140,7 +140,9 @@ func (r *IBMVPCMachineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *IBMVPCMachineReconciler) reconcileNormal(machineScope *scope.MachineScope) (ctrl.Result, error) {
-	controllerutil.AddFinalizer(machineScope.IBMVPCMachine, infrav1beta2.MachineFinalizer)
+	if controllerutil.AddFinalizer(machineScope.IBMVPCMachine, infrav1beta2.MachineFinalizer) {
+		return ctrl.Result{}, nil
+	}
 
 	// Make sure bootstrap data is available and populated.
 	if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {

diff --git a/controllers/ibmvpcmachine_controller_test.go b/controllers/ibmvpcmachine_controller_test.go
@@ -233,6 +233,7 @@ func TestIBMVPCMachineReconciler_reconcile(t *testing.T) {
 					Labels: map[string]string{
 						capiv1beta1.MachineControlPlaneNameLabel: "capi-control-plane-machine",
 					},
+					Finalizers: []string{infrav1beta2.MachineFinalizer},
 				},
 			},
 			Machine: &capiv1beta1.Machine{
@@ -290,6 +291,7 @@ func TestIBMVPCMachineLBReconciler_reconcile(t *testing.T) {
 					Labels: map[string]string{
 						capiv1beta1.MachineControlPlaneNameLabel: "capi-control-plane-machine",
 					},
+					Finalizers: []string{infrav1beta2.MachineFinalizer},
 				},
 			},
 			Machine: &capiv1beta1.Machine{