From b562e7f4a4d446f29e05037a945ba94e29d2e547 Mon Sep 17 00:00:00 2001 From: whitewindmills Date: Mon, 25 Mar 2024 14:06:14 +0800 Subject: [PATCH] Deprecate name/namespace/key labels Co-authored-by: changzhen Signed-off-by: whitewindmills --- .../policy/v1alpha1/well_known_constants.go | 31 +-- .../work/v1alpha1/well_known_constants.go | 34 --- .../work/v1alpha2/well_known_constants.go | 16 -- pkg/controllers/binding/binding_controller.go | 5 +- .../cluster_resource_binding_controller.go | 4 +- pkg/controllers/binding/common.go | 11 +- pkg/controllers/binding/common_test.go | 17 +- .../execution/execution_controller.go | 3 +- .../federatedhpa/federatedhpa_controller.go | 20 +- ...ederated_resource_quota_sync_controller.go | 4 +- .../hpa_scale_target_marker_predicate.go | 4 +- .../mcs/endpointslice_controller.go | 26 +- .../endpointslice_dispatch_controller.go | 12 +- .../multiclusterservice/mcs_controller.go | 3 - .../namespace/namespace_sync_controller.go | 6 +- .../status/work_status_controller.go | 7 +- .../status/work_status_controller_test.go | 43 +-- .../unifiedauth/unified_auth_controller.go | 6 +- pkg/detector/detector.go | 255 ++++++++---------- pkg/detector/handler.go | 32 +++ pkg/detector/policy.go | 107 ++++---- pkg/detector/preemption.go | 12 +- pkg/karmadactl/promote/promote.go | 4 +- pkg/scheduler/event_handler.go | 4 +- pkg/util/annotation.go | 14 + pkg/util/helper/binding.go | 20 +- pkg/util/helper/binding_test.go | 117 ++------ pkg/util/helper/work.go | 34 +-- pkg/util/helper/workstatus.go | 4 +- pkg/util/label.go | 3 +- pkg/util/objectwatcher/objectwatcher.go | 3 +- .../clusterpropagationpolicy/mutating.go | 8 + pkg/webhook/propagationpolicy/mutating.go | 8 + pkg/webhook/work/mutating.go | 3 + test/e2e/clusterpropagationpolicy_test.go | 44 ++- test/e2e/propagationpolicy_test.go | 51 ++-- 36 files changed, 426 insertions(+), 549 deletions(-) delete mode 100644 pkg/apis/work/v1alpha1/well_known_constants.go diff --git a/pkg/apis/policy/v1alpha1/well_known_constants.go b/pkg/apis/policy/v1alpha1/well_known_constants.go index 940e69375a39..f54fc8165fb5 100644 --- a/pkg/apis/policy/v1alpha1/well_known_constants.go +++ b/pkg/apis/policy/v1alpha1/well_known_constants.go @@ -16,6 +16,7 @@ limitations under the License. package v1alpha1 +// The well-known label key constant. const ( // PropagationPolicyPermanentIDLabel is the identifier of a PropagationPolicy object. // Karmada generates a unique identifier, such as metadata.UUID, for each PropagationPolicy object. @@ -34,27 +35,9 @@ const ( // PropagationPolicyUIDLabel is the uid of PropagationPolicy object. PropagationPolicyUIDLabel = "propagationpolicy.karmada.io/uid" - // PropagationPolicyNamespaceAnnotation is added to objects to specify associated PropagationPolicy namespace. - PropagationPolicyNamespaceAnnotation = "propagationpolicy.karmada.io/namespace" - - // PropagationPolicyNameAnnotation is added to objects to specify associated PropagationPolicy name. - PropagationPolicyNameAnnotation = "propagationpolicy.karmada.io/name" - // ClusterPropagationPolicyUIDLabel is the uid of ClusterPropagationPolicy object. ClusterPropagationPolicyUIDLabel = "clusterpropagationpolicy.karmada.io/uid" - // ClusterPropagationPolicyAnnotation is added to objects to specify associated ClusterPropagationPolicy name. - ClusterPropagationPolicyAnnotation = "clusterpropagationpolicy.karmada.io/name" - - // PropagationPolicyNamespaceLabel is added to objects to specify associated PropagationPolicy namespace. - PropagationPolicyNamespaceLabel = "propagationpolicy.karmada.io/namespace" - - // PropagationPolicyNameLabel is added to objects to specify associated PropagationPolicy's name. - PropagationPolicyNameLabel = "propagationpolicy.karmada.io/name" - - // ClusterPropagationPolicyLabel is added to objects to specify associated ClusterPropagationPolicy. - ClusterPropagationPolicyLabel = "clusterpropagationpolicy.karmada.io/name" - // NamespaceSkipAutoPropagationLabel is added to namespace objects to indicate if // the namespace should be skipped from propagating by the namespace controller. // For example, a namespace with the following label will be skipped: @@ -65,3 +48,15 @@ const ( // synced to new member clusters, but old member clusters still have it. NamespaceSkipAutoPropagationLabel = "namespace.karmada.io/skip-auto-propagation" ) + +// The well-known annotation key constant. +const ( + // PropagationPolicyNamespaceAnnotation is added to objects to specify associated PropagationPolicy namespace. + PropagationPolicyNamespaceAnnotation = "propagationpolicy.karmada.io/namespace" + + // PropagationPolicyNameAnnotation is added to objects to specify associated PropagationPolicy name. + PropagationPolicyNameAnnotation = "propagationpolicy.karmada.io/name" + + // ClusterPropagationPolicyAnnotation is added to objects to specify associated ClusterPropagationPolicy name. + ClusterPropagationPolicyAnnotation = "clusterpropagationpolicy.karmada.io/name" +) diff --git a/pkg/apis/work/v1alpha1/well_known_constants.go b/pkg/apis/work/v1alpha1/well_known_constants.go deleted file mode 100644 index b11daa5c0f81..000000000000 --- a/pkg/apis/work/v1alpha1/well_known_constants.go +++ /dev/null @@ -1,34 +0,0 @@ -/* -Copyright 2021 The Karmada Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -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" - - // WorkNameLabel is added to objects to specify associated Work's name. - WorkNameLabel = "work.karmada.io/name" -) diff --git a/pkg/apis/work/v1alpha2/well_known_constants.go b/pkg/apis/work/v1alpha2/well_known_constants.go index c09e14004a31..4c5fe9824e83 100644 --- a/pkg/apis/work/v1alpha2/well_known_constants.go +++ b/pkg/apis/work/v1alpha2/well_known_constants.go @@ -49,16 +49,6 @@ const ( // WorkUIDLabel is the uid of Work object. WorkUIDLabel = "work.karmada.io/uid" - // 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. @@ -77,12 +67,6 @@ const ( // - Manifest in Work object: describes the name of ClusterResourceBinding which the manifest derived from. ClusterResourceBindingAnnotationKey = "clusterresourcebinding.karmada.io/name" - // WorkNamespaceLabel is added to objects to specify associated Work's namespace. - WorkNamespaceLabel = "work.karmada.io/namespace" - - // WorkNameLabel is added to objects to specify associated Work's name. - WorkNameLabel = "work.karmada.io/name" - // BindingManagedByLabel is added to ResourceBinding to represent what kind of resource manages this Binding. BindingManagedByLabel = "binding.karmada.io/managed-by" ) diff --git a/pkg/controllers/binding/binding_controller.go b/pkg/controllers/binding/binding_controller.go index 4a641aa98fc4..474afc169965 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{Requeue: true}, 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 4e33ab0fa93e..c48c98041710 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{Requeue: true}, 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 a6bc134e1893..fdb2b11767e1 100644 --- a/pkg/controllers/binding/common.go +++ b/pkg/controllers/binding/common.go @@ -25,7 +25,6 @@ import ( configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" - 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/resourceinterpreter" "github.com/karmada-io/karmada/pkg/util" @@ -109,7 +108,7 @@ func ensureWork( klog.Errorf("Failed to apply overrides for %s/%s/%s, err is: %v", clonedWorkload.GetKind(), clonedWorkload.GetNamespace(), clonedWorkload.GetName(), err) return err } - workLabel := mergeLabel(clonedWorkload, workNamespace, binding, scope) + workLabel := mergeLabel(clonedWorkload, binding, scope) annotations := mergeAnnotations(clonedWorkload, workNamespace, binding, scope) annotations = mergeConflictResolution(clonedWorkload, conflictResolutionInBinding, annotations) @@ -153,28 +152,22 @@ func mergeTargetClusters(targetClusters []workv1alpha2.TargetCluster, requiredBy return targetClusters } -func mergeLabel(workload *unstructured.Unstructured, workNamespace string, binding metav1.Object, scope apiextensionsv1.ResourceScope) map[string]string { +func mergeLabel(workload *unstructured.Unstructured, binding metav1.Object, scope apiextensionsv1.ResourceScope) map[string]string { var workLabel = make(map[string]string) - util.MergeLabel(workload, workv1alpha1.WorkNamespaceLabel, workNamespace) - util.MergeLabel(workload, workv1alpha1.WorkNameLabel, names.GenerateWorkName(workload.GetKind(), workload.GetName(), workload.GetNamespace())) util.MergeLabel(workload, util.ManagedByKarmadaLabel, util.ManagedByKarmadaLabelValue) if scope == apiextensionsv1.NamespaceScoped { util.RemoveLabels(workload, workv1alpha2.ResourceBindingUIDLabel) 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 { util.RemoveLabels(workload, workv1alpha2.ClusterResourceBindingUIDLabel) 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 diff --git a/pkg/controllers/binding/common_test.go b/pkg/controllers/binding/common_test.go index 259f00671c33..5ba214520576 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) { @@ -106,12 +105,11 @@ func Test_mergeLabel(t *testing.T) { rbID := "93162d3c-ee8e-4995-9034-05f4d5d2c2b9" tests := []struct { - name string - workload *unstructured.Unstructured - workNamespace string - binding metav1.Object - scope v1.ResourceScope - want map[string]string + name string + workload *unstructured.Unstructured + binding metav1.Object + scope v1.ResourceScope + want map[string]string }{ { name: "NamespaceScoped", @@ -125,7 +123,6 @@ func Test_mergeLabel(t *testing.T) { }, }, }, - workNamespace: namespace, binding: &workv1alpha2.ClusterResourceBinding{ ObjectMeta: metav1.ObjectMeta{ Name: bindingName, @@ -138,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), }, }, { @@ -163,13 +159,12 @@ func Test_mergeLabel(t *testing.T) { scope: v1.ClusterScoped, want: map[string]string{ workv1alpha2.ClusterResourceBindingPermanentIDLabel: rbID, - workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", bindingName), }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := mergeLabel(tt.workload, tt.workNamespace, tt.binding, tt.scope); !reflect.DeepEqual(got, tt.want) { + if got := mergeLabel(tt.workload, tt.binding, tt.scope); !reflect.DeepEqual(got, tt.want) { t.Errorf("mergeLabel() = %v, want %v", got, tt.want) } }) diff --git a/pkg/controllers/execution/execution_controller.go b/pkg/controllers/execution/execution_controller.go index 253172dccda7..829d7eee2b38 100644 --- a/pkg/controllers/execution/execution_controller.go +++ b/pkg/controllers/execution/execution_controller.go @@ -170,7 +170,7 @@ func (c *Controller) tryDeleteWorkload(clusterName string, work *workv1alpha1.Wo } // Avoid deleting resources that not managed by karmada. - if util.GetLabelValue(clusterObj.GetLabels(), workv1alpha1.WorkNameLabel) != util.GetLabelValue(workload.GetLabels(), workv1alpha1.WorkNameLabel) { + if util.GetLabelValue(clusterObj.GetLabels(), workv1alpha2.WorkPermanentIDLabel) != util.GetLabelValue(workload.GetLabels(), workv1alpha2.WorkPermanentIDLabel) { klog.Infof("Abort deleting the resource(kind=%s, %s/%s) exists in cluster %v but not managed by karmada", clusterObj.GetKind(), clusterObj.GetNamespace(), clusterObj.GetName(), clusterName) return nil } @@ -211,7 +211,6 @@ func (c *Controller) syncToClusters(clusterName string, work *workv1alpha1.Work) errs = append(errs, err) continue } - util.MergeLabel(workload, workv1alpha2.WorkPermanentIDLabel, util.GetLabelValue(work.Labels, workv1alpha2.WorkPermanentIDLabel)) if err = c.tryCreateOrUpdateWorkload(clusterName, workload); err != nil { klog.Errorf("Failed to create or update resource(%v/%v) in the given member cluster %s, err is %v", workload.GetNamespace(), workload.GetName(), clusterName, err) diff --git a/pkg/controllers/federatedhpa/federatedhpa_controller.go b/pkg/controllers/federatedhpa/federatedhpa_controller.go index 3bd24e6f773d..249b59a4fce2 100644 --- a/pkg/controllers/federatedhpa/federatedhpa_controller.go +++ b/pkg/controllers/federatedhpa/federatedhpa_controller.go @@ -404,25 +404,23 @@ func (c *FHPAController) reconcileAutoscaler(ctx context.Context, hpa *autoscali func (c *FHPAController) getBindingByLabel(resourceLabel map[string]string, resourceRef autoscalingv2.CrossVersionObjectReference) (*workv1alpha2.ResourceBinding, error) { if len(resourceLabel) == 0 { - return nil, fmt.Errorf("Target resource has no label. ") + return nil, errors.New("target resource has no policy permanent-id label") } - var policyName, policyNameSpace string + var policyID string var selector labels.Selector - if _, ok := resourceLabel[policyv1alpha1.PropagationPolicyNameLabel]; ok { - policyName = resourceLabel[policyv1alpha1.PropagationPolicyNameLabel] - policyNameSpace = resourceLabel[policyv1alpha1.PropagationPolicyNamespaceLabel] + if _, ok := resourceLabel[policyv1alpha1.PropagationPolicyPermanentIDLabel]; ok { + policyID = resourceLabel[policyv1alpha1.PropagationPolicyPermanentIDLabel] selector = labels.SelectorFromSet(labels.Set{ - policyv1alpha1.PropagationPolicyNameLabel: policyName, - policyv1alpha1.PropagationPolicyNamespaceLabel: policyNameSpace, + policyv1alpha1.PropagationPolicyPermanentIDLabel: policyID, }) - } else if _, ok = resourceLabel[policyv1alpha1.ClusterPropagationPolicyLabel]; ok { - policyName = resourceLabel[policyv1alpha1.ClusterPropagationPolicyLabel] + } else if _, ok = resourceLabel[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel]; ok { + policyID = resourceLabel[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] selector = labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policyName, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, }) } else { - return nil, fmt.Errorf("No label of policy found. ") + return nil, errors.New("no label of policy permanent-id found") } binding := &workv1alpha2.ResourceBinding{} diff --git a/pkg/controllers/federatedresourcequota/federated_resource_quota_sync_controller.go b/pkg/controllers/federatedresourcequota/federated_resource_quota_sync_controller.go index e5f2a8f16b45..50cbd3badfe0 100644 --- a/pkg/controllers/federatedresourcequota/federated_resource_quota_sync_controller.go +++ b/pkg/controllers/federatedresourcequota/federated_resource_quota_sync_controller.go @@ -168,9 +168,7 @@ func (c *SyncController) buildWorks(quota *policyv1alpha1.FederatedResourceQuota resourceQuota.Namespace = quota.Namespace resourceQuota.Name = quota.Name resourceQuota.Labels = map[string]string{ - workv1alpha1.WorkNamespaceLabel: workNamespace, - workv1alpha1.WorkNameLabel: workName, - util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, + util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, } resourceQuota.Spec.Hard = extractClusterHardResourceList(quota.Spec, cluster.Name) diff --git a/pkg/controllers/hpascaletargetmarker/hpa_scale_target_marker_predicate.go b/pkg/controllers/hpascaletargetmarker/hpa_scale_target_marker_predicate.go index 6b0f4ea602c4..45d1af2e9cd3 100644 --- a/pkg/controllers/hpascaletargetmarker/hpa_scale_target_marker_predicate.go +++ b/pkg/controllers/hpascaletargetmarker/hpa_scale_target_marker_predicate.go @@ -91,7 +91,7 @@ func (r *HpaScaleTargetMarker) Generic(_ event.GenericEvent) bool { } func hasBeenPropagated(hpa *autoscalingv2.HorizontalPodAutoscaler) bool { - _, ppExist := hpa.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] - _, cppExist := hpa.GetLabels()[policyv1alpha1.ClusterPropagationPolicyLabel] + _, ppExist := hpa.GetLabels()[policyv1alpha1.PropagationPolicyPermanentIDLabel] + _, cppExist := hpa.GetLabels()[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] return ppExist || cppExist } diff --git a/pkg/controllers/mcs/endpointslice_controller.go b/pkg/controllers/mcs/endpointslice_controller.go index a9089fce1a44..d2e049ead910 100644 --- a/pkg/controllers/mcs/endpointslice_controller.go +++ b/pkg/controllers/mcs/endpointslice_controller.go @@ -33,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" 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/helper" "github.com/karmada-io/karmada/pkg/util/names" @@ -54,20 +55,20 @@ func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerr work := &workv1alpha1.Work{} if err := c.Client.Get(ctx, req.NamespacedName, work); err != nil { if apierrors.IsNotFound(err) { - // Cleanup derived EndpointSlices after work has been removed. - err = helper.DeleteEndpointSlice(c.Client, labels.Set{ - workv1alpha1.WorkNamespaceLabel: req.Namespace, - workv1alpha1.WorkNameLabel: req.Name, - }) - if err == nil { - return controllerruntime.Result{}, nil - } + return controllerruntime.Result{}, nil } return controllerruntime.Result{Requeue: true}, err } if !work.DeletionTimestamp.IsZero() { + // Cleanup derived EndpointSlices after work has been removed. + if err := helper.DeleteEndpointSlice(c.Client, labels.Set{ + workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel], + }); err != nil { + klog.Errorf("Failed to delete endpointslice of work %v, namespace is %v, err is %v", work.Name, work.Namespace, err) + return controllerruntime.Result{}, err + } return controllerruntime.Result{}, nil } @@ -76,8 +77,7 @@ func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerr // Once the conflict between service_export_controller.go and endpointslice_collect_controller.go is fixed, the following code should be deleted. if serviceName := util.GetLabelValue(work.Labels, util.ServiceNameLabel); serviceName == "" { err := helper.DeleteEndpointSlice(c.Client, labels.Set{ - workv1alpha1.WorkNamespaceLabel: req.Namespace, - workv1alpha1.WorkNameLabel: req.Name, + workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel], }) return controllerruntime.Result{}, err } @@ -130,10 +130,8 @@ func (c *EndpointSliceController) collectEndpointSliceFromWork(work *workv1alpha desiredEndpointSlice := deriveEndpointSlice(endpointSlice, clusterName) desiredEndpointSlice.Labels = map[string]string{ - workv1alpha1.WorkNamespaceLabel: work.Namespace, - workv1alpha1.WorkNameLabel: work.Name, - discoveryv1.LabelServiceName: names.GenerateDerivedServiceName(work.Labels[util.ServiceNameLabel]), - util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, + discoveryv1.LabelServiceName: names.GenerateDerivedServiceName(work.Labels[util.ServiceNameLabel]), + util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, } if err = helper.CreateOrUpdateEndpointSlice(c.Client, desiredEndpointSlice); err != nil { diff --git a/pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go b/pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go index adf7660ebfe2..a103905070cd 100644 --- a/pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go +++ b/pkg/controllers/multiclusterservice/endpointslice_dispatch_controller.go @@ -42,6 +42,7 @@ import ( clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" networkingv1alpha1 "github.com/karmada-io/karmada/pkg/apis/networking/v1alpha1" 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/events" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" @@ -383,15 +384,16 @@ func (c *EndpointsliceDispatchController) ensureEndpointSliceWork(mcs *networkin endpointSlice.Name = providerCluster + "-" + endpointSlice.Name clusterNamespace := names.GenerateExecutionSpaceName(consumerCluster) endpointSlice.Labels = map[string]string{ - discoveryv1.LabelServiceName: mcs.Name, - workv1alpha1.WorkNamespaceLabel: clusterNamespace, - workv1alpha1.WorkNameLabel: work.Name, - util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, - discoveryv1.LabelManagedBy: util.EndpointSliceDispatchControllerLabelValue, + discoveryv1.LabelServiceName: mcs.Name, + workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel], + util.ManagedByKarmadaLabel: util.ManagedByKarmadaLabelValue, + discoveryv1.LabelManagedBy: util.EndpointSliceDispatchControllerLabelValue, } endpointSlice.Annotations = map[string]string{ // This annotation is used to identify the source cluster of EndpointSlice and whether the eps are the newest version util.EndpointSliceProvisionClusterAnnotation: providerCluster, + workv1alpha2.WorkNamespaceAnnotation: clusterNamespace, + workv1alpha2.WorkNameAnnotation: work.Name, } workMeta := metav1.ObjectMeta{ diff --git a/pkg/controllers/multiclusterservice/mcs_controller.go b/pkg/controllers/multiclusterservice/mcs_controller.go index 3fc91c8c3c4e..fac9b5cd5973 100644 --- a/pkg/controllers/multiclusterservice/mcs_controller.go +++ b/pkg/controllers/multiclusterservice/mcs_controller.go @@ -496,10 +496,7 @@ func (c *MCSController) claimMultiClusterServiceForService(svc *corev1.Service, } // cleanup the policy labels - delete(svcCopy.Labels, policyv1alpha1.PropagationPolicyNameLabel) - delete(svcCopy.Labels, policyv1alpha1.PropagationPolicyNamespaceLabel) delete(svcCopy.Labels, policyv1alpha1.PropagationPolicyPermanentIDLabel) - delete(svcCopy.Labels, policyv1alpha1.ClusterPropagationPolicyLabel) delete(svcCopy.Labels, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) svcCopy.Labels[util.ResourceTemplateClaimedByLabel] = util.MultiClusterServiceKind diff --git a/pkg/controllers/namespace/namespace_sync_controller.go b/pkg/controllers/namespace/namespace_sync_controller.go index 2a29e32644cb..b3edafd9f0ba 100644 --- a/pkg/controllers/namespace/namespace_sync_controller.go +++ b/pkg/controllers/namespace/namespace_sync_controller.go @@ -40,7 +40,7 @@ import ( clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" - 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/controllers/binding" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/helper" @@ -160,8 +160,8 @@ func (c *Controller) buildWorks(namespace *corev1.Namespace, clusters []clusterv Annotations: annotations, } - util.MergeLabel(clonedNamespaced, workv1alpha1.WorkNamespaceLabel, workNamespace) - util.MergeLabel(clonedNamespaced, workv1alpha1.WorkNameLabel, workName) + util.MergeAnnotation(clonedNamespaced, workv1alpha2.WorkNamespaceAnnotation, workNamespace) + util.MergeAnnotation(clonedNamespaced, workv1alpha2.WorkNameAnnotation, workName) util.MergeLabel(clonedNamespaced, util.ManagedByKarmadaLabel, util.ManagedByKarmadaLabelValue) if err = helper.CreateOrUpdateWork(c.Client, objectMeta, clonedNamespaced); err != nil { diff --git a/pkg/controllers/status/work_status_controller.go b/pkg/controllers/status/work_status_controller.go index 7a5f79c04d07..b6ee07e891f0 100644 --- a/pkg/controllers/status/work_status_controller.go +++ b/pkg/controllers/status/work_status_controller.go @@ -39,6 +39,7 @@ import ( clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" 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/events" "github.com/karmada-io/karmada/pkg/resourceinterpreter" "github.com/karmada-io/karmada/pkg/sharedcli/ratelimiterflag" @@ -165,7 +166,7 @@ func generateKey(obj interface{}) (util.QueueKey, error) { // getClusterNameFromLabel gets cluster name from ownerLabel, if label not exist, means resource is not created by karmada. func getClusterNameFromLabel(resource *unstructured.Unstructured) (string, error) { - workNamespace := util.GetLabelValue(resource.GetLabels(), workv1alpha1.WorkNamespaceLabel) + workNamespace := util.GetAnnotationValue(resource.GetAnnotations(), workv1alpha2.WorkNamespaceAnnotation) if len(workNamespace) == 0 { klog.V(4).Infof("Ignore resource(%s/%s/%s) which not managed by karmada", resource.GetKind(), resource.GetNamespace(), resource.GetName()) return "", nil @@ -195,8 +196,8 @@ func (c *WorkStatusController) syncWorkStatus(key util.QueueKey) error { return err } - workNamespace := util.GetLabelValue(observedObj.GetLabels(), workv1alpha1.WorkNamespaceLabel) - workName := util.GetLabelValue(observedObj.GetLabels(), workv1alpha1.WorkNameLabel) + workNamespace := util.GetAnnotationValue(observedObj.GetAnnotations(), workv1alpha2.WorkNamespaceAnnotation) + workName := util.GetAnnotationValue(observedObj.GetAnnotations(), workv1alpha2.WorkNameAnnotation) if len(workNamespace) == 0 || len(workName) == 0 { klog.Infof("Ignore object(%s) which not managed by karmada.", fedKey.String()) return nil diff --git a/pkg/controllers/status/work_status_controller_test.go b/pkg/controllers/status/work_status_controller_test.go index 921f35375923..b1d87f69077f 100644 --- a/pkg/controllers/status/work_status_controller_test.go +++ b/pkg/controllers/status/work_status_controller_test.go @@ -40,6 +40,7 @@ import ( clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" 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/resourceinterpreter" "github.com/karmada-io/karmada/pkg/sharedcli/ratelimiterflag" "github.com/karmada-io/karmada/pkg/util" @@ -371,8 +372,8 @@ func TestGenerateKey(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: "karmada-es-cluster", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: "karmada-es-cluster", }, }, }, @@ -389,8 +390,8 @@ func TestGenerateKey(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: "karmada-cluster", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: "karmada-cluster", }, }, }, @@ -407,7 +408,7 @@ func TestGenerateKey(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ + "annotations": map[string]interface{}{ "test": "karmada-es-cluster", }, }, @@ -452,8 +453,8 @@ func TestGetClusterNameFromLabel(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: "karmada-es-cluster", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: "karmada-es-cluster", }, }, }, @@ -470,7 +471,7 @@ func TestGetClusterNameFromLabel(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ + "annotations": map[string]interface{}{ "foo": "karmada-es-cluster", }, }, @@ -488,8 +489,8 @@ func TestGetClusterNameFromLabel(t *testing.T) { "metadata": map[string]interface{}{ "name": "test", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: "karmada-cluster", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: "karmada-cluster", }, }, }, @@ -520,9 +521,9 @@ func newPodObj(namespace string) *unstructured.Unstructured { "metadata": map[string]interface{}{ "name": "pod", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: namespace, - workv1alpha1.WorkNameLabel: "work-name", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: namespace, + workv1alpha2.WorkNameAnnotation: "work-name", }, }, }, @@ -537,9 +538,9 @@ func newPod(workNs, workName string, wrongLabel ...bool) *corev1.Pod { ObjectMeta: metav1.ObjectMeta{ Name: "pod", Namespace: "default", - Labels: map[string]string{ - "test": workNs, - workv1alpha1.WorkNameLabel: workName, + Annotations: map[string]string{ + "test": workNs, + workv1alpha2.WorkNameAnnotation: workName, }, }, } @@ -548,9 +549,9 @@ func newPod(workNs, workName string, wrongLabel ...bool) *corev1.Pod { ObjectMeta: metav1.ObjectMeta{ Name: "pod", Namespace: "default", - Labels: map[string]string{ - workv1alpha1.WorkNamespaceLabel: workNs, - workv1alpha1.WorkNameLabel: workName, + Annotations: map[string]string{ + workv1alpha2.WorkNamespaceAnnotation: workNs, + workv1alpha2.WorkNameAnnotation: workName, }, }, } @@ -850,8 +851,8 @@ func TestWorkStatusController_recreateResourceIfNeeded(t *testing.T) { "metadata": map[string]interface{}{ "name": "pod1", "namespace": "default", - "labels": map[string]interface{}{ - workv1alpha1.WorkNamespaceLabel: "karmada-es-cluster", + "annotations": map[string]interface{}{ + workv1alpha2.WorkNamespaceAnnotation: "karmada-es-cluster", }, }, }, diff --git a/pkg/controllers/unifiedauth/unified_auth_controller.go b/pkg/controllers/unifiedauth/unified_auth_controller.go index 1a32f5114e0b..3aedfb2fbd8b 100644 --- a/pkg/controllers/unifiedauth/unified_auth_controller.go +++ b/pkg/controllers/unifiedauth/unified_auth_controller.go @@ -37,7 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" - 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/events" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/helper" @@ -235,8 +235,8 @@ func (c *Controller) buildWorks(cluster *clusterv1alpha1.Cluster, obj *unstructu }, } - util.MergeLabel(obj, workv1alpha1.WorkNamespaceLabel, workNamespace) - util.MergeLabel(obj, workv1alpha1.WorkNameLabel, clusterRoleBindingWorkName) + util.MergeAnnotation(obj, workv1alpha2.WorkNamespaceAnnotation, workNamespace) + util.MergeAnnotation(obj, workv1alpha2.WorkNameAnnotation, clusterRoleBindingWorkName) util.MergeLabel(obj, util.ManagedByKarmadaLabel, util.ManagedByKarmadaLabelValue) if err := helper.CreateOrUpdateWork(c.Client, objectMeta, obj); err != nil { diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go index bf40b7a16e69..23b0575637da 100644 --- a/pkg/detector/detector.go +++ b/pkg/detector/detector.go @@ -63,6 +63,22 @@ import ( "github.com/karmada-io/karmada/pkg/util/restmapper" ) +var ( + propagationPolicyMarkedLabels = []string{ + policyv1alpha1.PropagationPolicyPermanentIDLabel, + } + propagationPolicyMarkedAnnotations = []string{ + policyv1alpha1.PropagationPolicyNamespaceAnnotation, + policyv1alpha1.PropagationPolicyNameAnnotation, + } + clusterPropagationPolicyMarkedLabels = []string{ + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel, + } + clusterPropagationPolicyMarkedAnnotations = []string{ + policyv1alpha1.ClusterPropagationPolicyAnnotation, + } +) + // ResourceDetector is a resource watcher which watches all resources and reconcile the events. type ResourceDetector struct { // DiscoveryClientSet is used to resource discovery. @@ -119,14 +135,14 @@ func (d *ResourceDetector) Start(ctx context.Context) error { // setup policy reconcile worker policyWorkerOptions := util.Options{ Name: "propagationPolicy reconciler", - KeyFunc: ClusterWideKeyFunc, + KeyFunc: PolicyKeyFunc, ReconcileFunc: d.ReconcilePropagationPolicy, } d.policyReconcileWorker = util.NewAsyncWorker(policyWorkerOptions) d.policyReconcileWorker.Run(d.ConcurrentPropagationPolicySyncs, d.stopCh) clusterPolicyWorkerOptions := util.Options{ Name: "clusterPropagationPolicy reconciler", - KeyFunc: ClusterWideKeyFunc, + KeyFunc: PolicyKeyFunc, ReconcileFunc: d.ReconcileClusterPropagationPolicy, } d.clusterPolicyReconcileWorker = util.NewAsyncWorker(clusterPolicyWorkerOptions) @@ -136,7 +152,7 @@ func (d *ResourceDetector) Start(ctx context.Context) error { propagationPolicyGVR := schema.GroupVersionResource{ Group: policyv1alpha1.GroupVersion.Group, Version: policyv1alpha1.GroupVersion.Version, - Resource: "propagationpolicies", + Resource: policyv1alpha1.ResourcePluralPropagationPolicy, } policyHandler := fedinformer.NewHandlerOnEvents(d.OnPropagationPolicyAdd, d.OnPropagationPolicyUpdate, d.OnPropagationPolicyDelete) d.InformerManager.ForResource(propagationPolicyGVR, policyHandler) @@ -146,7 +162,7 @@ func (d *ResourceDetector) Start(ctx context.Context) error { clusterPropagationPolicyGVR := schema.GroupVersionResource{ Group: policyv1alpha1.GroupVersion.Group, Version: policyv1alpha1.GroupVersion.Version, - Resource: "clusterpropagationpolicies", + Resource: policyv1alpha1.ResourcePluralClusterPropagationPolicy, } clusterPolicyHandler := fedinformer.NewHandlerOnEvents(d.OnClusterPropagationPolicyAdd, d.OnClusterPropagationPolicyUpdate, d.OnClusterPropagationPolicyDelete) d.InformerManager.ForResource(clusterPropagationPolicyGVR, clusterPolicyHandler) @@ -439,7 +455,7 @@ func (d *ResourceDetector) ApplyPolicy(object *unstructured.Unstructured, object policyID, err := d.ClaimPolicyForObject(object, policy) if err != nil { - klog.Errorf("Failed to claim policy(%s) for object: %s", policy.Name, object) + klog.Errorf("Failed to claim policy(%s/%s) for object: %s", policy.Namespace, policy.Name, object) return err } @@ -453,8 +469,6 @@ func (d *ResourceDetector) ApplyPolicy(object *unstructured.Unstructured, object } policyLabels := map[string]string{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policy.GetNamespace(), - policyv1alpha1.PropagationPolicyNameLabel: policy.GetName(), policyv1alpha1.PropagationPolicyPermanentIDLabel: policyID, } policyAnnotations := map[string]string{ @@ -462,7 +476,7 @@ func (d *ResourceDetector) ApplyPolicy(object *unstructured.Unstructured, object policyv1alpha1.PropagationPolicyNameAnnotation: policy.GetName(), } - binding, err := d.BuildResourceBinding(object, objectKey, policyLabels, policyAnnotations, &policy.Spec) + binding, err := d.BuildResourceBinding(object, policyLabels, policyAnnotations, &policy.Spec) if err != nil { klog.Errorf("Failed to build resourceBinding for object: %s. error: %v", objectKey, err) return err @@ -553,7 +567,6 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, } policyLabels := map[string]string{ - policyv1alpha1.ClusterPropagationPolicyLabel: policy.GetName(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, } policyAnnotations := map[string]string{ @@ -564,7 +577,7 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, // For namespace-scoped resources, which namespace is not empty, building `ResourceBinding`. // For cluster-scoped resources, which namespace is empty, building `ClusterResourceBinding`. if object.GetNamespace() != "" { - binding, err := d.BuildResourceBinding(object, objectKey, policyLabels, policyAnnotations, &policy.Spec) + binding, err := d.BuildResourceBinding(object, policyLabels, policyAnnotations, &policy.Spec) if err != nil { klog.Errorf("Failed to build resourceBinding for object: %s. error: %v", objectKey, err) return err @@ -620,7 +633,7 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, klog.V(2).Infof("ResourceBinding(%s) is up to date.", binding.GetName()) } } else { - binding, err := d.BuildClusterResourceBinding(object, objectKey, policyLabels, policyAnnotations, &policy.Spec) + binding, err := d.BuildClusterResourceBinding(object, policyLabels, policyAnnotations, &policy.Spec) if err != nil { klog.Errorf("Failed to build clusterResourceBinding for object: %s. error: %v", objectKey, err) return err @@ -708,27 +721,9 @@ func (d *ResourceDetector) GetUnstructuredObject(objectKey keys.ClusterWideKey) return unstructuredObj, nil } -func (d *ResourceDetector) getPropagationPolicyID(policy *policyv1alpha1.PropagationPolicy) (string, error) { - id := util.GetLabelValue(policy.GetLabels(), policyv1alpha1.PropagationPolicyPermanentIDLabel) - if id == "" { - id = uuid.New().String() - policy.Labels = util.DedupeAndMergeLabels(policy.Labels, map[string]string{policyv1alpha1.PropagationPolicyPermanentIDLabel: id}) - if err := d.Client.Update(context.TODO(), policy); err != nil { - return id, err - } - } - - return id, nil -} - // ClaimPolicyForObject set policy identifier which the object associated with. func (d *ResourceDetector) ClaimPolicyForObject(object *unstructured.Unstructured, policy *policyv1alpha1.PropagationPolicy) (string, error) { - policyID, err := d.getPropagationPolicyID(policy) - if err != nil { - klog.Errorf("Get PropagationPolicy(%s/%s) ID error:%v", policy.Namespace, policy.Name, err) - return "", err - } - + policyID := policy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] objLabels := object.GetLabels() if objLabels == nil { objLabels = make(map[string]string) @@ -742,8 +737,6 @@ func (d *ResourceDetector) ClaimPolicyForObject(object *unstructured.Unstructure // Delete following line when release-1.9 delete(objLabels, policyv1alpha1.PropagationPolicyUIDLabel) - objLabels[policyv1alpha1.PropagationPolicyNamespaceLabel] = policy.Namespace - objLabels[policyv1alpha1.PropagationPolicyNameLabel] = policy.Name objLabels[policyv1alpha1.PropagationPolicyPermanentIDLabel] = policyID objectAnnotations := object.GetAnnotations() @@ -759,27 +752,9 @@ func (d *ResourceDetector) ClaimPolicyForObject(object *unstructured.Unstructure return policyID, d.Client.Update(context.TODO(), objectCopy) } -func (d *ResourceDetector) getClusterPropagationPolicyID(policy *policyv1alpha1.ClusterPropagationPolicy) (string, error) { - id := util.GetLabelValue(policy.GetLabels(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) - if id == "" { - id = uuid.New().String() - policy.Labels = util.DedupeAndMergeLabels(policy.Labels, map[string]string{policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: id}) - if err := d.Client.Update(context.TODO(), policy); err != nil { - return "", err - } - } - - return id, nil -} - // ClaimClusterPolicyForObject set cluster identifier which the object associated with func (d *ResourceDetector) ClaimClusterPolicyForObject(object *unstructured.Unstructured, policy *policyv1alpha1.ClusterPropagationPolicy) (string, error) { - policyID, err := d.getClusterPropagationPolicyID(policy) - if err != nil { - klog.Errorf("Get ClusterPropagationPolicy(%s) ID error:%v", policy.Name, err) - return "", err - } - + policyID := util.GetLabelValue(policy.GetLabels(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) claimedID := util.GetLabelValue(object.GetLabels(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) // object has been claimed, don't need to claim again if claimedID == policyID { @@ -788,11 +763,10 @@ func (d *ResourceDetector) ClaimClusterPolicyForObject(object *unstructured.Unst objectCopy := object.DeepCopy() // TODO: delete following 3 lines in release-1.9 - labels := objectCopy.GetLabels() - delete(labels, policyv1alpha1.ClusterPropagationPolicyUIDLabel) - objectCopy.SetLabels(labels) + ls := objectCopy.GetLabels() + delete(ls, policyv1alpha1.ClusterPropagationPolicyUIDLabel) + objectCopy.SetLabels(ls) - util.MergeLabel(objectCopy, policyv1alpha1.ClusterPropagationPolicyLabel, policy.Name) util.MergeLabel(objectCopy, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel, policyID) util.MergeAnnotation(objectCopy, policyv1alpha1.ClusterPropagationPolicyAnnotation, policy.Name) @@ -800,7 +774,7 @@ func (d *ResourceDetector) ClaimClusterPolicyForObject(object *unstructured.Unst } // BuildResourceBinding builds a desired ResourceBinding for object. -func (d *ResourceDetector) BuildResourceBinding(object *unstructured.Unstructured, objectKey keys.ClusterWideKey, +func (d *ResourceDetector) BuildResourceBinding(object *unstructured.Unstructured, labels, annotations map[string]string, policySpec *policyv1alpha1.PropagationSpec) (*workv1alpha2.ResourceBinding, error) { bindingName := names.GenerateBindingName(object.GetKind(), object.GetName()) propagationBinding := &workv1alpha2.ResourceBinding{ @@ -808,7 +782,7 @@ func (d *ResourceDetector) BuildResourceBinding(object *unstructured.Unstructure Name: bindingName, Namespace: object.GetNamespace(), OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(object, objectKey.GroupVersionKind()), + *metav1.NewControllerRef(object, object.GroupVersionKind()), }, Annotations: annotations, Labels: labels, @@ -845,14 +819,14 @@ func (d *ResourceDetector) BuildResourceBinding(object *unstructured.Unstructure } // BuildClusterResourceBinding builds a desired ClusterResourceBinding for object. -func (d *ResourceDetector) BuildClusterResourceBinding(object *unstructured.Unstructured, objectKey keys.ClusterWideKey, +func (d *ResourceDetector) BuildClusterResourceBinding(object *unstructured.Unstructured, labels, annotations map[string]string, policySpec *policyv1alpha1.PropagationSpec) (*workv1alpha2.ClusterResourceBinding, error) { bindingName := names.GenerateBindingName(object.GetKind(), object.GetName()) binding := &workv1alpha2.ClusterResourceBinding{ ObjectMeta: metav1.ObjectMeta{ Name: bindingName, OwnerReferences: []metav1.OwnerReference{ - *metav1.NewControllerRef(object, objectKey.GroupVersionKind()), + *metav1.NewControllerRef(object, object.GroupVersionKind()), }, Annotations: annotations, Labels: labels, @@ -1023,7 +997,7 @@ func (d *ResourceDetector) OnPropagationPolicyDelete(obj interface{}) { // When removing a PropagationPolicy, the relevant ResourceBinding will be removed and // the relevant objects will be put into queue again to try another policy. func (d *ResourceDetector) ReconcilePropagationPolicy(key util.QueueKey) error { - ckey, ok := key.(keys.ClusterWideKey) + ckey, ok := key.(*PolicyKey) if !ok { // should not happen klog.Error("Found invalid key when reconciling propagation policy.") return fmt.Errorf("invalid key") @@ -1033,7 +1007,7 @@ func (d *ResourceDetector) ReconcilePropagationPolicy(key util.QueueKey) error { if err != nil { if apierrors.IsNotFound(err) { klog.Infof("PropagationPolicy(%s) has been removed.", ckey.NamespaceKey()) - return d.HandlePropagationPolicyDeletion(ckey.Namespace, ckey.Name) + return d.HandlePropagationPolicyDeletion(ckey.PermanentID) } klog.Errorf("Failed to get PropagationPolicy(%s): %v", ckey.NamespaceKey(), err) return err @@ -1133,7 +1107,7 @@ func (d *ResourceDetector) OnClusterPropagationPolicyDelete(obj interface{}) { // When removing a ClusterPropagationPolicy, the relevant ClusterResourceBinding will be removed and // the relevant objects will be put into queue again to try another policy. func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey) error { - ckey, ok := key.(keys.ClusterWideKey) + ckey, ok := key.(*PolicyKey) if !ok { // should not happen klog.Error("Found invalid key when reconciling cluster propagation policy.") return fmt.Errorf("invalid key") @@ -1143,7 +1117,7 @@ func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey) if err != nil { if apierrors.IsNotFound(err) { klog.Infof("ClusterPropagationPolicy(%s) has been removed.", ckey.NamespaceKey()) - return d.HandleClusterPropagationPolicyDeletion(ckey.Name) + return d.HandleClusterPropagationPolicyDeletion(ckey.PermanentID) } klog.Errorf("Failed to get ClusterPropagationPolicy(%s): %v", ckey.NamespaceKey(), err) @@ -1160,75 +1134,82 @@ func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey) } // HandlePropagationPolicyDeletion handles PropagationPolicy delete event. -// After a policy is removed, the label marked on relevant resource template will be removed (which gives +// After a policy is removed, the label and annotations marked on relevant resource template will be removed (which gives // the resource template a change to match another policy). // // Note: The relevant ResourceBinding will continue to exist until the resource template is gone. -func (d *ResourceDetector) HandlePropagationPolicyDeletion(policyNS string, policyName string) error { - labelSet := labels.Set{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policyNS, - policyv1alpha1.PropagationPolicyNameLabel: policyName, - } - - rbs, err := helper.GetResourceBindings(d.Client, labelSet) +func (d *ResourceDetector) HandlePropagationPolicyDeletion(policyID string) error { + rbs, err := helper.GetResourceBindings(d.Client, labels.Set{policyv1alpha1.PropagationPolicyPermanentIDLabel: policyID}) if err != nil { - klog.Errorf("Failed to list propagation bindings: %v", err) + klog.Errorf("Failed to list propagation bindings with policy permanentID(%s): %v", policyID, err) return err } + cleanupMarksFunc := func(obj metav1.Object) { + util.RemoveLabels(obj, propagationPolicyMarkedLabels...) + util.RemoveAnnotations(obj, propagationPolicyMarkedAnnotations...) + } + var errs []error for index, binding := range rbs.Items { - // Must remove the labels from the resource template ahead of ResourceBinding, otherwise might lose the chance - // to do that in a retry loop(in particular, the label was successfully removed from ResourceBinding, but - // resource template not), since the ResourceBinding will not be listed again. - if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.PropagationPolicyNamespaceLabel, policyv1alpha1.PropagationPolicyNameLabel, policyv1alpha1.PropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from resource(%s-%s/%s) when propagation policy(%s/%s) removing, error: %v", - binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, policyNS, policyName, err) - return err + // Must remove the marks, such as labels and annotations, from the resource template ahead of ResourceBinding, + // otherwise might lose the chance to do that in a retry loop (in particular, the marks was successfully removed + // from ResourceBinding, but resource template not), since the ResourceBinding will not be listed again. + if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, cleanupMarksFunc); err != nil { + klog.Errorf("Failed to clean up marks from resource(%s-%s/%s) when propagationPolicy removed, error: %v", + binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, err) + errs = append(errs, err) + // Skip cleaning up policy labels and annotations from ResourceBinding, give a chance to do that in a retry loop. + continue } - // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.PropagationPolicyNamespaceLabel, policyv1alpha1.PropagationPolicyNameLabel, policyv1alpha1.PropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from resource binding(%s/%s) when propagation policy(%s/%s) removing, error: %v", - binding.Namespace, binding.Name, policyNS, policyName, err) - return err + // Clean up the marks from the reference binding so that the karmada scheduler won't reschedule the binding. + if err := d.CleanupResourceBindingMarks(&rbs.Items[index], cleanupMarksFunc); err != nil { + klog.Errorf("Failed to clean up marks from resource binding(%s/%s) when propagationPolicy removed, error: %v", + binding.Namespace, binding.Name, err) + errs = append(errs, err) } } - return nil + return errors.NewAggregate(errs) } // HandleClusterPropagationPolicyDeletion handles ClusterPropagationPolicy delete event. -// After a policy is removed, the label marked on relevant resource template will be removed (which gives +// After a policy is removed, the label and annotation marked on relevant resource template will be removed (which gives // the resource template a change to match another policy). // // Note: The relevant ClusterResourceBinding or ResourceBinding will continue to exist until the resource template is gone. -func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName string) error { +func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID string) error { var errs []error labelSet := labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policyName, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, + } + + cleanupMarksFun := func(obj metav1.Object) { + util.RemoveLabels(obj, clusterPropagationPolicyMarkedLabels...) + util.RemoveAnnotations(obj, clusterPropagationPolicyMarkedAnnotations...) } // load the ClusterResourceBindings which labeled with current policy crbs, err := helper.GetClusterResourceBindings(d.Client, labelSet) if err != nil { - klog.Errorf("Failed to load cluster resource binding by policy(%s), error: %v", policyName, err) + klog.Errorf("Failed to list clusterResourceBindings with clusterPropagationPolicy permanentID(%s), error: %v", policyID, err) errs = append(errs, err) } else if len(crbs.Items) > 0 { for index, binding := range crbs.Items { - // Must remove the labels from the resource template ahead of ClusterResourceBinding, otherwise might lose the chance - // to do that in a retry loop(in particular, the label was successfully removed from ClusterResourceBinding, but - // resource template not), since the ClusterResourceBinding will not be listed again. - if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from resource(%s-%s) when cluster propagation policy(%s) removing, error: %v", - binding.Spec.Resource.Kind, binding.Spec.Resource.Name, policyName, err) - errs = append(errs, err) - // Skip cleaning up policy labels from ClusterResourceBinding, give a chance to do that in a retry loop. + // Must remove the marks, such as labels and annotations, from the resource template ahead of + // ClusterResourceBinding, otherwise might lose the chance to do that in a retry loop (in particular, the + // marks was successfully removed from ClusterResourceBinding, but resource template not), since the + // ClusterResourceBinding will not be listed again. + if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, cleanupMarksFun); err != nil { + klog.Errorf("Failed to clean up marks from resource(%s-%s) when clusterPropagationPolicy removed, error: %v", + binding.Spec.Resource.Kind, binding.Spec.Resource.Name, err) + // Skip cleaning up policy labels and annotations from ClusterResourceBinding, give a chance to do that in a retry loop. continue } - // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupClusterResourceBindingLabels(&crbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from cluster resource binding(%s) when cluster propagation policy(%s) removing, error: %v", - binding.Name, policyName, err) + // Clean up the marks from the reference binding so that the Karmada scheduler won't reschedule the binding. + if err := d.CleanupClusterResourceBindingLabels(&crbs.Items[index], cleanupMarksFun); err != nil { + klog.Errorf("Failed to clean up marks from clusterResourceBinding(%s) when clusterPropagationPolicy removed, error: %v", + binding.Name, err) errs = append(errs, err) } } @@ -1237,25 +1218,25 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName str // load the ResourceBindings which labeled with current policy rbs, err := helper.GetResourceBindings(d.Client, labelSet) if err != nil { - klog.Errorf("Failed to load resource binding by policy(%s), error: %v", policyName, err) + klog.Errorf("Failed to list resourceBindings with clusterPropagationPolicy permanentID(%s), error: %v", policyID, err) errs = append(errs, err) } else if len(rbs.Items) > 0 { for index, binding := range rbs.Items { - // Must remove the labels from the resource template ahead of ResourceBinding, otherwise might lose the chance - // to do that in a retry loop(in particular, the label was successfully removed from ResourceBinding, but - // resource template not), since the ResourceBinding will not be listed again. - if err := d.CleanupLabels(binding.Spec.Resource, policyv1alpha1.ClusterPropagationPolicyLabel, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from resource(%s-%s/%s) when cluster propagation policy(%s) removing, error: %v", - binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, policyName, err) + // Must remove the marks, such as labels and annotations, from the resource template ahead of ResourceBinding, + // otherwise might lose the chance to do that in a retry loop (in particular, the label was successfully + // removed from ResourceBinding, but resource template not), since the ResourceBinding will not be listed again. + if err := d.CleanupResourceTemplateMarks(binding.Spec.Resource, cleanupMarksFun); err != nil { + klog.Errorf("Failed to clean up marks from resource(%s-%s/%s) when clusterPropagationPolicy removed, error: %v", + binding.Spec.Resource.Kind, binding.Spec.Resource.Namespace, binding.Spec.Resource.Name, err) errs = append(errs, err) - // Skip cleaning up policy labels from ResourceBinding, give a chance to do that in a retry loop. + // Skip cleaning up policy labels and annotations from ResourceBinding, give a chance to do that in a retry loop. continue } - // Clean up the labels from the reference binding so that the karmada scheduler won't reschedule the binding. - if err := d.CleanupResourceBindingLabels(&rbs.Items[index], policyv1alpha1.ClusterPropagationPolicyLabel, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel); err != nil { - klog.Errorf("Failed to clean up label from resource binding(%s/%s) when cluster propagation policy(%s) removing, error: %v", - binding.Namespace, binding.Name, policyName, err) + // Clean up the marks from the reference binding so that the Karmada scheduler won't reschedule the binding. + if err := d.CleanupResourceBindingMarks(&rbs.Items[index], cleanupMarksFun); err != nil { + klog.Errorf("Failed to clean up marks from resourceBinding(%s/%s) when clusterPropagationPolicy removed, error: %v", + binding.Namespace, binding.Name, err) errs = append(errs, err) } } @@ -1272,7 +1253,8 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName str func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *policyv1alpha1.PropagationPolicy) error { // If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label marked // on relevant resource template will be removed (which gives the resource template a change to match another policy). - err := d.cleanPPUnmatchedResourceBindings(policy.Namespace, policy.Name, policy.Spec.ResourceSelectors) + policyID := policy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] + err := d.cleanPPUnmatchedRBs(policyID, policy.Namespace, policy.Name, policy.Spec.ResourceSelectors) if err != nil { return err } @@ -1280,7 +1262,7 @@ func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *polic // When updating fields other than ResourceSelector, should first find the corresponding ResourceBinding // and add the bound object to the processor's queue for reconciliation to make sure that // PropagationPolicy's updates can be synchronized to ResourceBinding. - resourceBindings, err := d.listPPDerivedRB(policy.Namespace, policy.Name) + resourceBindings, err := d.listPPDerivedRBs(policyID, policy.Namespace, policy.Name) if err != nil { return err } @@ -1329,12 +1311,13 @@ func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *polic func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy *policyv1alpha1.ClusterPropagationPolicy) error { // If the Policy's ResourceSelectors change, causing certain resources to no longer match the Policy, the label marked // on relevant resource template will be removed (which gives the resource template a change to match another policy). - err := d.cleanCPPUnmatchedResourceBindings(policy.Name, policy.Spec.ResourceSelectors) + policyID := policy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] + err := d.cleanCPPUnmatchedRBs(policyID, policy.Name, policy.Spec.ResourceSelectors) if err != nil { return err } - err = d.cleanUnmatchedClusterResourceBinding(policy.Name, policy.Spec.ResourceSelectors) + err = d.cleanUnmatchedCRBs(policyID, policy.Name, policy.Spec.ResourceSelectors) if err != nil { return err } @@ -1342,11 +1325,11 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy // When updating fields other than ResourceSelector, should first find the corresponding ResourceBinding/ClusterResourceBinding // and add the bound object to the processor's queue for reconciliation to make sure that // ClusterPropagationPolicy's updates can be synchronized to ResourceBinding/ClusterResourceBinding. - resourceBindings, err := d.listCPPDerivedRB(policy.Name) + resourceBindings, err := d.listCPPDerivedRBs(policyID, policy.Name) if err != nil { return err } - clusterResourceBindings, err := d.listCPPDerivedCRB(policy.Name) + clusterResourceBindings, err := d.listCPPDerivedCRBs(policyID, policy.Name) if err != nil { return err } @@ -1392,11 +1375,11 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy return nil } -// CleanupLabels removes labels from object referencing by objRef. -func (d *ResourceDetector) CleanupLabels(objRef workv1alpha2.ObjectReference, labelKeys ...string) error { +// CleanupResourceTemplateMarks removes marks, such as labels and annotations, from object referencing by objRef. +func (d *ResourceDetector) CleanupResourceTemplateMarks(objRef workv1alpha2.ObjectReference, cleanupFunc func(obj metav1.Object)) error { workload, err := helper.FetchResourceTemplate(d.DynamicClient, d.InformerManager, d.RESTMapper, objRef) if err != nil { - // do nothing if resource template not exist, it might has been removed. + // do nothing if resource template not exist, it might have been removed. if apierrors.IsNotFound(err) { return nil } @@ -1405,7 +1388,7 @@ func (d *ResourceDetector) CleanupLabels(objRef workv1alpha2.ObjectReference, la } workload = workload.DeepCopy() - util.RemoveLabels(workload, labelKeys...) + cleanupFunc(workload) gvr, err := restmapper.GetGroupVersionResource(d.RESTMapper, workload.GroupVersionKind()) if err != nil { @@ -1422,15 +1405,10 @@ func (d *ResourceDetector) CleanupLabels(objRef workv1alpha2.ObjectReference, la return nil } -// CleanupResourceBindingLabels removes labels from resource binding. -func (d *ResourceDetector) CleanupResourceBindingLabels(rb *workv1alpha2.ResourceBinding, labels ...string) error { - bindingLabels := rb.GetLabels() - for _, l := range labels { - delete(bindingLabels, l) - } - +// CleanupResourceBindingMarks removes marks, such as labels and annotations, from resource binding. +func (d *ResourceDetector) CleanupResourceBindingMarks(rb *workv1alpha2.ResourceBinding, cleanupFunc func(obj metav1.Object)) error { return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) { - rb.SetLabels(bindingLabels) + cleanupFunc(rb) updateErr := d.Client.Update(context.TODO(), rb) if updateErr == nil { return nil @@ -1438,7 +1416,7 @@ func (d *ResourceDetector) CleanupResourceBindingLabels(rb *workv1alpha2.Resourc updated := &workv1alpha2.ResourceBinding{} if err = d.Client.Get(context.TODO(), client.ObjectKey{Namespace: rb.GetNamespace(), Name: rb.GetName()}, updated); err == nil { - rb = updated + rb = updated.DeepCopy() } else { klog.Errorf("Failed to get updated resource binding %s/%s: %v", rb.GetNamespace(), rb.GetName(), err) } @@ -1446,15 +1424,10 @@ func (d *ResourceDetector) CleanupResourceBindingLabels(rb *workv1alpha2.Resourc }) } -// CleanupClusterResourceBindingLabels removes labels from cluster resource binding. -func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2.ClusterResourceBinding, labels ...string) error { - bindingLabels := crb.GetLabels() - for _, l := range labels { - delete(bindingLabels, l) - } - +// CleanupClusterResourceBindingLabels removes marks, such as labels and annotations, from cluster resource binding. +func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error { return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) { - crb.SetLabels(bindingLabels) + cleanupFunc(crb) updateErr := d.Client.Update(context.TODO(), crb) if updateErr == nil { return nil @@ -1462,7 +1435,7 @@ func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2 updated := &workv1alpha2.ClusterResourceBinding{} if err = d.Client.Get(context.TODO(), client.ObjectKey{Name: crb.GetName()}, updated); err == nil { - crb = updated + crb = updated.DeepCopy() } else { klog.Errorf("Failed to get updated cluster resource binding %s: %v", crb.GetName(), err) } diff --git a/pkg/detector/handler.go b/pkg/detector/handler.go index c9f63a61bb12..d5f8fe491a22 100644 --- a/pkg/detector/handler.go +++ b/pkg/detector/handler.go @@ -19,8 +19,10 @@ package detector import ( "fmt" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" + policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/fedinformer/keys" ) @@ -59,3 +61,33 @@ func ResourceItemKeyFunc(obj interface{}) (util.QueueKey, error) { return key, nil } + +// PolicyKey is the object key of propagation policy. +type PolicyKey struct { + keys.ClusterWideKey + // PermanentID is the permanent ID of the referencing propagation policy. + PermanentID string +} + +// PolicyKeyFunc generates a PolicyKey for object. +func PolicyKeyFunc(obj interface{}) (util.QueueKey, error) { + key, err := keys.ClusterWideKeyFunc(obj) + if err != nil { + return nil, err + } + metaInfo, err := meta.Accessor(obj) + if err != nil { // should not happen + return nil, fmt.Errorf("object has no meta: %v", err) + } + + var ppID string + if len(metaInfo.GetNamespace()) == 0 { + ppID = metaInfo.GetLabels()[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] + } else { + ppID = metaInfo.GetLabels()[policyv1alpha1.PropagationPolicyPermanentIDLabel] + } + return &PolicyKey{ + ClusterWideKey: key, + PermanentID: ppID, + }, nil +} diff --git a/pkg/detector/policy.go b/pkg/detector/policy.go index 8b13be932258..2f6ec719de64 100644 --- a/pkg/detector/policy.go +++ b/pkg/detector/policy.go @@ -40,18 +40,21 @@ func (d *ResourceDetector) propagateResource(object *unstructured.Unstructured, objectKey keys.ClusterWideKey, resourceChangeByKarmada bool) error { // 1. Check if the object has been claimed by a PropagationPolicy, // if so, just apply it. + policyAnnotations := object.GetAnnotations() policyLabels := object.GetLabels() - claimedNamespace := util.GetLabelValue(policyLabels, policyv1alpha1.PropagationPolicyNamespaceLabel) - claimedName := util.GetLabelValue(policyLabels, policyv1alpha1.PropagationPolicyNameLabel) - if claimedNamespace != "" && claimedName != "" { - return d.getAndApplyPolicy(object, objectKey, resourceChangeByKarmada, claimedNamespace, claimedName) + claimedNamespace := util.GetAnnotationValue(policyAnnotations, policyv1alpha1.PropagationPolicyNamespaceAnnotation) + claimedName := util.GetAnnotationValue(policyAnnotations, policyv1alpha1.PropagationPolicyNameAnnotation) + claimedID := util.GetLabelValue(policyLabels, policyv1alpha1.PropagationPolicyPermanentIDLabel) + if claimedNamespace != "" && claimedName != "" && claimedID != "" { + return d.getAndApplyPolicy(object, objectKey, resourceChangeByKarmada, claimedNamespace, claimedName, claimedID) } // 2. Check if the object has been claimed by a ClusterPropagationPolicy, // if so, just apply it. - claimedName = util.GetLabelValue(policyLabels, policyv1alpha1.ClusterPropagationPolicyLabel) - if claimedName != "" { - return d.getAndApplyClusterPolicy(object, objectKey, resourceChangeByKarmada, claimedName) + claimedName = util.GetAnnotationValue(policyAnnotations, policyv1alpha1.ClusterPropagationPolicyAnnotation) + claimedID = util.GetLabelValue(policyLabels, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) + if claimedName != "" && claimedID != "" { + return d.getAndApplyClusterPolicy(object, objectKey, resourceChangeByKarmada, claimedName, claimedID) } // 3. attempt to match policy in its namespace. @@ -102,12 +105,12 @@ func (d *ResourceDetector) propagateResource(object *unstructured.Unstructured, } func (d *ResourceDetector) getAndApplyPolicy(object *unstructured.Unstructured, objectKey keys.ClusterWideKey, - resourceChangeByKarmada bool, policyNamespace, policyName string) error { + resourceChangeByKarmada bool, policyNamespace, policyName, claimedID string) error { policyObject, err := d.propagationPolicyLister.ByNamespace(policyNamespace).Get(policyName) if err != nil { if apierrors.IsNotFound(err) { klog.V(4).Infof("PropagationPolicy(%s/%s) has been removed.", policyNamespace, policyName) - return d.HandlePropagationPolicyDeletion(policyNamespace, policyName) + return d.HandlePropagationPolicyDeletion(claimedID) } klog.Errorf("Failed to get claimed policy(%s/%s),: %v", policyNamespace, policyName, err) return err @@ -140,12 +143,12 @@ func (d *ResourceDetector) getAndApplyPolicy(object *unstructured.Unstructured, } func (d *ResourceDetector) getAndApplyClusterPolicy(object *unstructured.Unstructured, objectKey keys.ClusterWideKey, - resourceChangeByKarmada bool, policyName string) error { + resourceChangeByKarmada bool, policyName, policyID string) error { policyObject, err := d.clusterPropagationPolicyLister.Get(policyName) if err != nil { if apierrors.IsNotFound(err) { klog.V(4).Infof("ClusterPropagationPolicy(%s) has been removed.", policyName) - return d.HandleClusterPropagationPolicyDeletion(policyName) + return d.HandleClusterPropagationPolicyDeletion(policyID) } klog.Errorf("Failed to get claimed policy(%s),: %v", policyName, err) @@ -178,46 +181,39 @@ func (d *ResourceDetector) getAndApplyClusterPolicy(object *unstructured.Unstruc return d.ApplyClusterPolicy(object, objectKey, resourceChangeByKarmada, matchedClusterPropagationPolicy) } -func (d *ResourceDetector) cleanPPUnmatchedResourceBindings(policyNamespace, policyName string, selectors []policyv1alpha1.ResourceSelector) error { - bindings, err := d.listPPDerivedRB(policyNamespace, policyName) +func (d *ResourceDetector) cleanPPUnmatchedRBs(policyID, policyNamespace, policyName string, selectors []policyv1alpha1.ResourceSelector) error { + bindings, err := d.listPPDerivedRBs(policyID, policyNamespace, policyName) if err != nil { return err } - removeLabels := []string{ - policyv1alpha1.PropagationPolicyNamespaceLabel, - policyv1alpha1.PropagationPolicyNameLabel, - } - return d.removeResourceBindingsLabels(bindings, selectors, removeLabels) + return d.removeRBsMarks(bindings, selectors, propagationPolicyMarkedLabels, propagationPolicyMarkedAnnotations) } -func (d *ResourceDetector) cleanCPPUnmatchedResourceBindings(policyName string, selectors []policyv1alpha1.ResourceSelector) error { - bindings, err := d.listCPPDerivedRB(policyName) +func (d *ResourceDetector) cleanCPPUnmatchedRBs(policyID, policyName string, selectors []policyv1alpha1.ResourceSelector) error { + bindings, err := d.listCPPDerivedRBs(policyID, policyName) if err != nil { return err } - removeLabels := []string{ - policyv1alpha1.ClusterPropagationPolicyLabel, - } - return d.removeResourceBindingsLabels(bindings, selectors, removeLabels) + return d.removeRBsMarks(bindings, selectors, clusterPropagationPolicyMarkedLabels, clusterPropagationPolicyMarkedAnnotations) } -func (d *ResourceDetector) cleanUnmatchedClusterResourceBinding(policyName string, selectors []policyv1alpha1.ResourceSelector) error { - bindings, err := d.listCPPDerivedCRB(policyName) +func (d *ResourceDetector) cleanUnmatchedCRBs(policyID, policyName string, selectors []policyv1alpha1.ResourceSelector) error { + bindings, err := d.listCPPDerivedCRBs(policyID, policyName) if err != nil { return err } - return d.removeClusterResourceBindingsLabels(bindings, selectors) + return d.removeCRBsMarks(bindings, selectors, clusterPropagationPolicyMarkedLabels, clusterPropagationPolicyMarkedAnnotations) } -func (d *ResourceDetector) removeResourceBindingsLabels(bindings *workv1alpha2.ResourceBindingList, selectors []policyv1alpha1.ResourceSelector, removeLabels []string) error { +func (d *ResourceDetector) removeRBsMarks(bindings *workv1alpha2.ResourceBindingList, selectors []policyv1alpha1.ResourceSelector, labels, annotations []string) error { var errs []error for _, binding := range bindings.Items { - removed, err := d.removeResourceLabelsIfNotMatch(binding.Spec.Resource, selectors, removeLabels...) + removed, err := d.removeResourceMarksIfMatched(binding.Spec.Resource, selectors, labels, annotations) if err != nil { - klog.Errorf("Failed to remove resource labels when resource not match with policy selectors, err: %v", err) + klog.Errorf("Failed to remove resource labels and annotations when resource not match with policy selectors, err: %v", err) errs = append(errs, err) continue } @@ -226,9 +222,8 @@ func (d *ResourceDetector) removeResourceBindingsLabels(bindings *workv1alpha2.R } bindingCopy := binding.DeepCopy() - for _, l := range removeLabels { - delete(bindingCopy.Labels, l) - } + util.RemoveLabels(bindingCopy, labels...) + util.RemoveAnnotations(bindingCopy, annotations...) err = d.Client.Update(context.TODO(), bindingCopy) if err != nil { klog.Errorf("Failed to update resourceBinding(%s/%s), err: %v", binding.Namespace, binding.Name, err) @@ -236,19 +231,16 @@ func (d *ResourceDetector) removeResourceBindingsLabels(bindings *workv1alpha2.R } } - if len(errs) > 0 { - return errors.NewAggregate(errs) - } - - return nil + return errors.NewAggregate(errs) } -func (d *ResourceDetector) removeClusterResourceBindingsLabels(bindings *workv1alpha2.ClusterResourceBindingList, selectors []policyv1alpha1.ResourceSelector) error { +func (d *ResourceDetector) removeCRBsMarks(bindings *workv1alpha2.ClusterResourceBindingList, + selectors []policyv1alpha1.ResourceSelector, removeLabels, removeAnnotations []string) error { var errs []error for _, binding := range bindings.Items { - removed, err := d.removeResourceLabelsIfNotMatch(binding.Spec.Resource, selectors, []string{policyv1alpha1.ClusterPropagationPolicyLabel}...) + removed, err := d.removeResourceMarksIfMatched(binding.Spec.Resource, selectors, removeLabels, removeAnnotations) if err != nil { - klog.Errorf("Failed to remove resource labels when resource not match with policy selectors, err: %v", err) + klog.Errorf("Failed to remove resource labels and annotations when resource not match with policy selectors, err: %v", err) errs = append(errs, err) continue } @@ -257,7 +249,8 @@ func (d *ResourceDetector) removeClusterResourceBindingsLabels(bindings *workv1a } bindingCopy := binding.DeepCopy() - delete(bindingCopy.Labels, policyv1alpha1.ClusterPropagationPolicyLabel) + util.RemoveLabels(bindingCopy, removeLabels...) + util.RemoveAnnotations(bindingCopy, removeAnnotations...) err = d.Client.Update(context.TODO(), bindingCopy) if err != nil { klog.Errorf("Failed to update clusterResourceBinding(%s), err: %v", binding.Name, err) @@ -265,13 +258,11 @@ func (d *ResourceDetector) removeClusterResourceBindingsLabels(bindings *workv1a } } - if len(errs) > 0 { - return errors.NewAggregate(errs) - } - return nil + return errors.NewAggregate(errs) } -func (d *ResourceDetector) removeResourceLabelsIfNotMatch(objectReference workv1alpha2.ObjectReference, selectors []policyv1alpha1.ResourceSelector, labelKeys ...string) (bool, error) { +func (d *ResourceDetector) removeResourceMarksIfMatched(objectReference workv1alpha2.ObjectReference, + selectors []policyv1alpha1.ResourceSelector, labels, annotations []string) (bool, error) { objectKey, err := helper.ConstructClusterWideKey(objectReference) if err != nil { return false, err @@ -290,7 +281,8 @@ func (d *ResourceDetector) removeResourceLabelsIfNotMatch(objectReference workv1 } object = object.DeepCopy() - util.RemoveLabels(object, labelKeys...) + util.RemoveLabels(object, labels...) + util.RemoveAnnotations(object, annotations...) err = d.Client.Update(context.TODO(), object) if err != nil { @@ -299,13 +291,12 @@ func (d *ResourceDetector) removeResourceLabelsIfNotMatch(objectReference workv1 return true, nil } -func (d *ResourceDetector) listPPDerivedRB(policyNamespace, policyName string) (*workv1alpha2.ResourceBindingList, error) { +func (d *ResourceDetector) listPPDerivedRBs(policyID, policyNamespace, policyName string) (*workv1alpha2.ResourceBindingList, error) { bindings := &workv1alpha2.ResourceBindingList{} listOpt := &client.ListOptions{ Namespace: policyNamespace, LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policyNamespace, - policyv1alpha1.PropagationPolicyNameLabel: policyName, + policyv1alpha1.PropagationPolicyPermanentIDLabel: policyID, }), } err := d.Client.List(context.TODO(), bindings, listOpt) @@ -317,11 +308,11 @@ func (d *ResourceDetector) listPPDerivedRB(policyNamespace, policyName string) ( return bindings, nil } -func (d *ResourceDetector) listCPPDerivedRB(policyName string) (*workv1alpha2.ResourceBindingList, error) { +func (d *ResourceDetector) listCPPDerivedRBs(policyID, policyName string) (*workv1alpha2.ResourceBindingList, error) { bindings := &workv1alpha2.ResourceBindingList{} listOpt := &client.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policyName, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, })} err := d.Client.List(context.TODO(), bindings, listOpt) if err != nil { @@ -332,11 +323,11 @@ func (d *ResourceDetector) listCPPDerivedRB(policyName string) (*workv1alpha2.Re return bindings, nil } -func (d *ResourceDetector) listCPPDerivedCRB(policyName string) (*workv1alpha2.ClusterResourceBindingList, error) { +func (d *ResourceDetector) listCPPDerivedCRBs(policyID, policyName string) (*workv1alpha2.ClusterResourceBindingList, error) { bindings := &workv1alpha2.ClusterResourceBindingList{} listOpt := &client.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policyName, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, })} err := d.Client.List(context.TODO(), bindings, listOpt) if err != nil { @@ -348,11 +339,11 @@ func (d *ResourceDetector) listCPPDerivedCRB(policyName string) (*workv1alpha2.C } // excludeClusterPolicy excludes cluster propagation policy. -// If propagation policy was claimed, cluster propagation policy should not exists. +// If propagation policy was claimed, cluster propagation policy should not exist. func excludeClusterPolicy(objLabels map[string]string) bool { - if _, ok := objLabels[policyv1alpha1.ClusterPropagationPolicyLabel]; !ok { + if _, ok := objLabels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel]; !ok { return false } - delete(objLabels, policyv1alpha1.ClusterPropagationPolicyLabel) + delete(objLabels, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) return true } diff --git a/pkg/detector/preemption.go b/pkg/detector/preemption.go index 3defea7bb9cb..f30d05b1aa95 100755 --- a/pkg/detector/preemption.go +++ b/pkg/detector/preemption.go @@ -54,7 +54,7 @@ func preemptionEnabled(preemption policyv1alpha1.PreemptionBehavior) bool { return true } -// handleClusterPropagationPolicyPreemption handles the preemption process of PropagationPolicy. +// handlePropagationPolicyPreemption handles the preemption process of PropagationPolicy. // The preemption rule: high-priority PP > low-priority PP > CPP. func (d *ResourceDetector) handlePropagationPolicyPreemption(policy *policyv1alpha1.PropagationPolicy) error { var errs []error @@ -104,9 +104,9 @@ func (d *ResourceDetector) handleClusterPropagationPolicyPreemption(policy *poli // preemptPropagationPolicy preempts resource template that is claimed by PropagationPolicy. func (d *ResourceDetector) preemptPropagationPolicy(resourceTemplate *unstructured.Unstructured, policy *policyv1alpha1.PropagationPolicy) (err error) { - rtLabels := resourceTemplate.GetLabels() - claimedPolicyNamespace := util.GetLabelValue(rtLabels, policyv1alpha1.PropagationPolicyNamespaceLabel) - claimedPolicyName := util.GetLabelValue(rtLabels, policyv1alpha1.PropagationPolicyNameLabel) + rtAnnotations := resourceTemplate.GetAnnotations() + claimedPolicyNamespace := util.GetAnnotationValue(rtAnnotations, policyv1alpha1.PropagationPolicyNamespaceAnnotation) + claimedPolicyName := util.GetAnnotationValue(rtAnnotations, policyv1alpha1.PropagationPolicyNameAnnotation) if claimedPolicyName == "" || claimedPolicyNamespace == "" { return nil } @@ -156,7 +156,7 @@ func (d *ResourceDetector) preemptPropagationPolicy(resourceTemplate *unstructur // preemptClusterPropagationPolicyDirectly directly preempts resource template claimed by ClusterPropagationPolicy regardless of priority. func (d *ResourceDetector) preemptClusterPropagationPolicyDirectly(resourceTemplate *unstructured.Unstructured, policy *policyv1alpha1.PropagationPolicy) (err error) { - claimedPolicyName := util.GetLabelValue(resourceTemplate.GetLabels(), policyv1alpha1.ClusterPropagationPolicyLabel) + claimedPolicyName := util.GetAnnotationValue(resourceTemplate.GetAnnotations(), policyv1alpha1.ClusterPropagationPolicyAnnotation) if claimedPolicyName == "" { return nil } @@ -184,7 +184,7 @@ func (d *ResourceDetector) preemptClusterPropagationPolicyDirectly(resourceTempl // preemptClusterPropagationPolicy preempts resource template that is claimed by ClusterPropagationPolicy. func (d *ResourceDetector) preemptClusterPropagationPolicy(resourceTemplate *unstructured.Unstructured, policy *policyv1alpha1.ClusterPropagationPolicy) (err error) { - claimedPolicyName := util.GetLabelValue(resourceTemplate.GetLabels(), policyv1alpha1.ClusterPropagationPolicyLabel) + claimedPolicyName := util.GetAnnotationValue(resourceTemplate.GetAnnotations(), policyv1alpha1.ClusterPropagationPolicyAnnotation) if claimedPolicyName == "" { return nil } diff --git a/pkg/karmadactl/promote/promote.go b/pkg/karmadactl/promote/promote.go index dd1e0604a63c..49769a7b3464 100644 --- a/pkg/karmadactl/promote/promote.go +++ b/pkg/karmadactl/promote/promote.go @@ -41,7 +41,6 @@ import ( configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" - workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" karmadaclientset "github.com/karmada-io/karmada/pkg/generated/clientset/versioned" "github.com/karmada-io/karmada/pkg/karmadactl/get" @@ -317,8 +316,7 @@ func (o *CommandPromoteOption) revertPromotedDeps(memberClusterFactory cmdutil.F return } // remove the karmada label of dependency resource in member cluster to avoid deleting it when deleting the resource in control plane - u.RemoveLabels(depObj, workv1alpha1.WorkNamespaceLabel) - u.RemoveLabels(depObj, workv1alpha1.WorkNameLabel) + u.RemoveLabels(depObj, workv1alpha2.WorkPermanentIDLabel) if len(depObj.GetNamespace()) != 0 { // update the dependency resource in member cluster _, err := memberDynamicClient.Resource(depGvr).Namespace(depObj.GetNamespace()).Update(context.Background(), depObj, metav1.UpdateOptions{}) diff --git a/pkg/scheduler/event_handler.go b/pkg/scheduler/event_handler.go index 4b19649796fb..c400eae5eee4 100644 --- a/pkg/scheduler/event_handler.go +++ b/pkg/scheduler/event_handler.go @@ -107,8 +107,8 @@ func (s *Scheduler) resourceBindingEventFilter(obj interface{}) bool { } } - return util.GetLabelValue(accessor.GetLabels(), policyv1alpha1.PropagationPolicyNameLabel) != "" || - util.GetLabelValue(accessor.GetLabels(), policyv1alpha1.ClusterPropagationPolicyLabel) != "" || + return util.GetLabelValue(accessor.GetLabels(), policyv1alpha1.PropagationPolicyPermanentIDLabel) != "" || + util.GetLabelValue(accessor.GetLabels(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel) != "" || util.GetLabelValue(accessor.GetLabels(), workv1alpha2.BindingManagedByLabel) != "" } diff --git a/pkg/util/annotation.go b/pkg/util/annotation.go index 47e9f2056b38..12ab2f9f03de 100644 --- a/pkg/util/annotation.go +++ b/pkg/util/annotation.go @@ -20,6 +20,7 @@ import ( "sort" "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" @@ -125,3 +126,16 @@ func DedupeAndMergeAnnotations(existAnnotation, newAnnotation map[string]string) } return existAnnotation } + +// RemoveAnnotations removes the annotations from the given object. +func RemoveAnnotations(obj metav1.Object, keys ...string) { + if len(keys) == 0 { + return + } + + objAnnotations := obj.GetAnnotations() + for _, key := range keys { + delete(objAnnotations, key) + } + obj.SetAnnotations(objAnnotations) +} 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 06d50aa0c53e..e9c7ebd2f96f 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. @@ -116,37 +115,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 get 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} + ls = labels.Set{workv1alpha2.ClusterResourceBindingPermanentIDLabel: bindingID} } - 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 + 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..2f4fa3ed3973 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 } diff --git a/pkg/util/label.go b/pkg/util/label.go index a0f78db4728e..61368a1cf9ea 100644 --- a/pkg/util/label.go +++ b/pkg/util/label.go @@ -20,6 +20,7 @@ import ( "sort" "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" @@ -77,7 +78,7 @@ func MergeLabel(obj *unstructured.Unstructured, labelKey string, labelValue stri } // RemoveLabels removes the labels from the given object. -func RemoveLabels(obj *unstructured.Unstructured, labelKeys ...string) { +func RemoveLabels(obj metav1.Object, labelKeys ...string) { if len(labelKeys) == 0 { return } diff --git a/pkg/util/objectwatcher/objectwatcher.go b/pkg/util/objectwatcher/objectwatcher.go index d2dd5be30713..ee03a9dc65c2 100644 --- a/pkg/util/objectwatcher/objectwatcher.go +++ b/pkg/util/objectwatcher/objectwatcher.go @@ -29,7 +29,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" - 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/resourceinterpreter" "github.com/karmada-io/karmada/pkg/util" @@ -283,7 +282,7 @@ func (o *objectWatcherImpl) NeedsUpdate(clusterName string, desiredObj, clusterO func (o *objectWatcherImpl) allowUpdate(clusterName string, desiredObj, clusterObj *unstructured.Unstructured) bool { // If the existing resource is managed by Karmada, then the updating is allowed. - if util.GetLabelValue(desiredObj.GetLabels(), workv1alpha1.WorkNameLabel) == util.GetLabelValue(clusterObj.GetLabels(), workv1alpha1.WorkNameLabel) { + if util.GetLabelValue(desiredObj.GetLabels(), workv1alpha2.WorkPermanentIDLabel) == util.GetLabelValue(clusterObj.GetLabels(), workv1alpha2.WorkPermanentIDLabel) { return true } diff --git a/pkg/webhook/clusterpropagationpolicy/mutating.go b/pkg/webhook/clusterpropagationpolicy/mutating.go index 20d1933a657f..f52134f91524 100644 --- a/pkg/webhook/clusterpropagationpolicy/mutating.go +++ b/pkg/webhook/clusterpropagationpolicy/mutating.go @@ -22,9 +22,12 @@ import ( "fmt" "net/http" + "github.com/google/uuid" + admissionv1 "k8s.io/api/admission/v1" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" + "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/helper" "github.com/karmada-io/karmada/pkg/util/validation" ) @@ -80,6 +83,11 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm } } + if req.Operation == admissionv1.Create { + // We always generate a unique UUID for newly created policies. + policy.Labels = util.DedupeAndMergeLabels(policy.Labels, map[string]string{policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: uuid.New().String()}) + } + marshaledBytes, err := json.Marshal(policy) if err != nil { return admission.Errored(http.StatusInternalServerError, err) diff --git a/pkg/webhook/propagationpolicy/mutating.go b/pkg/webhook/propagationpolicy/mutating.go index 7bd5879a63a3..1830bc15924a 100644 --- a/pkg/webhook/propagationpolicy/mutating.go +++ b/pkg/webhook/propagationpolicy/mutating.go @@ -22,10 +22,13 @@ import ( "fmt" "net/http" + "github.com/google/uuid" + admissionv1 "k8s.io/api/admission/v1" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" + "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/helper" "github.com/karmada-io/karmada/pkg/util/validation" ) @@ -92,6 +95,11 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm } } + if req.Operation == admissionv1.Create { + // We always generate a unique UUID for newly created policies. + policy.Labels = util.DedupeAndMergeLabels(policy.Labels, map[string]string{policyv1alpha1.PropagationPolicyPermanentIDLabel: uuid.New().String()}) + } + marshaledBytes, err := json.Marshal(policy) if err != nil { return admission.Errored(http.StatusInternalServerError, err) diff --git a/pkg/webhook/work/mutating.go b/pkg/webhook/work/mutating.go index 4d788bdd57cc..c0dbf3fb990d 100644 --- a/pkg/webhook/work/mutating.go +++ b/pkg/webhook/work/mutating.go @@ -27,7 +27,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" 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/resourceinterpreter/default/native/prune" + "github.com/karmada-io/karmada/pkg/util" ) // MutatingAdmission mutates API request if necessary. @@ -63,6 +65,7 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm klog.Errorf("Failed to remove irrelevant field for work(%s): %v", work.Name, err) return admission.Errored(http.StatusInternalServerError, err) } + util.MergeLabel(workloadObj, workv1alpha2.WorkPermanentIDLabel, util.GetLabelValue(work.Labels, workv1alpha2.WorkPermanentIDLabel)) workloadJSON, err := workloadObj.MarshalJSON() if err != nil { diff --git a/test/e2e/clusterpropagationpolicy_test.go b/test/e2e/clusterpropagationpolicy_test.go index ff595e36e553..2f9536b1b8c2 100644 --- a/test/e2e/clusterpropagationpolicy_test.go +++ b/test/e2e/clusterpropagationpolicy_test.go @@ -251,7 +251,7 @@ var _ = ginkgo.Describe("[AdvancedClusterPropagation] propagation testing", func return true } - _, exist := deployment.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] + _, exist := deployment.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] return !exist }) }) @@ -336,7 +336,7 @@ var _ = ginkgo.Describe("[AdvancedClusterPropagation] propagation testing", func return true } - _, exist := clusterRole.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] + _, exist := clusterRole.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] return !exist }) }) @@ -377,9 +377,13 @@ var _ = ginkgo.Describe("[AdvancedClusterPropagation] propagation testing", func }) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().ClusterPropagationPolicies().Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policy.Name, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -399,9 +403,13 @@ var _ = ginkgo.Describe("[AdvancedClusterPropagation] propagation testing", func } framework.PatchClusterPropagationPolicy(karmadaClient, policy.Name, patch, types.JSONPatchType) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().ClusterPropagationPolicies().Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policy.Name, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -570,8 +578,8 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() { ginkgo.By("check whether the deployment uses the highest explicit priority ClusterPropagationPolicy", func() { framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - klog.Infof("Matched ClusterPropagationPolicy:%s", deployment.GetLabels()[policyv1alpha1.ClusterPropagationPolicyLabel]) - return deployment.GetLabels()[policyv1alpha1.ClusterPropagationPolicyLabel] == higherPriorityLabelSelector + klog.Infof("Matched ClusterPropagationPolicy:%s", deployment.GetAnnotations()[policyv1alpha1.ClusterPropagationPolicyAnnotation]) + return deployment.GetAnnotations()[policyv1alpha1.ClusterPropagationPolicyAnnotation] == higherPriorityLabelSelector }) }) }) @@ -640,8 +648,8 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() { ginkgo.By("check whether the deployment uses the ClusterPropagationPolicy with name matched", func() { framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - klog.Infof("Matched ClusterPropagationPolicy:%s", deployment.GetLabels()[policyv1alpha1.ClusterPropagationPolicyLabel]) - return deployment.GetLabels()[policyv1alpha1.ClusterPropagationPolicyLabel] == explicitPriorityMatchName + klog.Infof("Matched ClusterPropagationPolicy:%s", deployment.GetAnnotations()[policyv1alpha1.ClusterPropagationPolicyAnnotation]) + return deployment.GetAnnotations()[policyv1alpha1.ClusterPropagationPolicyAnnotation] == explicitPriorityMatchName }) }) }) @@ -683,9 +691,13 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { }) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().ClusterPropagationPolicies().Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: policy.Name, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -701,7 +713,7 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { if dep.Labels == nil { return true } - return dep.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] == "" && dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" }) resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name) @@ -709,7 +721,7 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { if resourceBinding.Labels == nil { return true } - return resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] == "" && resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" }) }) }) @@ -753,9 +765,13 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { framework.WaitCRDDisappearedOnClusters(framework.ClusterNames(), crd.Name) }) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().ClusterPropagationPolicies().Get(context.TODO(), crdPolicy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ClusterResourceBindings().List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.ClusterPropagationPolicyLabel: crdPolicy.Name, + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -771,7 +787,7 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { if crd.Labels == nil { return true } - return crd.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] == "" && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" }) resourceBindingName := names.GenerateBindingName(crd.Kind, crd.Name) @@ -779,7 +795,7 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() { if crb.Labels == nil { return true } - return crb.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] == "" && crb.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return crb.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" }) }) }) diff --git a/test/e2e/propagationpolicy_test.go b/test/e2e/propagationpolicy_test.go index c65321f605c6..1a83d5cff6e3 100644 --- a/test/e2e/propagationpolicy_test.go +++ b/test/e2e/propagationpolicy_test.go @@ -583,21 +583,21 @@ var _ = ginkgo.Describe("[ImplicitPriority] propagation testing", func() { defer framework.RemovePropagationPolicy(karmadaClient, policyMatchLabelSelector.Namespace, policyMatchName.Name) framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - return deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] == priorityMatchName + return deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation] == priorityMatchName }) }) ginkgo.By("check whether the deployment uses the highest priority propagationPolicy (priorityMatchLabel)", func() { defer framework.RemovePropagationPolicy(karmadaClient, policyMatchLabelSelector.Namespace, policyMatchLabelSelector.Name) framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - return deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] == priorityMatchLabelSelector + return deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation] == priorityMatchLabelSelector }) }) ginkgo.By("check whether the deployment uses the highest priority propagationPolicy (priorityMatchAll)", func() { defer framework.RemovePropagationPolicy(karmadaClient, policyMatchLabelSelector.Namespace, policyPriorityMatchAll.Name) framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - return deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] == priorityMatchAll + return deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation] == priorityMatchAll }) }) }) @@ -684,9 +684,9 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() { ginkgo.By("check whether the deployment uses the highest explicit priority PropagationPolicy", func() { framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - klog.Infof("Match PropagationPolicy:%s/%s", deployment.GetLabels()[policyv1alpha1.PropagationPolicyNamespaceLabel], - deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel]) - return deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] == higherPriorityLabelSelector + klog.Infof("Match PropagationPolicy:%s/%s", deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNamespaceAnnotation], + deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation]) + return deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation] == higherPriorityLabelSelector }) }) }) @@ -756,9 +756,9 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() { ginkgo.By("check whether the deployment uses the PropagationPolicy with name matched", func() { framework.WaitDeploymentPresentOnClustersFitWith(framework.ClusterNames(), deployment.Namespace, deployment.Name, func(deployment *appsv1.Deployment) bool { - klog.Infof("Match PropagationPolicy:%s/%s", deployment.GetLabels()[policyv1alpha1.PropagationPolicyNamespaceLabel], - deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel]) - return deployment.GetLabels()[policyv1alpha1.PropagationPolicyNameLabel] == explicitPriorityMatchName + klog.Infof("Match PropagationPolicy:%s/%s", deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNamespaceAnnotation], + deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation]) + return deployment.GetAnnotations()[policyv1alpha1.PropagationPolicyNameAnnotation] == explicitPriorityMatchName }) }) }) @@ -841,12 +841,12 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { func(deployment *appsv1.Deployment) bool { return true }) framework.WaitDeploymentGetByClientFitWith(kubeClient, deployment01.Namespace, deployment01.Name, func(deployment *appsv1.Deployment) bool { - if deployment.Labels == nil { + if deployment.Annotations == nil { return true } - _, namespaceExist := deployment.Labels[policyv1alpha1.PropagationPolicyNamespaceLabel] - _, nameExist := deployment.Labels[policyv1alpha1.PropagationPolicyNameLabel] + _, namespaceExist := deployment.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] + _, nameExist := deployment.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] if namespaceExist || nameExist { return false } @@ -889,10 +889,13 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { }) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().PropagationPolicies(policy.Namespace).Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policy.Namespace, - policyv1alpha1.PropagationPolicyNameLabel: policy.Name, + policyv1alpha1.PropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -912,10 +915,13 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { } framework.PatchPropagationPolicy(karmadaClient, policy.Namespace, policy.Name, patch, types.JSONPatchType) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().PropagationPolicies(policy.Namespace).Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policy.Namespace, - policyv1alpha1.PropagationPolicyNameLabel: policy.Name, + policyv1alpha1.PropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -977,10 +983,13 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { }) gomega.Eventually(func() bool { + observedPolicy, err := karmadaClient.PolicyV1alpha1().PropagationPolicies(policy.Namespace).Get(context.TODO(), policy.Name, metav1.GetOptions{}) + if err != nil { + return false + } bindings, err := karmadaClient.WorkV1alpha2().ResourceBindings(testNamespace).List(context.TODO(), metav1.ListOptions{ LabelSelector: labels.SelectorFromSet(labels.Set{ - policyv1alpha1.PropagationPolicyNamespaceLabel: policy.Namespace, - policyv1alpha1.PropagationPolicyNameLabel: policy.Name, + policyv1alpha1.PropagationPolicyPermanentIDLabel: observedPolicy.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel], }).String(), }) if err != nil { @@ -996,8 +1005,7 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { if dep.Labels == nil { return true } - return dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == "" && dep.Labels[policyv1alpha1.PropagationPolicyNameLabel] == "" && - dep.Labels[policyv1alpha1.PropagationPolicyNamespaceLabel] == "" + return dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == "" }) @@ -1006,8 +1014,7 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { if resourceBinding.Labels == nil { return true } - return resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == "" && resourceBinding.Labels[policyv1alpha1.PropagationPolicyNameLabel] == "" && - resourceBinding.Labels[policyv1alpha1.PropagationPolicyNamespaceLabel] == "" + return resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == "" }) }) })