Merge pull request #4811 from whitewindmills/deprecated-rb_crb-labels

karmada-bot · web-flow · commit 4898c0fc55a1 · 2024-04-10T16:07:07.000+08:00
Deprecate key labels of rb/crb
diff --git a/pkg/apis/work/v1alpha1/well_known_constants.go b/pkg/apis/work/v1alpha1/well_known_constants.go
@@ -17,15 +17,6 @@ limitations under the License.
 package v1alpha1
 
 const (
-	// ResourceBindingNamespaceLabel is added to objects to specify associated ResourceBinding's namespace.
-	ResourceBindingNamespaceLabel = "resourcebinding.karmada.io/namespace"
-
-	// ResourceBindingNameLabel is added to objects to specify associated ResourceBinding's name.
-	ResourceBindingNameLabel = "resourcebinding.karmada.io/name"
-
-	// ClusterResourceBindingLabel is added to objects to specify associated ClusterResourceBinding.
-	ClusterResourceBindingLabel = "clusterresourcebinding.karmada.io/name"
-
 	// WorkNamespaceLabel is added to objects to specify associated Work's namespace.
 	WorkNamespaceLabel = "work.karmada.io/namespace"
 
diff --git a/pkg/apis/work/v1alpha2/well_known_constants.go b/pkg/apis/work/v1alpha2/well_known_constants.go
@@ -40,16 +40,6 @@ const (
 	// WorkNameAnnotation is added to objects to specify associated Work's name.
 	WorkNameAnnotation = "work.karmada.io/name"
 
-	// ResourceBindingReferenceKey is the key of ResourceBinding object.
-	// It is usually a unique hash value of ResourceBinding object's namespace and name, intended to be added to the Work object.
-	// It will be used to retrieve all Works objects that derived from a specific ResourceBinding object.
-	ResourceBindingReferenceKey = "resourcebinding.karmada.io/key"
-
-	// ClusterResourceBindingReferenceKey is the key of ClusterResourceBinding object.
-	// It is usually a unique hash value of ClusterResourceBinding object's namespace and name, intended to be added to the Work object.
-	// It will be used to retrieve all Works objects that derived by a specific ClusterResourceBinding object.
-	ClusterResourceBindingReferenceKey = "clusterresourcebinding.karmada.io/key"
-
 	// ResourceBindingNamespaceAnnotationKey is added to object to describe the associated ResourceBinding's namespace.
 	// It is added to:
 	// - Work object: describes the namespace of ResourceBinding which the Work derived from.
diff --git a/pkg/controllers/binding/binding_controller.go b/pkg/controllers/binding/binding_controller.go
@@ -82,7 +82,7 @@ func (c *ResourceBindingController) Reconcile(ctx context.Context, req controlle
 
 	if !binding.DeletionTimestamp.IsZero() {
 		klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String())
-		if err := helper.DeleteWorkByRBNamespaceAndName(c.Client, req.Namespace, req.Name); err != nil {
+		if err := helper.DeleteWorks(c.Client, req.Namespace, req.Name, binding.Labels[workv1alpha2.ResourceBindingPermanentIDLabel]); err != nil {
 			klog.Errorf("Failed to delete works related to %s/%s: %v", binding.GetNamespace(), binding.GetName(), err)
 			return controllerruntime.Result{}, err
 		}
@@ -143,7 +143,8 @@ func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBi
 }
 
 func (c *ResourceBindingController) removeOrphanWorks(binding *workv1alpha2.ResourceBinding) error {
-	works, err := helper.FindOrphanWorks(c.Client, binding.Namespace, binding.Name, helper.ObtainBindingSpecExistingClusters(binding.Spec))
+	works, err := helper.FindOrphanWorks(c.Client, binding.Namespace, binding.Name,
+		binding.Labels[workv1alpha2.ResourceBindingPermanentIDLabel], helper.ObtainBindingSpecExistingClusters(binding.Spec))
 	if err != nil {
 		klog.Errorf("Failed to find orphan works by resourceBinding(%s/%s). Error: %v.",
 			binding.GetNamespace(), binding.GetName(), err)
diff --git a/pkg/controllers/binding/cluster_resource_binding_controller.go b/pkg/controllers/binding/cluster_resource_binding_controller.go
@@ -82,7 +82,7 @@ func (c *ClusterResourceBindingController) Reconcile(ctx context.Context, req co
 
 	if !clusterResourceBinding.DeletionTimestamp.IsZero() {
 		klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String())
-		if err := helper.DeleteWorkByCRBName(c.Client, req.Name); err != nil {
+		if err := helper.DeleteWorks(c.Client, "", req.Name, clusterResourceBinding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel]); err != nil {
 			klog.Errorf("Failed to delete works related to %s: %v", clusterResourceBinding.GetName(), err)
 			return controllerruntime.Result{}, err
 		}
@@ -142,7 +142,7 @@ func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.Clu
 }
 
 func (c *ClusterResourceBindingController) removeOrphanWorks(binding *workv1alpha2.ClusterResourceBinding) error {
-	works, err := helper.FindOrphanWorks(c.Client, "", binding.Name, helper.ObtainBindingSpecExistingClusters(binding.Spec))
+	works, err := helper.FindOrphanWorks(c.Client, "", binding.Name, binding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], helper.ObtainBindingSpecExistingClusters(binding.Spec))
 	if err != nil {
 		klog.Errorf("Failed to find orphan works by ClusterResourceBinding(%s). Error: %v.", binding.GetName(), err)
 		c.EventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonCleanupWorkFailed, err.Error())
diff --git a/pkg/controllers/binding/common.go b/pkg/controllers/binding/common.go
@@ -157,17 +157,11 @@ func mergeLabel(workload *unstructured.Unstructured, binding metav1.Object, scop
 	util.MergeLabel(workload, util.ManagedByKarmadaLabel, util.ManagedByKarmadaLabelValue)
 	if scope == apiextensionsv1.NamespaceScoped {
 		bindingID := util.GetLabelValue(binding.GetLabels(), workv1alpha2.ResourceBindingPermanentIDLabel)
-		util.MergeLabel(workload, workv1alpha2.ResourceBindingReferenceKey, names.GenerateBindingReferenceKey(binding.GetNamespace(), binding.GetName()))
 		util.MergeLabel(workload, workv1alpha2.ResourceBindingPermanentIDLabel, bindingID)
-
-		workLabel[workv1alpha2.ResourceBindingReferenceKey] = names.GenerateBindingReferenceKey(binding.GetNamespace(), binding.GetName())
 		workLabel[workv1alpha2.ResourceBindingPermanentIDLabel] = bindingID
 	} else {
 		bindingID := util.GetLabelValue(binding.GetLabels(), workv1alpha2.ClusterResourceBindingPermanentIDLabel)
-		util.MergeLabel(workload, workv1alpha2.ClusterResourceBindingReferenceKey, names.GenerateBindingReferenceKey("", binding.GetName()))
 		util.MergeLabel(workload, workv1alpha2.ClusterResourceBindingPermanentIDLabel, bindingID)
-
-		workLabel[workv1alpha2.ClusterResourceBindingReferenceKey] = names.GenerateBindingReferenceKey("", binding.GetName())
 		workLabel[workv1alpha2.ClusterResourceBindingPermanentIDLabel] = bindingID
 	}
 	return workLabel
@@ -234,7 +228,7 @@ func mergeConflictResolution(workload *unstructured.Unstructured, conflictResolu
 		return annotations
 	} else if conflictResolutionInRT != "" {
 		// ignore its value and add logs if conflictResolutionInRT is neither abort nor overwrite.
-		klog.Warningf("ignore the invalid conflict-resolution annotation in ResourceTemplate %s/%s/%s: %s",
+		klog.Warningf("Ignore the invalid conflict-resolution annotation in ResourceTemplate %s/%s/%s: %s",
 			workload.GetKind(), workload.GetNamespace(), workload.GetName(), conflictResolutionInRT)
 	}
 
diff --git a/pkg/controllers/binding/common_test.go b/pkg/controllers/binding/common_test.go
@@ -26,7 +26,6 @@ import (
 
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
-	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
 func Test_mergeTargetClusters(t *testing.T) {
@@ -136,7 +135,6 @@ func Test_mergeLabel(t *testing.T) {
 			scope: v1.NamespaceScoped,
 			want: map[string]string{
 				workv1alpha2.ResourceBindingPermanentIDLabel: rbID,
-				workv1alpha2.ResourceBindingReferenceKey:     names.GenerateBindingReferenceKey(namespace, bindingName),
 			},
 		},
 		{
@@ -161,13 +159,21 @@ func Test_mergeLabel(t *testing.T) {
 			scope: v1.ClusterScoped,
 			want: map[string]string{
 				workv1alpha2.ClusterResourceBindingPermanentIDLabel: rbID,
-				workv1alpha2.ClusterResourceBindingReferenceKey:     names.GenerateBindingReferenceKey("", bindingName),
 			},
 		},
 	}
+
+	checker := func(got, want map[string]string) bool {
+		for key, val := range want {
+			if got[key] != val {
+				return false
+			}
+		}
+		return true
+	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := mergeLabel(tt.workload, tt.binding, tt.scope); !reflect.DeepEqual(got, tt.want) {
+			if got := mergeLabel(tt.workload, tt.binding, tt.scope); !checker(got, tt.want) {
 				t.Errorf("mergeLabel() = %v, want %v", got, tt.want)
 			}
 		})
diff --git a/pkg/dependenciesdistributor/dependencies_distributor.go b/pkg/dependenciesdistributor/dependencies_distributor.go
@@ -522,10 +522,6 @@ func (d *DependenciesDistributor) createOrUpdateAttachedBinding(attachedBinding
 	key := client.ObjectKeyFromObject(attachedBinding)
 	err := d.Client.Get(context.TODO(), key, existBinding)
 	if err == nil {
-		if util.GetLabelValue(existBinding.Labels, workv1alpha2.ResourceBindingPermanentIDLabel) == "" {
-			existBinding.Labels = util.DedupeAndMergeLabels(existBinding.Labels,
-				map[string]string{workv1alpha2.ResourceBindingPermanentIDLabel: uuid.New().String()})
-		}
 		existBinding.Spec.RequiredBy = mergeBindingSnapshot(existBinding.Spec.RequiredBy, attachedBinding.Spec.RequiredBy)
 		existBinding.Labels = util.DedupeAndMergeLabels(existBinding.Labels, attachedBinding.Labels)
 		existBinding.Spec.Resource = attachedBinding.Spec.Resource
diff --git a/pkg/util/helper/binding.go b/pkg/util/helper/binding.go
@@ -194,9 +194,9 @@ func ObtainBindingSpecExistingClusters(bindingSpec workv1alpha2.ResourceBindingS
 
 // FindOrphanWorks retrieves all works that labeled with current binding(ResourceBinding or ClusterResourceBinding) objects,
 // then pick the works that not meet current binding declaration.
-func FindOrphanWorks(c client.Client, bindingNamespace, bindingName string, expectClusters sets.Set[string]) ([]workv1alpha1.Work, error) {
+func FindOrphanWorks(c client.Client, bindingNamespace, bindingName, bindingID string, expectClusters sets.Set[string]) ([]workv1alpha1.Work, error) {
 	var needJudgeWorks []workv1alpha1.Work
-	workList, err := GetWorksByBindingNamespaceName(c, bindingNamespace, bindingName)
+	workList, err := GetWorksByBindingID(c, bindingID, bindingNamespace != "")
 	if err != nil {
 		klog.Errorf("Failed to get works by binding object (%s/%s): %v", bindingNamespace, bindingName, err)
 		return nil, err
@@ -344,21 +344,11 @@ func GetResourceBindings(c client.Client, ls labels.Set) (*workv1alpha2.Resource
 	return bindings, c.List(context.TODO(), bindings, listOpt)
 }
 
-// DeleteWorkByRBNamespaceAndName will delete all Work objects by ResourceBinding namespace and name.
-func DeleteWorkByRBNamespaceAndName(c client.Client, namespace, name string) error {
-	return DeleteWorks(c, namespace, name)
-}
-
-// DeleteWorkByCRBName will delete all Work objects by ClusterResourceBinding name.
-func DeleteWorkByCRBName(c client.Client, name string) error {
-	return DeleteWorks(c, "", name)
-}
-
 // DeleteWorks will delete all Work objects by labels.
-func DeleteWorks(c client.Client, namespace, name string) error {
-	workList, err := GetWorksByBindingNamespaceName(c, namespace, name)
+func DeleteWorks(c client.Client, namespace, name, bindingID string) error {
+	workList, err := GetWorksByBindingID(c, bindingID, namespace != "")
 	if err != nil {
-		klog.Errorf("Failed to get works by ResourceBinding(%s/%s) : %v", namespace, name, err)
+		klog.Errorf("Failed to get works by (Cluster)ResourceBinding(%s/%s) : %v", namespace, name, err)
 		return err
 	}
 
diff --git a/pkg/util/helper/binding_test.go b/pkg/util/helper/binding_test.go
diff --git a/pkg/util/helper/work.go b/pkg/util/helper/work.go
diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go
