Merge pull request #4561 from zhzhuang-zju/annotation

e2e: Remove annotaions when pp/cpp is deleted

pkg/detector/detector.go

@@ -112,7 +112,7 @@ type ResourceDetector struct {
 
 	RESTMapper meta.RESTMapper
 
-	// waitingObjects tracks of objects which haven't be propagated yet as lack of appropriate policies.
+	// waitingObjects tracks of objects which haven't been propagated yet as lack of appropriate policies.
 	waitingObjects map[keys.ClusterWideKey]struct{}
 	// waitingLock is the lock for waitingObjects operation.
 	waitingLock sync.RWMutex
@@ -1242,7 +1242,7 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID strin
 			}
 
 			// 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 {
+			if err := d.CleanupClusterResourceBindingMarks(&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)
@@ -1459,8 +1459,8 @@ func (d *ResourceDetector) CleanupResourceBindingMarks(rb *workv1alpha2.Resource
 	})
 }
 
-// CleanupClusterResourceBindingLabels removes marks, such as labels and annotations, from cluster resource binding.
+// CleanupClusterResourceBindingMarks removes marks, such as labels and annotations, from cluster resource binding.
-func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
+func (d *ResourceDetector) CleanupClusterResourceBindingMarks(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
 	return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
 		cleanupFunc(crb)
 		updateErr := d.Client.Update(context.TODO(), crb)

test/e2e/clusterpropagationpolicy_test.go

@@ -659,7 +659,7 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() {
 // Delete when delete a clusterPropagationPolicy, and no more clusterPropagationPolicy matches the object, something like
 // labels should be cleaned.
 var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
-	ginkgo.Context("delete clusterPropagation and remove the labels from the resource template and reference binding", func() {
+	ginkgo.Context("delete clusterPropagation and remove the labels and annotations from the resource template and reference binding", func() {
 		var policy *policyv1alpha1.ClusterPropagationPolicy
 		var deployment *appsv1.Deployment
 		var targetMember string
@@ -707,26 +707,32 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
 			}, pollTimeout, pollInterval).Should(gomega.Equal(true))
 		})
 
-		ginkgo.It("delete ClusterPropagationPolicy and check whether labels are deleted correctly", func() {
+		ginkgo.It("delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly", func() {
 			framework.RemoveClusterPropagationPolicy(karmadaClient, policy.Name)
 			framework.WaitDeploymentFitWith(kubeClient, deployment.Namespace, deployment.Name, func(dep *appsv1.Deployment) bool {
-				if dep.Labels == nil {
+				if dep.Labels != nil && dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
+				if dep.Annotations != nil && dep.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
+					return false
+				}
+				return true
 			})
 
 			resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name)
 			framework.WaitResourceBindingFitWith(karmadaClient, deployment.Namespace, resourceBindingName, func(resourceBinding *workv1alpha2.ResourceBinding) bool {
-				if resourceBinding.Labels == nil {
+				if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
+				if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
+					return false
+				}
+				return true
 			})
 		})
 	})
 
-	ginkgo.Context("delete clusterPropagation and remove the labels from the resource template and reference clusterBinding", func() {
+	ginkgo.Context("delete clusterPropagation and remove the labels and annotations from the resource template and reference clusterBinding", func() {
 		var crdGroup string
 		var randStr string
 		var crdSpecNames apiextensionsv1.CustomResourceDefinitionNames
@@ -781,21 +787,27 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
 			}, pollTimeout, pollInterval).Should(gomega.Equal(true))
 		})
 
-		ginkgo.It("delete ClusterPropagationPolicy and check whether labels are deleted correctly", func() {
+		ginkgo.It("delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly", func() {
 			framework.RemoveClusterPropagationPolicy(karmadaClient, crdPolicy.Name)
 			framework.WaitCRDFitWith(dynamicClient, crd.Name, func(crd *apiextensionsv1.CustomResourceDefinition) bool {
-				if crd.Labels == nil {
+				if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
+				if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
+					return false
+				}
+				return true
 			})
 
 			resourceBindingName := names.GenerateBindingName(crd.Kind, crd.Name)
 			framework.WaitClusterResourceBindingFitWith(karmadaClient, resourceBindingName, func(crb *workv1alpha2.ClusterResourceBinding) bool {
-				if crb.Labels == nil {
+				if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return crb.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
+				if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
+					return false
+				}
+				return true
 			})
 		})
 	})

test/e2e/coverage_docs/clusterpropagationpolicy_test.md

@@ -25,10 +25,10 @@
 | Test the same explicit priority propagation for deployment                       | same explicit priority ClusterPropagationPolicy propagation testing                       | [Choose from same-priority PropagationPolicies](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#choose-from-same-priority-propagationpolicies)                                |
 
 #### Delete clusterPropagation testing
-| Test Case                                           | E2E Describe Text                                                                               | Comments   |
+| Test Case                                           | E2E Describe Text                                                                                               | Comments   |
-|-----------------------------------------------------|-------------------------------------------------------------------------------------------------|------------|
+|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|------------|
-| Test delete clusterpropagationpolicy for deployment | delete ClusterPropagationPolicy and check whether labels are deleted correctly(namespace scope) |            |
+| Test delete clusterpropagationpolicy for deployment | delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly(namespace scope) |            |
-| Test delete clusterpropagationpolicy for CRD        | delete ClusterPropagationPolicy and check whether labels are deleted correctly(cluster scope)   |            |
+| Test delete clusterpropagationpolicy for CRD        | delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly(cluster scope)   |            |
 
 #### TODO
 1. May need add the test case when the [deployment updates](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#update-deployment).

test/e2e/coverage_docs/propagationpolicy_test.md

@@ -1,6 +1,7 @@
 ### propagation policy e2e test coverage analysis
 
 #### Basic propagation testing
+
 | Test Case                                                      | E2E Describe Text                      | Comments                                                                                       |
 |----------------------------------------------------------------|----------------------------------------|------------------------------------------------------------------------------------------------|
 | Test the propagationPolicy for deployment                      | deployment propagation testing         | [Resource propagating](https://karmada.io/docs/next/userguide/scheduling/resource-propagating) |
@@ -12,23 +13,26 @@
 | Test the propagationPolicy for roleBinding                     | roleBinding propagation testing        |                                                                                                |
 
 #### ImplicitPriority propagation testing
+
 | Test Case                                                     | E2E Describe Text                                                                                                                | Comments                                                                                                                           |
 |---------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------|
 | priorityMatchName/priorityMatchLabel/priorityMatchAll testing | check whether the deployment uses the highest priority propagationPolicy (priorityMatchName/priorityMatchLabel/priorityMatchAll) | [Configure implicit priority](https://karmada.io/docs/next/userguide/scheduling/resource-propagating/#configure-implicit-priority) |
 
 #### ExplicitPriority propagation testing
+
 | Test Case                                                                        | E2E Describe Text                                                                  | Comments                                                                                                                                                              |
 |----------------------------------------------------------------------------------|------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | Test the high explicit/low priority/implicit priority propagation for deployment | high explicit/low priority/implicit priority PropagationPolicy propagation testing | [Configure explicit priority](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#configure-explicit-priority)                                     |
 | Test the same explicit priority propagation for deployment                       | same explicit priority PropagationPolicy propagation testing                       | [Choose from same-priority PropagationPolicies](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#choose-from-same-priority-propagationpolicies) |
 
 #### Advanced propagation testing
-| Test Case                                                                | E2E Describe Text                                                           | Comments                                                                                                                    |
+
-|--------------------------------------------------------------------------|-----------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
+| Test Case                                                                | E2E Describe Text                                                                           | Comments                                                                                                                    |
-| Test add resourceSelector of the propagationPolicy for the deployment    | add resourceSelectors item                                                  | [Update propagationPolicy](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#update-propagationpolicy) |
+|--------------------------------------------------------------------------|---------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
-| Test update resourceSelector of the propagationPolicy for the deployment | update resourceSelectors item                                               |                                                                                                                             |
+| Test add resourceSelector of the propagationPolicy for the deployment    | add resourceSelectors item                                                                  | [Update propagationPolicy](https://karmada.io/docs/next/userguide/scheduling/resource-propagating#update-propagationpolicy) |
-| Test update propagateDeps of the propagationPolicy for the deployment    | update policy propagateDeps                                                 |                                                                                                                             |
+| Test update resourceSelector of the propagationPolicy for the deployment | update resourceSelectors item                                                               |                                                                                                                             |
-| Test update placement of the propagationPolicy for the deployment        | update policy placement                                                     |                                                                                                                             |
+| Test update propagateDeps of the propagationPolicy for the deployment    | update policy propagateDeps                                                                 |                                                                                                                             |
-| Test delete propagationpolicy for deployment                             | delete the propagationPolicy and check whether labels are deleted correctly |                                                                                                                             |
+| Test update placement of the propagationPolicy for the deployment        | update policy placement                                                                     |                                                                                                                             |
-| Test modify the old propagationPolicy to unbind and create a new one     | modify the old propagationPolicy to unbind and create a new one             |                                                                                                                             |
+| Test delete propagationpolicy for deployment                             | delete the propagationPolicy and check whether labels and annotations are deleted correctly |                                                                                                                             |
-| Test delete the old propagationPolicy to unbind and create a new one     | delete the old propagationPolicy to unbind and create a new one             |                                                                                                                             |
+| Test modify the old propagationPolicy to unbind and create a new one     | modify the old propagationPolicy to unbind and create a new one                             |                                                                                                                             |
+| Test delete the old propagationPolicy to unbind and create a new one     | delete the old propagationPolicy to unbind and create a new one                             |                                                                                                                             |

test/e2e/propagationpolicy_test.go

@@ -999,22 +999,27 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() {
 			}, pollTimeout, pollInterval).Should(gomega.Equal(true))
 		})
 
-		ginkgo.It("delete the propagationPolicy and check whether labels are deleted correctly", func() {
+		ginkgo.It("delete the propagationPolicy and check whether labels and annotations are deleted correctly", func() {
 			framework.RemovePropagationPolicy(karmadaClient, policy.Namespace, policy.Name)
 			framework.WaitDeploymentFitWith(kubeClient, deployment.Namespace, deployment.Name, func(dep *appsv1.Deployment) bool {
-				if dep.Labels == nil {
+				if dep.Labels != nil && dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == ""
+				if dep.Annotations != nil && dep.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] != "" && dep.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] != "" {
-
+					return false
+				}
+				return true
 			})
 
 			resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name)
 			framework.WaitResourceBindingFitWith(karmadaClient, deployment.Namespace, resourceBindingName, func(resourceBinding *workv1alpha2.ResourceBinding) bool {
-				if resourceBinding.Labels == nil {
+				if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" {
-					return true
+					return false
 				}
-				return resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == ""
+				if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] != "" && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] != "" {
+					return false
+				}
+				return true
 			})
 		})
 	})