address comments.

Signed-off-by: morvencao <[email protected]>
open-cluster-management-io · Nov 23, 2022 · 5dbc6af · 5dbc6af
1 parent 54a22eb
commit 5dbc6af
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 32 deletions.
diff --git a/pkg/hub/addon/discovery_controller.go b/pkg/hub/addon/discovery_controller.go
@@ -137,18 +137,16 @@ func (c *addOnFeatureDiscoveryController) syncAddOn(ctx context.Context, cluster
 	}
 
 	addOn, err := c.addOnLister.ManagedClusterAddOns(clusterName).Get(addOnName)
+	key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOnName)
 	switch {
 	case errors.IsNotFound(err):
 		// addon is deleted
-		key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOnName)
 		delete(labels, key)
 	case err != nil:
 		return err
 	case !addOn.DeletionTimestamp.IsZero():
-		key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOnName)
 		delete(labels, key)
 	default:
-		key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOn.Name)
 		labels[key] = getAddOnLabelValue(addOn)
 	}
 
@@ -164,8 +162,10 @@ func (c *addOnFeatureDiscoveryController) syncAddOn(ctx context.Context, cluster
 	// build cluster labels patch
 	patchBytes, err := json.Marshal(map[string]interface{}{
 		"metadata": map[string]interface{}{
-			"labels": labels,
-		},
+			"labels":          labels,
+			"uid":             cluster.UID,
+			"resourceVersion": cluster.ResourceVersion,
+		}, // to ensure they appear in the patch as preconditions
 	})
 	if err != nil {
 		return fmt.Errorf("failed to create patch for cluster %s: %w", cluster.Name, err)
@@ -247,8 +247,10 @@ func (c *addOnFeatureDiscoveryController) syncCluster(ctx context.Context, clust
 	// build cluster labels patch
 	patchBytes, err := json.Marshal(map[string]interface{}{
 		"metadata": map[string]interface{}{
-			"labels": addOnLabels,
-		},
+			"labels":          addOnLabels,
+			"uid":             cluster.UID,
+			"resourceVersion": cluster.ResourceVersion,
+		}, // to ensure they appear in the patch as preconditions
 	})
 	if err != nil {
 		return fmt.Errorf("failed to create patch for cluster %s: %w", cluster.Name, err)

diff --git a/pkg/hub/addon/discovery_controller_test.go b/pkg/hub/addon/discovery_controller_test.go
@@ -387,44 +387,27 @@ func TestDiscoveryController_Sync(t *testing.T) {
 }
 
 func assertPatchAddonLabel(t *testing.T, actionPatch clienttesting.PatchActionImpl, addOnName, addOnStatus string) {
-	var patchObj map[string]map[string]map[string]string
-	if err := json.Unmarshal(actionPatch.Patch, &patchObj); err != nil {
-		t.Errorf("failed to unmarshal patch %s: %v", patchObj, err)
+	var patchClusterObj clusterv1.ManagedCluster
+	if err := json.Unmarshal(actionPatch.Patch, &patchClusterObj); err != nil {
+		t.Errorf("failed to unmarshal patch %s: %v", actionPatch.Patch, err)
 	}
-	metadata, ok := patchObj["metadata"]
-	if !ok {
-		t.Errorf("patch %s doesn't contain metadata field", patchObj)
-	}
-	labels, ok := metadata["labels"]
-	if !ok {
-		t.Errorf("patch %s doesn't contain metadata.labels field", patchObj)
-	}
-
+	labels := patchClusterObj.Labels
 	key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOnName)
 	value, ok := labels[key]
 	if !ok {
 		t.Errorf("label %q not found", key)
 	}
-
 	if value != addOnStatus {
 		t.Errorf("expect label value %q but found %q", addOnStatus, value)
 	}
 }
 
 func assertPatchNoAddonLabel(t *testing.T, actionPatch clienttesting.PatchActionImpl, addOnName string) {
-	var patchObj map[string]map[string]map[string]string
-	if err := json.Unmarshal(actionPatch.Patch, &patchObj); err != nil {
-		t.Errorf("failed to unmarshal patch %s: %v", patchObj, err)
+	var patchClusterObj clusterv1.ManagedCluster
+	if err := json.Unmarshal(actionPatch.Patch, &patchClusterObj); err != nil {
+		t.Errorf("failed to unmarshal patch %s: %v", actionPatch.Patch, err)
 	}
-	metadata, ok := patchObj["metadata"]
-	if !ok {
-		t.Errorf("patch %s doesn't contain metadata field", patchObj)
-	}
-	labels, ok := metadata["labels"]
-	if !ok {
-		t.Errorf("patch %s doesn't contain metadata.labels field", patchObj)
-	}
-
+	labels := patchClusterObj.Labels
 	key := fmt.Sprintf("%s%s", addOnFeaturePrefix, addOnName)
 	if _, ok := labels[key]; ok {
 		t.Errorf("label %q found", key)

diff --git a/pkg/hub/taint/controller.go b/pkg/hub/taint/controller.go
@@ -96,6 +96,10 @@ func (c *taintController) sync(ctx context.Context, syncCtx factory.SyncContext)
 		}
 		// build cluster taints patch
 		patchBytes, err := json.Marshal(map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"uid":             managedCluster.UID,
+				"resourceVersion": managedCluster.ResourceVersion,
+			}, // to ensure they appear in the patch as preconditions
 			"spec": map[string]interface{}{
 				"taints": newTaints,
 			},