From 59b8835bda0710c42e2ae30f6c13c13f2a513480 Mon Sep 17 00:00:00 2001 From: whitewindmills Date: Mon, 8 Apr 2024 17:12:16 +0800 Subject: [PATCH] Deprecate name/namespace labels of pp/cpp Co-authored-by: changzhen Signed-off-by: whitewindmills --- .../policy/v1alpha1/well_known_constants.go | 30 ++- .../federatedhpa/federatedhpa_controller.go | 23 +- .../hpa_scale_target_marker_predicate.go | 4 +- pkg/detector/detector.go | 247 +++++++++--------- pkg/detector/policy.go | 107 ++++---- pkg/detector/preemption.go | 60 ++--- pkg/scheduler/event_handler.go | 4 +- pkg/util/annotation.go | 14 + pkg/util/label.go | 3 +- pkg/util/worker.go | 2 +- test/e2e/clusterpropagationpolicy_test.go | 44 +++- test/e2e/propagationpolicy_test.go | 51 ++-- 12 files changed, 302 insertions(+), 287 deletions(-) diff --git a/pkg/apis/policy/v1alpha1/well_known_constants.go b/pkg/apis/policy/v1alpha1/well_known_constants.go index bd7b554099c0..7e5c9129926d 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. @@ -31,6 +32,19 @@ const ( // In backup scenarios, when applying the backup resource manifest in a new cluster, the UUID may change. ClusterPropagationPolicyPermanentIDLabel = "clusterpropagationpolicy.karmada.io/permanent-id" + // 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: + // labels: + // namespace.karmada.io/skip-auto-propagation: "true" + // + // NOTE: If create a ns without this label, then patch it with this label, the ns will not be + // 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" @@ -39,23 +53,19 @@ const ( // ClusterPropagationPolicyAnnotation is added to objects to specify associated ClusterPropagationPolicy name. ClusterPropagationPolicyAnnotation = "clusterpropagationpolicy.karmada.io/name" +) +// TODO(whitewindmills): These deprecated labels will be removed in a future version. +const ( // PropagationPolicyNamespaceLabel is added to objects to specify associated PropagationPolicy namespace. + // Deprecated PropagationPolicyNamespaceLabel = "propagationpolicy.karmada.io/namespace" // PropagationPolicyNameLabel is added to objects to specify associated PropagationPolicy's name. + // Deprecated PropagationPolicyNameLabel = "propagationpolicy.karmada.io/name" // ClusterPropagationPolicyLabel is added to objects to specify associated ClusterPropagationPolicy. + // Deprecated 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: - // labels: - // namespace.karmada.io/skip-auto-propagation: "true" - // - // NOTE: If create a ns without this label, then patch it with this label, the ns will not be - // synced to new member clusters, but old member clusters still have it. - NamespaceSkipAutoPropagationLabel = "namespace.karmada.io/skip-auto-propagation" ) diff --git a/pkg/controllers/federatedhpa/federatedhpa_controller.go b/pkg/controllers/federatedhpa/federatedhpa_controller.go index 3bd24e6f773d..8d789e580cb6 100644 --- a/pkg/controllers/federatedhpa/federatedhpa_controller.go +++ b/pkg/controllers/federatedhpa/federatedhpa_controller.go @@ -404,25 +404,20 @@ 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 label") } - var policyName, policyNameSpace string var selector labels.Selector - if _, ok := resourceLabel[policyv1alpha1.PropagationPolicyNameLabel]; ok { - policyName = resourceLabel[policyv1alpha1.PropagationPolicyNameLabel] - policyNameSpace = resourceLabel[policyv1alpha1.PropagationPolicyNamespaceLabel] + if policyID, ok := resourceLabel[policyv1alpha1.PropagationPolicyPermanentIDLabel]; ok { 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 policyID, ok = resourceLabel[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel]; ok { 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{} @@ -432,7 +427,7 @@ func (c *FHPAController) getBindingByLabel(resourceLabel map[string]string, reso return nil, err } if len(bindingList.Items) == 0 { - return nil, fmt.Errorf("Length of binding list is zero. ") + return nil, errors.New("length of binding list is zero") } found := false @@ -444,7 +439,7 @@ func (c *FHPAController) getBindingByLabel(resourceLabel map[string]string, reso } } if !found { - return nil, fmt.Errorf("No binding matches the target resource. ") + return nil, errors.New("no binding matches the target resource") } return binding, nil @@ -452,7 +447,7 @@ func (c *FHPAController) getBindingByLabel(resourceLabel map[string]string, reso func (c *FHPAController) getTargetCluster(binding *workv1alpha2.ResourceBinding) ([]string, error) { if len(binding.Spec.Clusters) == 0 { - return nil, fmt.Errorf("Binding has no schedulable clusters. ") + return nil, errors.New("binding has no schedulable clusters") } var allClusters []string 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/detector/detector.go b/pkg/detector/detector.go index a5a46b9feab3..b0bfd790bbd4 100644 --- a/pkg/detector/detector.go +++ b/pkg/detector/detector.go @@ -63,6 +63,27 @@ import ( "github.com/karmada-io/karmada/pkg/util/restmapper" ) +var ( + propagationPolicyMarkedLabels = []string{ + policyv1alpha1.PropagationPolicyPermanentIDLabel, + // TODO(whitewindmills): Delete the following two lines in a future version. + policyv1alpha1.PropagationPolicyNamespaceLabel, + policyv1alpha1.PropagationPolicyNameLabel, + } + propagationPolicyMarkedAnnotations = []string{ + policyv1alpha1.PropagationPolicyNamespaceAnnotation, + policyv1alpha1.PropagationPolicyNameAnnotation, + } + clusterPropagationPolicyMarkedLabels = []string{ + policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel, + // TODO(whitewindmills): Delete the following line in a future version. + policyv1alpha1.ClusterPropagationPolicyLabel, + } + 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. @@ -136,7 +157,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, nil) d.InformerManager.ForResource(propagationPolicyGVR, policyHandler) @@ -146,7 +167,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, nil) d.InformerManager.ForResource(clusterPropagationPolicyGVR, clusterPolicyHandler) @@ -436,7 +457,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 } @@ -450,6 +471,7 @@ func (d *ResourceDetector) ApplyPolicy(object *unstructured.Unstructured, object } policyLabels := map[string]string{ + // TODO(whitewindmills): Delete the following two lines in a future version. policyv1alpha1.PropagationPolicyNamespaceLabel: policy.GetNamespace(), policyv1alpha1.PropagationPolicyNameLabel: policy.GetName(), policyv1alpha1.PropagationPolicyPermanentIDLabel: policyID, @@ -459,7 +481,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 @@ -547,6 +569,7 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, } policyLabels := map[string]string{ + // TODO(whitewindmills): Delete the following line in a future version. policyv1alpha1.ClusterPropagationPolicyLabel: policy.GetName(), policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel: policyID, } @@ -558,7 +581,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 @@ -592,10 +615,7 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, bindingCopy.Spec.ConflictResolution = binding.Spec.ConflictResolution return nil }) - if err != nil { - return err - } - return nil + return err }) if err != nil { @@ -611,7 +631,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 @@ -646,6 +666,7 @@ func (d *ResourceDetector) ApplyClusterPolicy(object *unstructured.Unstructured, }) return err }) + if err != nil { klog.Errorf("Failed to apply cluster policy(%s) for object: %s. error: %v", policy.Name, objectKey, err) return err @@ -731,6 +752,7 @@ func (d *ResourceDetector) ClaimPolicyForObject(object *unstructured.Unstructure } } + // TODO(whitewindmills): Delete the following two lines in a future version. objLabels[policyv1alpha1.PropagationPolicyNamespaceLabel] = policy.Namespace objLabels[policyv1alpha1.PropagationPolicyNameLabel] = policy.Name objLabels[policyv1alpha1.PropagationPolicyPermanentIDLabel] = policyID @@ -776,7 +798,7 @@ func (d *ResourceDetector) ClaimClusterPolicyForObject(object *unstructured.Unst } objectCopy := object.DeepCopy() - + // TODO(whitewindmills): Delete the following line in a future version. util.MergeLabel(objectCopy, policyv1alpha1.ClusterPropagationPolicyLabel, policy.Name) util.MergeLabel(objectCopy, policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel, policyID) @@ -785,7 +807,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{ @@ -793,7 +815,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, @@ -830,14 +852,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, @@ -925,24 +947,12 @@ func (d *ResourceDetector) GetMatching(resourceSelectors []policyv1alpha1.Resour // OnPropagationPolicyAdd handles object add event and push the object to queue. func (d *ResourceDetector) OnPropagationPolicyAdd(obj interface{}) { - key, err := ClusterWideKeyFunc(obj) - if err != nil { - return - } - - klog.V(2).Infof("Create PropagationPolicy(%s)", key) - d.policyReconcileWorker.Add(key) + d.policyReconcileWorker.Enqueue(obj) } // OnPropagationPolicyUpdate handles object update event and push the object to queue. func (d *ResourceDetector) OnPropagationPolicyUpdate(oldObj, newObj interface{}) { - key, err := ClusterWideKeyFunc(newObj) - if err != nil { - return - } - - klog.V(2).Infof("Update PropagationPolicy(%s)", key) - d.policyReconcileWorker.Add(key) + d.policyReconcileWorker.Enqueue(newObj) // Temporary solution of corner case: After the priority(.spec.priority) of // PropagationPolicy changed from high priority (e.g. 5) to low priority(e.g. 3), @@ -962,7 +972,7 @@ func (d *ResourceDetector) OnPropagationPolicyUpdate(oldObj, newObj interface{}) var unstructuredOldObj *unstructured.Unstructured var unstructuredNewObj *unstructured.Unstructured - unstructuredOldObj, err = helper.ToUnstructured(oldObj) + unstructuredOldObj, err := helper.ToUnstructured(oldObj) if err != nil { klog.Errorf("Failed to transform oldObj, error: %v", err) return @@ -1020,7 +1030,7 @@ func (d *ResourceDetector) ReconcilePropagationPolicy(key util.QueueKey) error { if !propagationObject.DeletionTimestamp.IsZero() { klog.Infof("PropagationPolicy(%s) is being deleted.", ckey.NamespaceKey()) - if err = d.HandlePropagationPolicyDeletion(propagationObject.Namespace, propagationObject.Name); err != nil { + if err = d.HandlePropagationPolicyDeletion(propagationObject.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel]); err != nil { return err } if controllerutil.RemoveFinalizer(propagationObject, util.PropagationPolicyControllerFinalizer) { @@ -1048,24 +1058,12 @@ func (d *ResourceDetector) ReconcilePropagationPolicy(key util.QueueKey) error { // OnClusterPropagationPolicyAdd handles object add event and push the object to queue. func (d *ResourceDetector) OnClusterPropagationPolicyAdd(obj interface{}) { - key, err := ClusterWideKeyFunc(obj) - if err != nil { - return - } - - klog.V(2).Infof("Create ClusterPropagationPolicy(%s)", key) - d.clusterPolicyReconcileWorker.Add(key) + d.clusterPolicyReconcileWorker.Enqueue(obj) } // OnClusterPropagationPolicyUpdate handles object update event and push the object to queue. func (d *ResourceDetector) OnClusterPropagationPolicyUpdate(oldObj, newObj interface{}) { - key, err := ClusterWideKeyFunc(newObj) - if err != nil { - return - } - - klog.V(2).Infof("Update ClusterPropagationPolicy(%s)", key) - d.clusterPolicyReconcileWorker.Add(key) + d.clusterPolicyReconcileWorker.Enqueue(newObj) // Temporary solution of corner case: After the priority(.spec.priority) of // ClusterPropagationPolicy changed from high priority (e.g. 5) to low priority(e.g. 3), @@ -1085,7 +1083,7 @@ func (d *ResourceDetector) OnClusterPropagationPolicyUpdate(oldObj, newObj inter var unstructuredOldObj *unstructured.Unstructured var unstructuredNewObj *unstructured.Unstructured - unstructuredOldObj, err = helper.ToUnstructured(oldObj) + unstructuredOldObj, err := helper.ToUnstructured(oldObj) if err != nil { klog.Errorf("Failed to transform oldObj, error: %v", err) return @@ -1144,7 +1142,7 @@ func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey) if !propagationObject.DeletionTimestamp.IsZero() { klog.Infof("ClusterPropagationPolicy(%s) is being deleted.", ckey.NamespaceKey()) - if err = d.HandleClusterPropagationPolicyDeletion(propagationObject.Name); err != nil { + if err = d.HandleClusterPropagationPolicyDeletion(propagationObject.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel]); err != nil { return err } if controllerutil.RemoveFinalizer(propagationObject, util.ClusterPropagationPolicyControllerFinalizer) { @@ -1171,75 +1169,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) } } @@ -1248,25 +1253,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) } } @@ -1283,7 +1288,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 } @@ -1291,7 +1297,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 } @@ -1340,12 +1346,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 } @@ -1353,11 +1360,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 } @@ -1403,11 +1410,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 } @@ -1416,7 +1423,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 { @@ -1433,15 +1440,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 @@ -1449,7 +1451,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) } @@ -1457,15 +1459,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 @@ -1473,7 +1470,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/policy.go b/pkg/detector/policy.go index 8b13be932258..e7bb3a6ad837 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.removeResourceMarksIfNotMatched(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.removeResourceMarksIfNotMatched(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) removeResourceMarksIfNotMatched(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,13 @@ 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.ClusterPropagationPolicyPermanentIDLabel) + // TODO(whitewindmills): Delete the following line in a future version. delete(objLabels, policyv1alpha1.ClusterPropagationPolicyLabel) return true } diff --git a/pkg/detector/preemption.go b/pkg/detector/preemption.go index 3defea7bb9cb..bbbf14ecb1bf 100755 --- a/pkg/detector/preemption.go +++ b/pkg/detector/preemption.go @@ -23,6 +23,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" @@ -37,7 +38,7 @@ import ( // PriorityKey is the unique propagation policy key with priority. type PriorityKey struct { - util.QueueKey + runtime.Object // Priority is the priority of the propagation policy. Priority int32 } @@ -54,7 +55,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 +105,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 +157,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 +185,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 } @@ -265,11 +266,14 @@ func (d *ResourceDetector) HandleDeprioritizedPropagationPolicy(oldPolicy policy klog.Errorf("Failed to list PropagationPolicy from namespace: %s, error: %v", newPolicy.GetNamespace(), err) return } + if len(policies) == 0 { + klog.Infof("No PropagationPolicy to preempt the PropagationPolicy(%s/%s).", newPolicy.GetNamespace(), newPolicy.GetName()) + } // Use the priority queue to sort the listed policies to ensure the // higher priority PropagationPolicy be process first to avoid possible // multiple preemption. - var sortedPotentialKeys *pq.Queue + sortedPotentialKeys := pq.NewWith(priorityDescendingComparator) for i := range policies { var potentialPolicy policyv1alpha1.PropagationPolicy if err = helper.ConvertToTypedObject(policies[i], &potentialPolicy); err != nil { @@ -286,19 +290,9 @@ func (d *ResourceDetector) HandleDeprioritizedPropagationPolicy(oldPolicy policy potentialPolicy.Spec.Preemption == policyv1alpha1.PreemptAlways && potentialPolicy.ExplicitPriority() > newPolicy.ExplicitPriority() && potentialPolicy.ExplicitPriority() < oldPolicy.ExplicitPriority() { - potentialKey, err := ClusterWideKeyFunc(&potentialPolicy) - if err != nil { - klog.Errorf("Failed to convert PropagationPolicy to queued key: %v", err) - continue - } - - klog.Infof("Enqueuing PropagationPolicy(%s/%s) in case of PropagationPolicy(%s/%s) priority changes", potentialPolicy.GetNamespace(), potentialPolicy.GetName(), newPolicy.GetNamespace(), newPolicy.GetName()) - if sortedPotentialKeys == nil { - sortedPotentialKeys = pq.NewWith(priorityDescendingComparator) - } - + klog.Infof("Enqueuing PropagationPolicy(%s/%s) in case of PropagationPolicy(%s/%s) priority changes.", potentialPolicy.GetNamespace(), potentialPolicy.GetName(), newPolicy.GetNamespace(), newPolicy.GetName()) sortedPotentialKeys.Enqueue(&PriorityKey{ - QueueKey: potentialKey, + Object: &potentialPolicy, Priority: potentialPolicy.ExplicitPriority(), }) } @@ -318,11 +312,14 @@ func (d *ResourceDetector) HandleDeprioritizedClusterPropagationPolicy(oldPolicy klog.Errorf("Failed to list ClusterPropagationPolicy, error: %v", err) return } + if len(policies) == 0 { + klog.Infof("No ClusterPropagationPolicy to preempt the ClusterPropagationPolicy(%s).", newPolicy.GetName()) + } // Use the priority queue to sort the listed policies to ensure the // higher priority ClusterPropagationPolicy be process first to avoid possible // multiple preemption. - var sortedPotentialKeys *pq.Queue + sortedPotentialKeys := pq.NewWith(priorityDescendingComparator) for i := range policies { var potentialPolicy policyv1alpha1.ClusterPropagationPolicy if err = helper.ConvertToTypedObject(policies[i], &potentialPolicy); err != nil { @@ -339,20 +336,10 @@ func (d *ResourceDetector) HandleDeprioritizedClusterPropagationPolicy(oldPolicy potentialPolicy.Spec.Preemption == policyv1alpha1.PreemptAlways && potentialPolicy.ExplicitPriority() > newPolicy.ExplicitPriority() && potentialPolicy.ExplicitPriority() < oldPolicy.ExplicitPriority() { - potentialKey, err := ClusterWideKeyFunc(&potentialPolicy) - if err != nil { - klog.Errorf("Failed to convert ClusterPropagationPolicy to queued key: %v", err) - continue - } - - klog.Infof("Enqueuing ClusterPropagationPolicy(%s) in case of ClusterPropagationPolicy(%s) priority changes", + klog.Infof("Enqueuing ClusterPropagationPolicy(%s) in case of ClusterPropagationPolicy(%s) priority changes.", potentialPolicy.GetName(), newPolicy.GetName()) - if sortedPotentialKeys == nil { - sortedPotentialKeys = pq.NewWith(priorityDescendingComparator) - } - sortedPotentialKeys.Enqueue(&PriorityKey{ - QueueKey: potentialKey, + Object: &potentialPolicy, Priority: potentialPolicy.ExplicitPriority(), }) } @@ -362,18 +349,13 @@ func (d *ResourceDetector) HandleDeprioritizedClusterPropagationPolicy(oldPolicy // requeuePotentialKeys re-queues potential policy keys. func requeuePotentialKeys(sortedPotentialKeys *pq.Queue, worker util.AsyncWorker) { - // No suitable policy key to re-queue. - if sortedPotentialKeys == nil { - return - } - for { key, ok := sortedPotentialKeys.Dequeue() if !ok { break } - worker.Add(key.(*PriorityKey).QueueKey) + worker.Enqueue(key.(*PriorityKey).Object) } } 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 c08aeafb4f31..f41e2bfeb16a 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" @@ -112,3 +113,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/label.go b/pkg/util/label.go index 097ef60b55a1..e626de7440a2 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" @@ -74,7 +75,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/worker.go b/pkg/util/worker.go index cbd20f1be827..a8663351347b 100644 --- a/pkg/util/worker.go +++ b/pkg/util/worker.go @@ -92,7 +92,7 @@ func NewAsyncWorker(opt Options) AsyncWorker { func (w *asyncWorker) Enqueue(obj interface{}) { key, err := w.keyFunc(obj) if err != nil { - klog.Warningf("Failed to generate key for obj: %+v", obj) + klog.Errorf("Failed to generate key for obj: %+v, err: %v", obj, err) return } 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] == "" }) }) })