Remove nil errors

Signed-off-by: z1cheng <[email protected]>

pkg/controllers/federate/controller.go

@@ -417,17 +417,14 @@ func (c *FederateController) ensureFinalizer(
 ) (*unstructured.Unstructured, error) {
 	logger := klog.FromContext(ctx).WithValues("finalizer", FinalizerFederateController)
 
-	isUpdated, err := finalizersutil.AddFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizer to source object: %w", err)
-	}
+	isUpdated := finalizersutil.AddFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
 	if !isUpdated {
 		return sourceObj, nil
 	}
 
 	logger.V(1).Info("Adding finalizer to source object")
 
-	sourceObj, err = c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
+	sourceObj, err := c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
 		ctx,
 		sourceObj,
 		metav1.UpdateOptions{},
@@ -446,17 +443,14 @@ func (c *FederateController) removeFinalizer(
 ) (*unstructured.Unstructured, error) {
 	logger := klog.FromContext(ctx).WithValues("finalizer", FinalizerFederateController)
 
-	isUpdated, err := finalizersutil.RemoveFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizer from source object: %w", err)
-	}
+	isUpdated := finalizersutil.RemoveFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
 	if !isUpdated {
 		return sourceObj, nil
 	}
 
 	logger.V(1).Info("Removing finalizer from source object")
 
-	sourceObj, err = c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
+	sourceObj, err := c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
 		ctx,
 		sourceObj,
 		metav1.UpdateOptions{},

pkg/controllers/federatedcluster/controller.go

@@ -353,10 +353,7 @@ func (c *FederatedClusterController) ensureFinalizer(
 	ctx context.Context,
 	cluster *fedcorev1a1.FederatedCluster,
 ) (*fedcorev1a1.FederatedCluster, error) {
-	updated, err := finalizers.AddFinalizers(cluster, sets.NewString(FinalizerFederatedClusterController))
-	if err != nil {
-		return nil, err
-	}
+	updated := finalizers.AddFinalizers(cluster, sets.NewString(FinalizerFederatedClusterController))
 
 	if updated {
 		return c.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})

pkg/controllers/sync/controller.go

@@ -624,12 +624,7 @@ func (s *SyncController) syncToClusters(
 			)
 			continue
 		}
-		hasFinalizer, err := finalizersutil.HasFinalizer(cluster, FinalizerCascadingDelete)
-		if err != nil {
-			shouldRecheckAfterDispatch = true
-			dispatcher.RecordClusterError(fedcorev1a1.FinalizerCheckFailed, clusterName, err)
-			continue
-		}
+		hasFinalizer := finalizersutil.HasFinalizer(cluster, FinalizerCascadingDelete)
 		if !hasFinalizer {
 			// we should not sync before finalizer is added
 			shouldRecheckAfterDispatch = true
@@ -971,9 +966,9 @@ func (s *SyncController) ensureFinalizer(ctx context.Context, fedResource Federa
 	ctx, keyedLogger := logging.InjectLoggerValues(ctx, "finalizer-name", FinalizerSyncController)
 
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.AddFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	isUpdated := finalizersutil.AddFinalizers(obj, sets.NewString(FinalizerSyncController))
+	if !isUpdated {
+		return fmt.Errorf("failed to add finalizer to federated object")
 	}
 
 	keyedLogger.V(1).Info("Adding finalizer to federated object")
@@ -995,9 +990,9 @@ func (s *SyncController) removeFinalizer(ctx context.Context, fedResource Federa
 	ctx, keyedLogger := logging.InjectLoggerValues(ctx, "finalizer-name", FinalizerSyncController)
 
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.RemoveFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	isUpdated := finalizersutil.RemoveFinalizers(obj, sets.NewString(FinalizerSyncController))
+	if !isUpdated {
+		return fmt.Errorf("failed to remove finalizer from federated object")
 	}
 
 	keyedLogger.V(1).Info("Removing finalizer from federated object")
@@ -1024,9 +1019,9 @@ func (s *SyncController) ensureClusterFinalizer(ctx context.Context, cluster *fe
 		if err != nil {
 			return err
 		}
-		isUpdated, err := finalizersutil.AddFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
-		if err != nil || !isUpdated {
-			return err
+		isUpdated := finalizersutil.AddFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
+		if !isUpdated {
+			return fmt.Errorf("failed to add finalizer to federated cluster")
 		}
 		cluster, err = s.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})
 		return err
@@ -1046,9 +1041,9 @@ func (s *SyncController) removeClusterFinalizer(ctx context.Context, cluster *fe
 		if err != nil {
 			return err
 		}
-		isUpdated, err := finalizersutil.RemoveFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
-		if err != nil || !isUpdated {
-			return err
+		isUpdated := finalizersutil.RemoveFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
+		if !isUpdated {
+			return fmt.Errorf("failed to remove finalizer from cluster")
 		}
 		cluster, err = s.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})
 		return err

pkg/controllers/sync/dispatch/unmanaged.go

@@ -132,19 +132,9 @@ func (d *unmanagedDispatcherImpl) Delete(
 		// Avoid mutating the resource in the informer cache
 		clusterObj := clusterObj.DeepCopy()
 
-		needUpdate, err := removeRetainObjectFinalizer(clusterObj)
-		if err != nil {
-			if d.recorder == nil {
-				wrappedErr := d.wrapOperationError(err, clusterName, op)
-				keyedLogger.Error(wrappedErr, "Failed to delete target object in cluster")
-			} else {
-				d.recorder.recordOperationError(ctx, fedcorev1a1.DeletionFailed, clusterName, op, err)
-			}
-			result = false
-			return result
-		}
+		needUpdate := removeRetainObjectFinalizer(clusterObj)
 		if needUpdate {
-			clusterObj, err = client.Resource(d.targetGVR).Namespace(targetName.Namespace).Update(
+			_, err := client.Resource(d.targetGVR).Namespace(targetName.Namespace).Update(
 				ctx,
 				clusterObj,
 				metav1.UpdateOptions{},
@@ -177,7 +167,7 @@ func (d *unmanagedDispatcherImpl) Delete(
 		// To avoid this, we explicitly set the PropagationPolicy to Background like `kubectl delete` does by default.
 		// Ref: https://github.com/kubernetes/kubernetes/pull/65908
 		deletionPropagation := metav1.DeletePropagationBackground
-		err = client.Resource(d.targetGVR).Namespace(targetName.Namespace).Delete(
+		err := client.Resource(d.targetGVR).Namespace(targetName.Namespace).Delete(
 			ctx,
 			targetName.Name,
 			metav1.DeleteOptions{PropagationPolicy: &deletionPropagation},
@@ -218,15 +208,7 @@ func (d *unmanagedDispatcherImpl) RemoveManagedLabel(
 		updateObj := clusterObj.DeepCopy()
 
 		managedlabel.RemoveManagedLabel(updateObj)
-		if _, err := removeRetainObjectFinalizer(updateObj); err != nil {
-			if d.recorder == nil {
-				wrappedErr := d.wrapOperationError(err, clusterName, op)
-				keyedLogger.Error(wrappedErr, "Failed to remove managed label from target object in cluster")
-			} else {
-				d.recorder.recordOperationError(ctx, fedcorev1a1.LabelRemovalFailed, clusterName, op, err)
-			}
-			return false
-		}
+		removeRetainObjectFinalizer(updateObj)
 
 		var err error
 		if _, err = client.Resource(d.targetGVR).Namespace(clusterObj.GetNamespace()).Update(
@@ -258,6 +240,6 @@ func wrapOperationError(err error, operation, targetGVR, targetName, clusterName
 	return errors.Wrapf(err, "Failed to "+eventTemplate, operation, targetGVR, targetName, clusterName)
 }
 
-func removeRetainObjectFinalizer(obj *unstructured.Unstructured) (bool, error) {
+func removeRetainObjectFinalizer(obj *unstructured.Unstructured) bool {
 	return finalizers.RemoveFinalizers(obj, sets.NewString(RetainTerminatingObjectFinalizer))
 }

pkg/controllers/sync/resource.go

@@ -198,9 +198,7 @@ func (r *federatedResource) ObjectForCluster(clusterName string) (*unstructured.
 }
 
 func addRetainObjectFinalizer(obj *unstructured.Unstructured) error {
-	if _, err := finalizers.AddFinalizers(obj, sets.NewString(dispatch.RetainTerminatingObjectFinalizer)); err != nil {
-		return err
-	}
+	finalizers.AddFinalizers(obj, sets.NewString(dispatch.RetainTerminatingObjectFinalizer))
 	return nil
 }
 

pkg/util/finalizers/finalizers.go

@@ -27,31 +27,31 @@ import (
 )
 
 // HasFinalizer returns true if the given object has the given finalizer in its ObjectMeta.
-func HasFinalizer(obj metav1.Object, finalizer string) (bool, error) {
+func HasFinalizer(obj metav1.Object, finalizer string) bool {
 	finalizers := sets.NewString(obj.GetFinalizers()...)
-	return finalizers.Has(finalizer), nil
+	return finalizers.Has(finalizer)
 }
 
 // AddFinalizers adds the given finalizers to the given objects ObjectMeta.
 // Returns true if the object was updated.
-func AddFinalizers(obj metav1.Object, newFinalizers sets.String) (bool, error) {
+func AddFinalizers(obj metav1.Object, newFinalizers sets.String) bool {
 	oldFinalizers := sets.NewString(obj.GetFinalizers()...)
 	if oldFinalizers.IsSuperset(newFinalizers) {
-		return false, nil
+		return false
 	}
 	allFinalizers := oldFinalizers.Union(newFinalizers)
 	obj.SetFinalizers(allFinalizers.List())
-	return true, nil
+	return true
 }
 
 // RemoveFinalizers removes the given finalizers from the given objects ObjectMeta.
 // Returns true if the object was updated.
-func RemoveFinalizers(obj metav1.Object, finalizers sets.String) (bool, error) {
+func RemoveFinalizers(obj metav1.Object, finalizers sets.String) bool {
 	oldFinalizers := sets.NewString(obj.GetFinalizers()...)
 	if oldFinalizers.Intersection(finalizers).Len() == 0 {
-		return false, nil
+		return false
 	}
 	newFinalizers := oldFinalizers.Difference(finalizers)
 	obj.SetFinalizers(newFinalizers.List())
-	return true, nil
+	return true
 }

pkg/util/finalizers/finalizers_test.go

@@ -74,7 +74,7 @@ func TestHasFinalizer(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		hasFinalizer, _ := HasFinalizer(test.obj, test.finalizer)
+		hasFinalizer := HasFinalizer(test.obj, test.finalizer)
 		assert.Equal(
 			t,
 			hasFinalizer,
@@ -123,7 +123,7 @@ func TestAddFinalizers(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		isUpdated, _ := AddFinalizers(test.obj, test.finalizers)
+		isUpdated := AddFinalizers(test.obj, test.finalizers)
 		assert.Equal(
 			t,
 			isUpdated,
@@ -184,7 +184,7 @@ func TestRemoveFinalizers(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		isUpdated, _ := RemoveFinalizers(test.obj, test.finalizers)
+		isUpdated := RemoveFinalizers(test.obj, test.finalizers)
 		assert.Equal(
 			t,
 			isUpdated,