From 48425663cc912a76fb8845e82161724248d1e3dc Mon Sep 17 00:00:00 2001 From: zhzhuang-zju Date: Sun, 28 Apr 2024 15:15:49 +0800 Subject: [PATCH] e2e: Remove annotaions when pp/cpp is deleted Signed-off-by: zhzhuang-zju --- pkg/detector/detector.go | 8 ++-- test/e2e/clusterpropagationpolicy_test.go | 44 ++++++++++++------- .../clusterpropagationpolicy_test.md | 8 ++-- .../coverage_docs/propagationpolicy_test.md | 22 ++++++---- test/e2e/propagationpolicy_test.go | 21 +++++---- 5 files changed, 62 insertions(+), 41 deletions(-) diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go index b0bfd790bbd4..54fdb680f73c 100644 --- a/pkg/detector/detector.go +++ b/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. -func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error { +// CleanupClusterResourceBindingMarks removes marks, such as labels and annotations, from cluster resource binding. +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) diff --git a/test/e2e/clusterpropagationpolicy_test.go b/test/e2e/clusterpropagationpolicy_test.go index 2f9536b1b8c2..e1d21f2d2173 100644 --- a/test/e2e/clusterpropagationpolicy_test.go +++ b/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 { - return true + if dep.Labels != nil && dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" { + return false + } + if dep.Annotations != nil && dep.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" { + return false } - return dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return true }) resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name) framework.WaitResourceBindingFitWith(karmadaClient, deployment.Namespace, resourceBindingName, func(resourceBinding *workv1alpha2.ResourceBinding) bool { - if resourceBinding.Labels == nil { - return true + if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" { + return false + } + if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" { + return false } - return resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + 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 { - return true + if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" { + return false + } + if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" { + return false } - return crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return true }) resourceBindingName := names.GenerateBindingName(crd.Kind, crd.Name) framework.WaitClusterResourceBindingFitWith(karmadaClient, resourceBindingName, func(crb *workv1alpha2.ClusterResourceBinding) bool { - if crb.Labels == nil { - return true + if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" { + return false + } + if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" { + return false } - return crb.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == "" + return true }) }) }) diff --git a/test/e2e/coverage_docs/clusterpropagationpolicy_test.md b/test/e2e/coverage_docs/clusterpropagationpolicy_test.md index 685db9c14928..b89e31886a92 100644 --- a/test/e2e/coverage_docs/clusterpropagationpolicy_test.md +++ b/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 delete clusterpropagationpolicy for deployment | delete ClusterPropagationPolicy and check whether labels are deleted correctly(namespace scope) | | -| Test delete clusterpropagationpolicy for CRD | delete ClusterPropagationPolicy and check whether labels are deleted correctly(cluster scope) | | +| Test Case | E2E Describe Text | Comments | +|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|------------| +| 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 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). diff --git a/test/e2e/coverage_docs/propagationpolicy_test.md b/test/e2e/coverage_docs/propagationpolicy_test.md index 74db94518cf4..e9faba074b7d 100644 --- a/test/e2e/coverage_docs/propagationpolicy_test.md +++ b/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 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 update propagateDeps of the propagationPolicy for the deployment | update policy propagateDeps | | -| Test update placement of the propagationPolicy for the deployment | update policy placement | | -| Test delete propagationpolicy for deployment | delete the propagationPolicy and check whether labels are deleted correctly | | -| 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 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 update propagateDeps of the propagationPolicy for the deployment | update policy propagateDeps | | +| Test update placement of the propagationPolicy for the deployment | update policy placement | | +| Test delete propagationpolicy for deployment | delete the propagationPolicy and check whether labels and annotations are deleted correctly | | +| 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 | | diff --git a/test/e2e/propagationpolicy_test.go b/test/e2e/propagationpolicy_test.go index 1a83d5cff6e3..85176cb975d1 100644 --- a/test/e2e/propagationpolicy_test.go +++ b/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 { - return true + if dep.Labels != nil && dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" { + 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 { - return true + if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" { + return false + } + if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] != "" && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] != "" { + return false } - return resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == "" + return true }) }) })