From f8d6be53209d53e9bb4ad47218dbb5104189203c Mon Sep 17 00:00:00 2001 From: whitewindmills Date: Mon, 8 Apr 2024 17:38:46 +0800 Subject: [PATCH] Deprecate key labels of rb/crb Signed-off-by: whitewindmills --- .../work/v1alpha1/well_known_constants.go | 9 -- .../work/v1alpha2/well_known_constants.go | 10 -- pkg/controllers/binding/binding_controller.go | 5 +- .../cluster_resource_binding_controller.go | 4 +- pkg/controllers/binding/common.go | 8 +- pkg/controllers/binding/common_test.go | 14 ++- .../dependencies_distributor.go | 4 - pkg/util/helper/binding.go | 20 +-- pkg/util/helper/binding_test.go | 117 ++++-------------- pkg/util/helper/work.go | 41 ++---- pkg/util/helper/workstatus.go | 6 +- 11 files changed, 56 insertions(+), 182 deletions(-) diff --git a/pkg/apis/work/v1alpha1/well_known_constants.go b/pkg/apis/work/v1alpha1/well_known_constants.go index b11daa5c0f81..d7b94628dbc4 100644 --- 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 index eda0610ad1b5..f2c4f8d1ad77 100644 --- 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 index 34caa09a3788..b4967e000f3d 100644 --- 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 index be06e321c06a..440e77601097 100644 --- 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 index 146fcf470326..e16ab2de352a 100644 --- 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 index 962c0268a052..a2df1110835d 100644 --- 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 index e533186da2f2..4e05f37ace5a 100644 --- 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 index 5e0d03a57098..4c324c8f267a 100644 --- 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 index 5cc1283f1e63..0b5ab243249b 100644 --- a/pkg/util/helper/binding_test.go +++ b/pkg/util/helper/binding_test.go @@ -410,6 +410,7 @@ func TestFindOrphanWorks(t *testing.T) { c client.Client bindingNamespace string bindingName string + bindingID string expectClusters sets.Set[string] } tests := []struct { @@ -428,17 +429,14 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: "wrong format", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ResourceBindingNamespaceAnnotationKey: "default", - workv1alpha2.ResourceBindingNameAnnotationKey: "binding", + workv1alpha2.ResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, ).Build(), bindingNamespace: "default", bindingName: "binding", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", expectClusters: sets.New("clusterx"), }, want: nil, @@ -454,11 +452,7 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "cluster1", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ResourceBindingNamespaceAnnotationKey: "default", - workv1alpha2.ResourceBindingNameAnnotationKey: "binding", + workv1alpha2.ResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, @@ -474,33 +468,20 @@ func TestFindOrphanWorks(t *testing.T) { }, }, }, - &workv1alpha1.Work{ - ObjectMeta: metav1.ObjectMeta{ - Name: "not-selected-because-of-annotation", - Namespace: names.ExecutionSpacePrefix + "cluster1", - ResourceVersion: "999", - Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "binding"), - }, - }, - }, &workv1alpha1.Work{ ObjectMeta: metav1.ObjectMeta{ Name: "not-selected-because-of-cluster", Namespace: names.ExecutionSpacePrefix + "clusterx", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ResourceBindingNamespaceAnnotationKey: "default", - workv1alpha2.ResourceBindingNameAnnotationKey: "binding", + workv1alpha2.ResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, ).Build(), bindingNamespace: "default", bindingName: "binding", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", expectClusters: sets.New("clusterx"), }, want: []workv1alpha1.Work{ @@ -510,11 +491,7 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "cluster1", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ResourceBindingNamespaceAnnotationKey: "default", - workv1alpha2.ResourceBindingNameAnnotationKey: "binding", + workv1alpha2.ResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, @@ -531,10 +508,7 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "cluster1", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ClusterResourceBindingAnnotationKey: "binding", + workv1alpha2.ClusterResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, @@ -544,19 +518,6 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "cluster1", ResourceVersion: "999", Labels: map[string]string{}, - Annotations: map[string]string{ - workv1alpha2.ClusterResourceBindingAnnotationKey: "binding", - }, - }, - }, - &workv1alpha1.Work{ - ObjectMeta: metav1.ObjectMeta{ - Name: "not-selected-because-of-annotation", - Namespace: names.ExecutionSpacePrefix + "cluster1", - ResourceVersion: "999", - Labels: map[string]string{ - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", "binding"), - }, }, }, &workv1alpha1.Work{ @@ -565,16 +526,14 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "clusterx", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ClusterResourceBindingAnnotationKey: "binding", + workv1alpha2.ClusterResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, ).Build(), bindingNamespace: "", bindingName: "binding", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", expectClusters: sets.New("clusterx"), }, want: []workv1alpha1.Work{ @@ -584,10 +543,7 @@ func TestFindOrphanWorks(t *testing.T) { Namespace: names.ExecutionSpacePrefix + "cluster1", ResourceVersion: "999", Labels: map[string]string{ - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", "binding"), - }, - Annotations: map[string]string{ - workv1alpha2.ClusterResourceBindingAnnotationKey: "binding", + workv1alpha2.ClusterResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, }, }, @@ -597,7 +553,7 @@ func TestFindOrphanWorks(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := FindOrphanWorks(tt.args.c, tt.args.bindingNamespace, tt.args.bindingName, tt.args.expectClusters) + got, err := FindOrphanWorks(tt.args.c, tt.args.bindingNamespace, tt.args.bindingName, tt.args.bindingID, tt.args.expectClusters) if (err != nil) != tt.wantErr { t.Errorf("FindOrphanWorks() error = %v, wantErr %v", err, tt.wantErr) return @@ -1020,6 +976,7 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { c client.Client namespace string name string + bindingID string } tests := []struct { name string @@ -1033,19 +990,20 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).Build(), namespace: "default", name: "foo", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, want: nil, wantErr: false, }, { - name: "delete", + name: "delete rb's work", args: args{ c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects( &workv1alpha1.Work{ ObjectMeta: metav1.ObjectMeta{ Name: "w1", Namespace: names.ExecutionSpacePrefix + "cluster", Labels: map[string]string{ - workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey("default", "foo"), + workv1alpha2.ResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, Annotations: map[string]string{ workv1alpha2.ResourceBindingNameAnnotationKey: "foo", @@ -1056,48 +1014,20 @@ func TestDeleteWorkByRBNamespaceAndName(t *testing.T) { ).Build(), namespace: "default", name: "foo", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, want: []workv1alpha1.Work{}, wantErr: false, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := DeleteWorkByRBNamespaceAndName(tt.args.c, tt.args.namespace, tt.args.name); (err != nil) != tt.wantErr { - t.Errorf("DeleteWorkByRBNamespaceAndName() error = %v, wantErr %v", err, tt.wantErr) - } - list := &workv1alpha1.WorkList{} - if err := tt.args.c.List(context.TODO(), list); err != nil { - t.Error(err) - return - } - if got := list.Items; !reflect.DeepEqual(got, tt.want) { - t.Errorf("DeleteWorkByRBNamespaceAndName() got = %v, want %v", got, tt.want) - } - }) - } -} - -func TestDeleteWorkByCRBName(t *testing.T) { - type args struct { - c client.Client - name string - } - tests := []struct { - name string - args args - want []workv1alpha1.Work - wantErr bool - }{ { - name: "delete", + name: "delete crb's work", args: args{ c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects( &workv1alpha1.Work{ ObjectMeta: metav1.ObjectMeta{ Name: "w1", Namespace: names.ExecutionSpacePrefix + "cluster", Labels: map[string]string{ - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", "foo"), + workv1alpha2.ClusterResourceBindingPermanentIDLabel: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, Annotations: map[string]string{ workv1alpha2.ClusterResourceBindingAnnotationKey: "foo", @@ -1105,7 +1035,8 @@ func TestDeleteWorkByCRBName(t *testing.T) { }, }, ).Build(), - name: "foo", + name: "foo", + bindingID: "3617252f-b1bb-43b0-98a1-c7de833c472c", }, want: []workv1alpha1.Work{}, wantErr: false, @@ -1113,8 +1044,8 @@ func TestDeleteWorkByCRBName(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := DeleteWorkByCRBName(tt.args.c, tt.args.name); (err != nil) != tt.wantErr { - t.Errorf("DeleteWorkByRBNamespaceAndName() error = %v, wantErr %v", err, tt.wantErr) + if err := DeleteWorks(tt.args.c, tt.args.namespace, tt.args.name, tt.args.bindingID); (err != nil) != tt.wantErr { + t.Errorf("DeleteWorks() error = %v, wantErr %v", err, tt.wantErr) } list := &workv1alpha1.WorkList{} if err := tt.args.c.List(context.TODO(), list); err != nil { @@ -1122,7 +1053,7 @@ func TestDeleteWorkByCRBName(t *testing.T) { return } if got := list.Items; !reflect.DeepEqual(got, tt.want) { - t.Errorf("DeleteWorkByRBNamespaceAndName() got = %v, want %v", got, tt.want) + t.Errorf("DeleteWorks() got = %v, want %v", got, tt.want) } }) } diff --git a/pkg/util/helper/work.go b/pkg/util/helper/work.go index 7266d2e31385..abcfe0233664 100644 --- a/pkg/util/helper/work.go +++ b/pkg/util/helper/work.go @@ -36,7 +36,6 @@ 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. @@ -85,10 +84,7 @@ func CreateOrUpdateWork(client client.Client, workMeta metav1.ObjectMeta, resour } return nil }) - if err != nil { - return err - } - return nil + return err }) if err != nil { klog.Errorf("Failed to create/update work %s/%s. Error: %v", work.GetNamespace(), work.GetName(), err) @@ -106,7 +102,7 @@ func CreateOrUpdateWork(client client.Client, workMeta metav1.ObjectMeta, resour return nil } -// GetWorksByLabelsSet get WorkList by matching labels.Set. +// GetWorksByLabelsSet gets WorkList by matching labels.Set. func GetWorksByLabelsSet(c client.Client, ls labels.Set) (*workv1alpha1.WorkList, error) { workList := &workv1alpha1.WorkList{} listOpt := &client.ListOptions{LabelSelector: labels.SelectorFromSet(ls)} @@ -114,37 +110,16 @@ 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) +// GetWorksByBindingID gets WorkList by matching same binding's permanent id. +func GetWorksByBindingID(c client.Client, bindingID string, namespaced bool) (*workv1alpha1.WorkList, error) { var ls labels.Set - if bindingNamespace != "" { - ls = labels.Set{workv1alpha2.ResourceBindingReferenceKey: referenceKey} + if namespaced { + ls = labels.Set{workv1alpha2.ResourceBindingPermanentIDLabel: bindingID} } 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]) - } - } + ls = labels.Set{workv1alpha2.ClusterResourceBindingPermanentIDLabel: bindingID} } - return retWorkList, nil + return GetWorksByLabelsSet(c, ls) } // GenEventRef returns the event reference. sets the UID(.spec.uid) that might be missing for fire events. diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 54d451de9a8d..ca40aef2cdd9 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -61,7 +61,7 @@ func AggregateResourceBindingWorkStatus( resourceTemplate *unstructured.Unstructured, eventRecorder record.EventRecorder, ) error { - workList, err := GetWorksByBindingNamespaceName(c, binding.Namespace, binding.Name) + workList, err := GetWorksByBindingID(c, binding.Labels[workv1alpha2.ResourceBindingPermanentIDLabel], true) if err != nil { return err } @@ -119,7 +119,7 @@ func AggregateClusterResourceBindingWorkStatus( resourceTemplate *unstructured.Unstructured, eventRecorder record.EventRecorder, ) error { - workList, err := GetWorksByBindingNamespaceName(c, "", binding.Name) + workList, err := GetWorksByBindingID(c, binding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], false) if err != nil { return err } @@ -249,7 +249,7 @@ func assembleWorkStatus(works []workv1alpha1.Work, workload *unstructured.Unstru return statuses, nil } -// GetManifestIndex get the index of clusterObj in manifest list, if not exist return -1. +// GetManifestIndex gets the index of clusterObj in manifest list, if not exist return -1. func GetManifestIndex(manifests []workv1alpha1.Manifest, clusterObj *unstructured.Unstructured) (int, error) { for index, rawManifest := range manifests { manifest := &unstructured.Unstructured{}