removing finalizer manager method

Author: yash97

controllers/crds/cninode_controller.go

@@ -133,7 +133,7 @@ func (r *CNINodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 	if cniNode.GetDeletionTimestamp().IsZero() {
 		cniNodeCopy := cniNode.DeepCopy()
-		shouldPatch, err := r.ensureTagsAndLables(cniNodeCopy, node, nodeFound)
+		shouldPatch, err := r.ensureTagsAndLabels(cniNodeCopy, node, nodeFound)
 		shouldPatch = r.ensureFinalizer(cniNodeCopy) || shouldPatch
 
 		if shouldPatch {
@@ -164,8 +164,11 @@ func (r *CNINodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 				}
 			}
 
-			if err := r.finalizerManager.RemoveFinalizers(ctx, cniNode, config.NodeTerminationFinalizer); err != nil {
+			if err := r.removeFinalizers(ctx, cniNode, config.NodeTerminationFinalizer); err != nil {
 				r.log.Error(err, "failed to remove finalizer on CNINode, will retry", "cniNode", cniNode.Name, "finalizer", config.NodeTerminationFinalizer)
+				if apierrors.IsConflict(err) {
+					return ctrl.Result{Requeue: true}, nil
+				}
 				return ctrl.Result{}, err
 			}
 			return ctrl.Result{}, nil
@@ -185,7 +188,7 @@ func (r *CNINodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 				Spec: cniNode.Spec,
 			}
 
-			if err := r.finalizerManager.RemoveFinalizers(ctx, cniNode, config.NodeTerminationFinalizer); err != nil {
+			if err := r.removeFinalizers(ctx, cniNode, config.NodeTerminationFinalizer); err != nil {
 				r.log.Error(err, "failed to remove finalizer on CNINode, will retry")
 				return ctrl.Result{}, err
 			}
@@ -255,7 +258,7 @@ func (r *CNINodeReconciler) GetNodeID(node *v1.Node) (string, error) {
 	return "", fmt.Errorf("invalid provider ID format for node %s, with providerId", node.Spec.ProviderID)
 }
 
-func (r *CNINodeReconciler) ensureTagsAndLables(cniNode *v1alpha1.CNINode, node *v1.Node, nodeFound bool) (bool, error) {
+func (r *CNINodeReconciler) ensureTagsAndLabels(cniNode *v1alpha1.CNINode, node *v1.Node, nodeFound bool) (bool, error) {
 	shouldPatch := false
 	var err error
 	if cniNode.Spec.Tags == nil {
@@ -296,3 +299,19 @@ func (r *CNINodeReconciler) ensureFinalizer(cniNode *v1alpha1.CNINode) bool {
 	}
 	return shouldPatch
 }
+
+func (r *CNINodeReconciler) removeFinalizers(ctx context.Context, cniNode *v1alpha1.CNINode, finalizer string) error {
+	cniNodeCopy := cniNode.DeepCopy()
+	needsUpdate := false
+
+	if controllerutil.ContainsFinalizer(cniNodeCopy, finalizer) {
+		r.log.Info("removing finalizer for cninode", "name", cniNode.GetName(), "finalizer", finalizer)
+		controllerutil.RemoveFinalizer(cniNodeCopy, finalizer)
+		needsUpdate = true
+	}
+
+	if !needsUpdate {
+		return nil
+	}
+	return r.Client.Patch(ctx, cniNodeCopy, client.MergeFromWithOptions(cniNode, client.MergeFromWithOptimisticLock{}))
+}

controllers/crds/cninode_controller_test.go

@@ -121,10 +121,6 @@ func TestCNINodeReconcile(t *testing.T) {
 					return f.mockResourceCleaner
 				}
 				f.mockResourceCleaner.EXPECT().DeleteLeakedResources().Times(0)
-
-				f.mockFinalizerManager.EXPECT().
-					RemoveFinalizers(gomock.Any(), gomock.Any(), config.NodeTerminationFinalizer).
-					Return(nil)
 			},
 			asserts: func(res reconcile.Result, err error, cniNode *v1alpha1.CNINode) {
 				assert.NoError(t, err)
@@ -157,9 +153,6 @@ func TestCNINodeReconcile(t *testing.T) {
 					return f.mockResourceCleaner
 				}
 				f.mockResourceCleaner.EXPECT().DeleteLeakedResources().Times(1).Return(nil)
-				f.mockFinalizerManager.EXPECT().
-					RemoveFinalizers(gomock.Any(), gomock.Any(), config.NodeTerminationFinalizer).
-					Return(nil)
 
 			},
 			asserts: func(res reconcile.Result, err error, cniNode *v1alpha1.CNINode) {
@@ -180,7 +173,7 @@ func TestCNINodeReconcile(t *testing.T) {
 					},
 					Spec: v1alpha1.CNINodeSpec{
 						Tags: map[string]string{
-							config.VPCCNIClusterNameKey: mockClusterName,
+							config.VPCCNIClusterNameKey:      mockClusterName,
 							config.NetworkInterfaceNodeIDKey: "i-1234567890",
 						},
 					},
@@ -193,6 +186,7 @@ func TestCNINodeReconcile(t *testing.T) {
 				assert.Contains(t, cniNode.Finalizers, config.NodeTerminationFinalizer)
 			},
 		},
+		
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -218,8 +212,10 @@ func TestCNINodeReconcile(t *testing.T) {
 			res, err := mock.Reconciler.Reconcile(context.Background(), reconcileRequest)
 
 			cniNode := &v1alpha1.CNINode{}
-			getErr := mock.Reconciler.Client.Get(context.Background(), reconcileRequest.NamespacedName, cniNode)
-			assert.NoError(t, getErr)
+			if tt.args.mockCNINode.GetDeletionTimestamp() == nil {
+				getErr := mock.Reconciler.Client.Get(context.Background(), reconcileRequest.NamespacedName, cniNode)
+				assert.NoError(t, getErr)
+			}
 
 			if tt.asserts != nil {
 				tt.asserts(res, err, cniNode)