refactor: remove redundant the returned error

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

pkg/controllers/nsautoprop/controller.go

@@ -18,7 +18,6 @@ package nsautoprop
 
 import (
 	"context"
-	"fmt"
 	"regexp"
 	"strings"
 	"time"
@@ -371,12 +370,7 @@ func (c *Controller) ensureAnnotation(
 	fedNamespace *fedcorev1a1.ClusterFederatedObject,
 	key, value string,
 ) (bool, error) {
-	needsUpdate, err := annotationutil.AddAnnotation(fedNamespace, key, value)
-	if err != nil {
-		return false, fmt.Errorf(
-			"failed to add %s annotation to %s, err: %w",
-			key, fedNamespace.GetName(), err)
-	}
+	needsUpdate := annotationutil.AddAnnotation(fedNamespace, key, value)
 
 	return needsUpdate, nil
 }

pkg/controllers/scheduler/scheduler.go

@@ -481,11 +481,7 @@ func (s *Scheduler) prepareToSchedule(
 		logger.Error(err, "Failed to compute scheduling annotations")
 		return nil, nil, nil, &worker.StatusError
 	}
-	annotationChanged, err := updateSchedulingAnnotations(triggersText, deferredReasons, fedObject)
-	if err != nil {
-		logger.Error(err, "Failed to update scheduling annotations")
-		return nil, nil, nil, &worker.StatusError
-	}
+	annotationChanged := updateSchedulingAnnotations(triggersText, deferredReasons, fedObject)
 
 	shouldSkipScheduling := false
 	if !triggersChanged {

pkg/controllers/scheduler/schedulingtriggers.go

@@ -225,17 +225,14 @@ func computeSchedulingAnnotations(
 func updateSchedulingAnnotations(
 	triggers, deferredReasons string,
 	fedObject fedcorev1a1.GenericFederatedObject,
-) (annotationChanged bool, err error) {
-	triggersChanged, err := annotation.AddAnnotation(fedObject, SchedulingTriggersAnnotation, triggers)
-	if err != nil {
-		return false, err
-	}
+) (annotationChanged bool) {
+	triggersChanged := annotation.AddAnnotation(fedObject, SchedulingTriggersAnnotation, triggers)
 	if len(deferredReasons) == 0 {
-		deferred, err := annotation.RemoveAnnotation(fedObject, SchedulingDeferredReasonsAnnotation)
-		return triggersChanged || deferred, err
+		deferred := annotation.RemoveAnnotation(fedObject, SchedulingDeferredReasonsAnnotation)
+		return triggersChanged || deferred
 	}
-	deferred, err := annotation.AddAnnotation(fedObject, SchedulingDeferredReasonsAnnotation, deferredReasons)
-	return triggersChanged || deferred, err
+	deferred := annotation.AddAnnotation(fedObject, SchedulingDeferredReasonsAnnotation, deferredReasons)
+	return triggersChanged || deferred
 }
 
 func computeSchedulingTriggers(

pkg/controllers/scheduler/schedulingtriggers_test.go

@@ -614,11 +614,7 @@ func Test_computeSchedulingAnnotations(t *testing.T) {
 					t.Errorf("Marshal() unexpected err: %v", err)
 					return
 				}
-				_, err = annotation.AddAnnotation(fedObj, SchedulingTriggersAnnotation, oldTriggerText)
-				if err != nil {
-					t.Errorf("AddAnnotation() unexpected err: %v", err)
-					return
-				}
+				annotation.AddAnnotation(fedObj, SchedulingTriggersAnnotation, oldTriggerText)
 			}
 
 			newPolicy := generatePolicy(tt.newStatus.policyName, tt.newStatus.reschedulePolicy)

pkg/controllers/status/controller.go

@@ -414,14 +414,11 @@ func (s *StatusController) reconcile(
 
 	var hasRSDigestsAnnotation bool
 	if existingStatus != nil {
-		hasRSDigestsAnnotation, err = annotation.HasAnnotationKeyValue(
+		hasRSDigestsAnnotation = annotation.HasAnnotationKeyValue(
 			existingStatus,
 			common.LatestReplicasetDigestsAnnotation,
 			rsDigestsAnnotation,
 		)
-		if err != nil {
-			return worker.StatusError
-		}
 	}
 
 	collectedStatus := newCollectedStatusObject(fedObject, clusterStatuses)

pkg/controllers/statusaggregator/plugins/deployment.go

@@ -156,25 +156,19 @@ func (receiver *DeploymentPlugin) AggregateStatuses(
 	}
 
 	rsDigestsAnnotation := string(rsDigestsAnnotationBytes)
-	hasRSDigestsAnnotation, err := annotation.HasAnnotationKeyValue(
+	hasRSDigestsAnnotation := annotation.HasAnnotationKeyValue(
 		sourceObject,
 		common.LatestReplicasetDigestsAnnotation,
 		rsDigestsAnnotation,
 	)
-	if err != nil {
-		return nil, false, err
-	}
 
 	if hasRSDigestsAnnotation {
 		return sourceObject, needUpdate, nil
 	} else {
 		needUpdate = true
 	}
 
-	_, err = annotation.AddAnnotation(sourceObject, common.LatestReplicasetDigestsAnnotation, rsDigestsAnnotation)
-	if err != nil {
-		return nil, false, err
-	}
+	annotation.AddAnnotation(sourceObject, common.LatestReplicasetDigestsAnnotation, rsDigestsAnnotation)
 
 	return sourceObject, needUpdate, nil
 }

pkg/controllers/sync/resource.go

@@ -166,9 +166,7 @@ func (r *federatedResource) ObjectForCluster(clusterName string) (*unstructured.
 	obj := r.template.DeepCopy()
 
 	if obj.GetGeneration() != 0 {
-		if _, err := annotationutil.AddAnnotation(obj, common.SourceGenerationAnnotation, fmt.Sprintf("%d", obj.GetGeneration())); err != nil {
-			return nil, err
-		}
+		annotationutil.AddAnnotation(obj, common.SourceGenerationAnnotation, fmt.Sprintf("%d", obj.GetGeneration()))
 	}
 
 	switch r.TargetGVK() {

pkg/util/annotation/annotation.go

@@ -17,9 +17,6 @@ limitations under the License.
 package annotation
 
 import (
-	"fmt"
-	"reflect"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -30,87 +27,56 @@ const (
 )
 
 // HasAnnotationKey returns true if the given object has the given annotation key in its ObjectMeta.
-func HasAnnotationKey(obj metav1.Object, key string) (bool, error) {
-	if IsNilPointer(obj) {
-		return false, fmt.Errorf("object(%T) is nil pointer", obj)
-	}
+func HasAnnotationKey(obj metav1.Object, key string) bool {
 	annotations := obj.GetAnnotations()
 	if annotations == nil {
-		return false, nil
+		return false
 	}
 	_, ok := annotations[key]
-	return ok, nil
+	return ok
 }
 
 // HasAnnotationKeyValue returns true if the given object has the given annotation key and value in its ObjectMeta.
-func HasAnnotationKeyValue(obj metav1.Object, key, value string) (bool, error) {
-	if IsNilPointer(obj) {
-		return false, fmt.Errorf("object(%T) is nil pointer", obj)
-	}
+func HasAnnotationKeyValue(obj metav1.Object, key, value string) bool {
 	annotations := obj.GetAnnotations()
 	if annotations == nil {
-		return false, nil
+		return false
 	}
 	val, ok := annotations[key]
-	return ok && value == val, nil
+	return ok && value == val
 }
 
 // AddAnnotation adds the given annotation key and value to the given objects ObjectMeta,
 // and overwrites the annotation value if it already exists.
 // Returns true if the object was updated.
-func AddAnnotation(obj metav1.Object, key, value string) (bool, error) {
-	if IsNilPointer(obj) {
-		return false, fmt.Errorf("object(%T) is nil pointer", obj)
-	}
-
-	if key == "" {
-		return false, fmt.Errorf("key is a empty string.")
-	}
-
-	has, err := HasAnnotationKeyValue(obj, key, value)
-	if has && err == nil {
-		return false, nil
-	}
-	if err != nil {
-		return false, err
+func AddAnnotation(obj metav1.Object, key, value string) bool {
+	has := HasAnnotationKeyValue(obj, key, value)
+	if has {
+		return false
 	}
 	annotations := obj.GetAnnotations()
 	if annotations == nil {
 		annotations = make(map[string]string)
 	}
 	annotations[key] = value
 	obj.SetAnnotations(annotations)
-	return true, nil
+	return true
 }
 
 // RemoveAnnotation removes the given annotation key from the given objects ObjectMeta.
 // Returns true if the object was updated.
-func RemoveAnnotation(obj metav1.Object, key string) (bool, error) {
-	if IsNilPointer(obj) {
-		return false, fmt.Errorf("object(%T) is nil pointer", obj)
-	}
-	has, err := HasAnnotationKey(obj, key)
-	if !has && err == nil {
-		return false, nil
-	}
-	if err != nil {
-		return false, err
+func RemoveAnnotation(obj metav1.Object, key string) bool {
+	has := HasAnnotationKey(obj, key)
+	if !has {
+		return false
 	}
 
 	annotations := obj.GetAnnotations()
 	if annotations == nil {
-		return false, nil
+		return false
 	}
 
 	delete(annotations, key)
 	obj.SetAnnotations(annotations)
-	return true, nil
-}
-
-// IsNilPointer returns true if i is nil pointer or value of i is nil.
-func IsNilPointer(i interface{}) bool {
-	if i == nil || (reflect.ValueOf(i).Kind() == reflect.Ptr && reflect.ValueOf(i).IsNil()) {
-		return true
-	}
-	return false
+	return true
 }

pkg/util/annotation/annotation_test.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
-	meta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -38,11 +37,6 @@ func TestHasAnnotationKey(t *testing.T) {
 		annotation string
 		result     bool
 	}{
-		{
-			newObj(map[string]string{}),
-			"",
-			false,
-		},
 		{
 			newObj(map[string]string{}),
 			"someAnnotation",
@@ -70,7 +64,7 @@ func TestHasAnnotationKey(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		hasAnnotationKey, _ := HasAnnotationKey(test.obj, test.annotation)
+		hasAnnotationKey := HasAnnotationKey(test.obj, test.annotation)
 		assert.Equal(
 			t,
 			hasAnnotationKey,
@@ -87,24 +81,12 @@ func TestHasAnnotationKeyValue(t *testing.T) {
 		value  string
 		result bool
 	}{
-		{
-			newObj(map[string]string{}),
-			"",
-			"",
-			false,
-		},
 		{
 			newObj(map[string]string{}),
 			"someAnnotation",
 			"",
 			false,
 		},
-		{
-			newObj(map[string]string{"someAnnotation": ""}),
-			"",
-			"",
-			false,
-		},
 		{
 			newObj(map[string]string{"someAnnotation": ""}),
 			"anotherAnnotation",
@@ -137,7 +119,7 @@ func TestHasAnnotationKeyValue(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		hasAnnotationKeyValue, _ := HasAnnotationKeyValue(test.obj, test.key, test.value)
+		hasAnnotationKeyValue := HasAnnotationKeyValue(test.obj, test.key, test.value)
 		assert.Equal(
 			t,
 			hasAnnotationKeyValue,
@@ -155,20 +137,6 @@ func TestAddAnnotation(t *testing.T) {
 		isUpdated      bool
 		newAnnotations map[string]string
 	}{
-		{
-			newObj(map[string]string{}),
-			"",
-			"",
-			false,
-			map[string]string{},
-		},
-		{
-			newObj(nil),
-			"",
-			"",
-			false,
-			map[string]string(nil),
-		},
 		{
 			newObj(map[string]string{}),
 			"someAnnotation",
@@ -206,15 +174,14 @@ func TestAddAnnotation(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		isUpdated, _ := AddAnnotation(test.obj, test.key, test.value)
+		isUpdated := AddAnnotation(test.obj, test.key, test.value)
 		assert.Equal(
 			t,
 			isUpdated,
 			test.isUpdated,
 			fmt.Sprintf("Test case %d failed. Expected isUpdated: %v, actual: %v", index, test.isUpdated, isUpdated),
 		)
-		accessor, _ := meta.Accessor(test.obj)
-		newAnnotations := accessor.GetAnnotations()
+		newAnnotations := test.obj.GetAnnotations()
 		assert.Equal(
 			t,
 			test.newAnnotations,
@@ -236,18 +203,6 @@ func TestRemoveAnnotation(t *testing.T) {
 		isUpdated      bool
 		newAnnotations map[string]string
 	}{
-		{
-			newObj(map[string]string{}),
-			"",
-			false,
-			map[string]string{},
-		},
-		{
-			newObj(nil),
-			"",
-			false,
-			nil,
-		},
 		{
 			newObj(map[string]string{}),
 			"someAnnotation",
@@ -268,15 +223,14 @@ func TestRemoveAnnotation(t *testing.T) {
 		},
 	}
 	for index, test := range testCases {
-		isUpdated, _ := RemoveAnnotation(test.obj, test.key)
+		isUpdated := RemoveAnnotation(test.obj, test.key)
 		assert.Equal(
 			t,
 			isUpdated,
 			test.isUpdated,
 			fmt.Sprintf("Test case %d failed. Expected isUpdated: %v, actual: %v", index, test.isUpdated, isUpdated),
 		)
-		accessor, _ := meta.Accessor(test.obj)
-		newAnnotations := accessor.GetAnnotations()
+		newAnnotations := test.obj.GetAnnotations()
 		assert.Equal(
 			t,
 			test.newAnnotations,

pkg/util/pendingcontrollers/pendingcontrollers.go

@@ -71,10 +71,7 @@ func SetPendingControllers(
 	if err != nil {
 		return false, fmt.Errorf("failed to marshal json: %w", err)
 	}
-	updated, err = annotationutil.AddAnnotation(fedObject, PendingControllersAnnotation, string(annotationValue))
-	if err != nil {
-		return updated, fmt.Errorf("failed to add annotation: %w", err)
-	}
+	updated = annotationutil.AddAnnotation(fedObject, PendingControllersAnnotation, string(annotationValue))
 	return updated, err
 }