Merge pull request #2086 from chaunceyjiang/issue_2071

fix binding reference key hash collision

pkg/util/helper/binding.go

@@ -89,18 +89,14 @@ func GetBindingClusterNames(targetClusters []workv1alpha2.TargetCluster, binding
 func FindOrphanWorks(c client.Client, bindingNamespace, bindingName string, clusterNames []string, scope apiextensionsv1.ResourceScope) ([]workv1alpha1.Work, error) {
 	var needJudgeWorks []workv1alpha1.Work
 	if scope == apiextensionsv1.NamespaceScoped {
-		workList, err := GetWorksByLabelsSet(c, labels.Set{
-			workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(bindingNamespace, bindingName),
-		})
+		workList, err := GetWorksByBindingNamespaceName(c, bindingNamespace, bindingName)
 		if err != nil {
 			klog.Errorf("Failed to get works by ResourceBinding(%s/%s): %v", bindingNamespace, bindingName, err)
 			return nil, err
 		}
 		needJudgeWorks = append(needJudgeWorks, workList.Items...)
 	} else {
-		workList, err := GetWorksByLabelsSet(c, labels.Set{
-			workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", bindingName),
-		})
+		workList, err := GetWorksByBindingNamespaceName(c, "", bindingName)
 		if err != nil {
 			klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", bindingName, err)
 			return nil, err
@@ -201,23 +197,19 @@ func GetResourceBindings(c client.Client, ls labels.Set) (*workv1alpha2.Resource
 
 // DeleteWorkByRBNamespaceAndName will delete all Work objects by ResourceBinding namespace and name.
 func DeleteWorkByRBNamespaceAndName(c client.Client, namespace, name string) error {
-	return DeleteWorks(c, labels.Set{
-		workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(namespace, name),
-	})
+	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, labels.Set{
-		workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", name),
-	})
+	return DeleteWorks(c, "", name)
 }
 
 // DeleteWorks will delete all Work objects by labels.
-func DeleteWorks(c client.Client, selector labels.Set) error {
-	workList, err := GetWorksByLabelsSet(c, selector)
+func DeleteWorks(c client.Client, namespace, name string) error {
+	workList, err := GetWorksByBindingNamespaceName(c, namespace, name)
 	if err != nil {
-		klog.Errorf("Failed to get works by label %v: %v", selector, err)
+		klog.Errorf("Failed to get works by ResourceBinding(%s/%s) : %v", namespace, name, err)
 		return err
 	}
 

pkg/util/helper/work.go

@@ -18,6 +18,7 @@ import (
 	workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
 	"github.com/karmada-io/karmada/pkg/util"
+	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
 // CreateOrUpdateWork creates a Work object if not exist, or updates if it already exist.
@@ -83,6 +84,39 @@ func GetWorksByLabelsSet(c client.Client, ls labels.Set) (*workv1alpha1.WorkList
 	return workList, c.List(context.TODO(), workList, listOpt)
 }
 
+// GetWorksByBindingNamespaceName get WorkList by matching same Namespace and same Name.
+func GetWorksByBindingNamespaceName(c client.Client, bindingNamespace, bindingName string) (*workv1alpha1.WorkList, error) {
+	referenceKey := names.GenerateBindingReferenceKey(bindingNamespace, bindingName)
+	var ls labels.Set
+	if bindingNamespace != "" {
+		ls = labels.Set{workv1alpha2.ResourceBindingReferenceKey: referenceKey}
+	} else {
+		ls = labels.Set{workv1alpha2.ClusterResourceBindingReferenceKey: referenceKey}
+	}
+
+	workList, err := GetWorksByLabelsSet(c, ls)
+	if err != nil {
+		return nil, err
+	}
+	retWorkList := &workv1alpha1.WorkList{}
+	// Due to the hash collision problem, we have to filter the Works by annotation.
+	// More details please refer to https://github.com/karmada-io/karmada/issues/2071.
+	for i := range workList.Items {
+		if len(bindingNamespace) > 0 { // filter Works that derived by 'ResourceBinding'
+			if util.GetAnnotationValue(workList.Items[i].GetAnnotations(), workv1alpha2.ResourceBindingNameAnnotationKey) == bindingName &&
+				util.GetAnnotationValue(workList.Items[i].GetAnnotations(), workv1alpha2.ResourceBindingNamespaceAnnotationKey) == bindingNamespace {
+				retWorkList.Items = append(retWorkList.Items, workList.Items[i])
+			}
+		} else { // filter Works that derived by 'ClusterResourceBinding'
+			if util.GetAnnotationValue(workList.Items[i].GetAnnotations(), workv1alpha2.ClusterResourceBindingAnnotationKey) == bindingName {
+				retWorkList.Items = append(retWorkList.Items, workList.Items[i])
+			}
+		}
+	}
+
+	return retWorkList, nil
+}
+
 // GenEventRef returns the event reference. sets the UID(.spec.uid) that might be missing for fire events.
 // Do nothing if the UID already exist, otherwise set the UID from annotation.
 func GenEventRef(resource *unstructured.Unstructured) (*corev1.ObjectReference, error) {

pkg/util/helper/workstatus.go

@@ -10,7 +10,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -38,9 +37,7 @@ const (
 // AggregateResourceBindingWorkStatus will collect all work statuses with current ResourceBinding objects,
 // then aggregate status info to current ResourceBinding status.
 func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.ResourceBinding, workload *unstructured.Unstructured) error {
-	workList, err := GetWorksByLabelsSet(c, labels.Set{
-		workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(binding.Namespace, binding.Name),
-	})
+	workList, err := GetWorksByBindingNamespaceName(c, binding.Namespace, binding.Name)
 	if err != nil {
 		klog.Errorf("Failed to get works by ResourceBinding(%s/%s): %v", binding.Namespace, binding.Name, err)
 		return err
@@ -83,9 +80,7 @@ func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.R
 // AggregateClusterResourceBindingWorkStatus will collect all work statuses with current ClusterResourceBinding objects,
 // then aggregate status info to current ClusterResourceBinding status.
 func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.ClusterResourceBinding, workload *unstructured.Unstructured) error {
-	workList, err := GetWorksByLabelsSet(c, labels.Set{
-		workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", binding.Name),
-	})
+	workList, err := GetWorksByBindingNamespaceName(c, "", binding.Name)
 	if err != nil {
 		klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", binding.Name, err)
 		return err