address comments to fix bug (Azure#1080)

Author: britaniar

pkg/controllers/workapplier/availability_tracker.go

@@ -292,11 +292,11 @@ func trackServiceExportAvailability(curObj *unstructured.Unstructured) (Manifest
 		return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
 	}
 	// Validate annotation weight. Updating the annotation won't change the object generation,
-	// so the current status is not reliable and need to validate the annotation again here
+	// so the current status is not reliable and need to validate the annotation again here.
 	weight, err := objectmeta.ExtractWeightFromServiceExport(&svcExport)
 	if err != nil {
-		klog.Errorf(err.Error(), "ServiceExport has invalid weight", "serviceExport", svcExportObj)
-		return ManifestProcessingAvailabilityResultTypeNotYetAvailable, err
+		klog.Error(err, "ServiceExport has invalid weight", "serviceExport", svcExportObj)
+		return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
 	}
 	if weight != 0 {
 		// Check conflict condition for non-zero weight

pkg/controllers/workapplier/availability_tracker_test.go

@@ -18,7 +18,6 @@ package workapplier
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -1013,7 +1012,6 @@ func TestServiceExportAvailability(t *testing.T) {
 		weight                                       string
 		status                                       fleetnetworkingv1alpha1.ServiceExportStatus
 		wantManifestProcessingAvailabilityResultType ManifestProcessingAvailabilityResultType
-		err                                          error
 	}{
 		{
 			name:   "available svcExport (annotation weight is 0)",
@@ -1029,7 +1027,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (ServiceExportValid is false)",
@@ -1045,7 +1042,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (different generation, annotation weight is 0)",
@@ -1061,7 +1057,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "available svcExport with no conflict (annotation weight is 1)",
@@ -1083,7 +1078,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport with conflict (annotation weight is 1)",
@@ -1105,7 +1099,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable invalid svcExport (annotation weight is 1)",
@@ -1121,7 +1114,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (different generation, annotation weight is 1)",
@@ -1143,7 +1135,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (no annotation weight, no conflict condition)",
@@ -1159,7 +1150,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "available svcExport (no annotation weight)",
@@ -1181,7 +1171,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (no annotation weight with conflict)",
@@ -1203,7 +1192,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (no annotation weight, different generation)",
@@ -1225,7 +1213,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (no annotation weight, invalid service export)",
@@ -1241,7 +1228,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: nil,
 		},
 		{
 			name:   "unavailable svcExport (invalid weight)",
@@ -1257,7 +1243,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: fmt.Errorf("the weight annotation is not a valid integer: a"),
 		},
 		{
 			name:   "unavailable svcExport (out of range weight)",
@@ -1273,7 +1258,6 @@ func TestServiceExportAvailability(t *testing.T) {
 				},
 			},
 			wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
-			err: fmt.Errorf("the weight annotation is not in the range [0, 1000]: 1002"),
 		},
 	}
 
@@ -1285,14 +1269,8 @@ func TestServiceExportAvailability(t *testing.T) {
 			}
 			svcExport.Status = tc.status
 			gotResTyp, err := trackServiceExportAvailability(toUnstructured(t, svcExport))
-
-			// Check for errors
 			if err != nil {
-				if tc.err == nil || err.Error() != tc.err.Error() {
-					t.Fatalf("trackServiceExportAvailability() = %v, want %v", err, tc.err)
-				}
-			} else if tc.err != nil {
-				t.Fatalf("trackServiceExportAvailability() = nil, want error: %v", tc.err)
+				t.Fatalf("trackServiceExportAvailability() = %v, want no error", err)
 			}
 
 			// Check the result type