Skip to content

Commit fff3699

Browse files
karmada-botkarmada-bot
authored
Merge pull request #4561 from zhzhuang-zju/annotation
e2e: Remove annotaions when pp/cpp is deleted
2 parents 81916d6 + 4842566 commit fff3699

File tree

5 files changed

+62
-41
lines changed

5 files changed

+62
-41
lines changed

‎pkg/detector/detector.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ type ResourceDetector struct {
112112

113113
RESTMapper meta.RESTMapper
114114

115-
// waitingObjects tracks of objects which haven't be propagated yet as lack of appropriate policies.
115+
// waitingObjects tracks of objects which haven't been propagated yet as lack of appropriate policies.
116116
waitingObjects map[keys.ClusterWideKey]struct{}
117117
// waitingLock is the lock for waitingObjects operation.
118118
waitingLock sync.RWMutex
@@ -1242,7 +1242,7 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyID strin
12421242
}
12431243

12441244
// Clean up the marks from the reference binding so that the Karmada scheduler won't reschedule the binding.
1245-
if err := d.CleanupClusterResourceBindingLabels(&crbs.Items[index], cleanupMarksFun); err != nil {
1245+
if err := d.CleanupClusterResourceBindingMarks(&crbs.Items[index], cleanupMarksFun); err != nil {
12461246
klog.Errorf("Failed to clean up marks from clusterResourceBinding(%s) when clusterPropagationPolicy removed, error: %v",
12471247
binding.Name, err)
12481248
errs = append(errs, err)
@@ -1459,8 +1459,8 @@ func (d *ResourceDetector) CleanupResourceBindingMarks(rb *workv1alpha2.Resource
14591459
})
14601460
}
14611461

1462-
// CleanupClusterResourceBindingLabels removes marks, such as labels and annotations, from cluster resource binding.
1463-
func (d *ResourceDetector) CleanupClusterResourceBindingLabels(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
1462+
// CleanupClusterResourceBindingMarks removes marks, such as labels and annotations, from cluster resource binding.
1463+
func (d *ResourceDetector) CleanupClusterResourceBindingMarks(crb *workv1alpha2.ClusterResourceBinding, cleanupFunc func(obj metav1.Object)) error {
14641464
return retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
14651465
cleanupFunc(crb)
14661466
updateErr := d.Client.Update(context.TODO(), crb)

‎test/e2e/clusterpropagationpolicy_test.go‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ var _ = ginkgo.Describe("[ExplicitPriority] propagation testing", func() {
659659
// Delete when delete a clusterPropagationPolicy, and no more clusterPropagationPolicy matches the object, something like
660660
// labels should be cleaned.
661661
var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
662-
ginkgo.Context("delete clusterPropagation and remove the labels from the resource template and reference binding", func() {
662+
ginkgo.Context("delete clusterPropagation and remove the labels and annotations from the resource template and reference binding", func() {
663663
var policy *policyv1alpha1.ClusterPropagationPolicy
664664
var deployment *appsv1.Deployment
665665
var targetMember string
@@ -707,26 +707,32 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
707707
}, pollTimeout, pollInterval).Should(gomega.Equal(true))
708708
})
709709

710-
ginkgo.It("delete ClusterPropagationPolicy and check whether labels are deleted correctly", func() {
710+
ginkgo.It("delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly", func() {
711711
framework.RemoveClusterPropagationPolicy(karmadaClient, policy.Name)
712712
framework.WaitDeploymentFitWith(kubeClient, deployment.Namespace, deployment.Name, func(dep *appsv1.Deployment) bool {
713-
if dep.Labels == nil {
714-
return true
713+
if dep.Labels != nil && dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
714+
return false
715+
}
716+
if dep.Annotations != nil && dep.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
717+
return false
715718
}
716-
return dep.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
719+
return true
717720
})
718721

719722
resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name)
720723
framework.WaitResourceBindingFitWith(karmadaClient, deployment.Namespace, resourceBindingName, func(resourceBinding *workv1alpha2.ResourceBinding) bool {
721-
if resourceBinding.Labels == nil {
722-
return true
724+
if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
725+
return false
726+
}
727+
if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
728+
return false
723729
}
724-
return resourceBinding.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
730+
return true
725731
})
726732
})
727733
})
728734

729-
ginkgo.Context("delete clusterPropagation and remove the labels from the resource template and reference clusterBinding", func() {
735+
ginkgo.Context("delete clusterPropagation and remove the labels and annotations from the resource template and reference clusterBinding", func() {
730736
var crdGroup string
731737
var randStr string
732738
var crdSpecNames apiextensionsv1.CustomResourceDefinitionNames
@@ -781,21 +787,27 @@ var _ = ginkgo.Describe("[Delete] clusterPropagation testing", func() {
781787
}, pollTimeout, pollInterval).Should(gomega.Equal(true))
782788
})
783789

784-
ginkgo.It("delete ClusterPropagationPolicy and check whether labels are deleted correctly", func() {
790+
ginkgo.It("delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly", func() {
785791
framework.RemoveClusterPropagationPolicy(karmadaClient, crdPolicy.Name)
786792
framework.WaitCRDFitWith(dynamicClient, crd.Name, func(crd *apiextensionsv1.CustomResourceDefinition) bool {
787-
if crd.Labels == nil {
788-
return true
793+
if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
794+
return false
795+
}
796+
if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
797+
return false
789798
}
790-
return crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
799+
return true
791800
})
792801

793802
resourceBindingName := names.GenerateBindingName(crd.Kind, crd.Name)
794803
framework.WaitClusterResourceBindingFitWith(karmadaClient, resourceBindingName, func(crb *workv1alpha2.ClusterResourceBinding) bool {
795-
if crb.Labels == nil {
796-
return true
804+
if crd.Labels != nil && crd.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] != "" {
805+
return false
806+
}
807+
if crd.Annotations != nil && crd.Annotations[policyv1alpha1.ClusterPropagationPolicyAnnotation] != "" {
808+
return false
797809
}
798-
return crb.Labels[policyv1alpha1.ClusterPropagationPolicyPermanentIDLabel] == ""
810+
return true
799811
})
800812
})
801813
})

‎test/e2e/coverage_docs/clusterpropagationpolicy_test.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@
2525
| 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) |
2626

2727
#### Delete clusterPropagation testing
28-
| Test Case | E2E Describe Text | Comments |
29-
|-----------------------------------------------------|-------------------------------------------------------------------------------------------------|------------|
30-
| Test delete clusterpropagationpolicy for deployment | delete ClusterPropagationPolicy and check whether labels are deleted correctly(namespace scope) | |
31-
| Test delete clusterpropagationpolicy for CRD | delete ClusterPropagationPolicy and check whether labels are deleted correctly(cluster scope) | |
28+
| Test Case | E2E Describe Text | Comments |
29+
|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------------|------------|
30+
| Test delete clusterpropagationpolicy for deployment | delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly(namespace scope) | |
31+
| Test delete clusterpropagationpolicy for CRD | delete ClusterPropagationPolicy and check whether labels and annotations are deleted correctly(cluster scope) | |
3232

3333
#### TODO
3434
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‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
### propagation policy e2e test coverage analysis
22

33
#### Basic propagation testing
4+
45
| Test Case | E2E Describe Text | Comments |
56
|----------------------------------------------------------------|----------------------------------------|------------------------------------------------------------------------------------------------|
67
| Test the propagationPolicy for deployment | deployment propagation testing | [Resource propagating](https://karmada.io/docs/next/userguide/scheduling/resource-propagating) |
@@ -12,23 +13,26 @@
1213
| Test the propagationPolicy for roleBinding | roleBinding propagation testing | |
1314

1415
#### ImplicitPriority propagation testing
16+
1517
| Test Case | E2E Describe Text | Comments |
1618
|---------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------|
1719
| 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) |
1820

1921
#### ExplicitPriority propagation testing
22+
2023
| Test Case | E2E Describe Text | Comments |
2124
|----------------------------------------------------------------------------------|------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------|
2225
| 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) |
2326
| 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) |
2427

2528
#### Advanced propagation testing
26-
| Test Case | E2E Describe Text | Comments |
27-
|--------------------------------------------------------------------------|-----------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
28-
| 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) |
29-
| Test update resourceSelector of the propagationPolicy for the deployment | update resourceSelectors item | |
30-
| Test update propagateDeps of the propagationPolicy for the deployment | update policy propagateDeps | |
31-
| Test update placement of the propagationPolicy for the deployment | update policy placement | |
32-
| Test delete propagationpolicy for deployment | delete the propagationPolicy and check whether labels are deleted correctly | |
33-
| Test modify the old propagationPolicy to unbind and create a new one | modify the old propagationPolicy to unbind and create a new one | |
34-
| Test delete the old propagationPolicy to unbind and create a new one | delete the old propagationPolicy to unbind and create a new one | |
29+
30+
| Test Case | E2E Describe Text | Comments |
31+
|--------------------------------------------------------------------------|---------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
32+
| 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) |
33+
| Test update resourceSelector of the propagationPolicy for the deployment | update resourceSelectors item | |
34+
| Test update propagateDeps of the propagationPolicy for the deployment | update policy propagateDeps | |
35+
| Test update placement of the propagationPolicy for the deployment | update policy placement | |
36+
| Test delete propagationpolicy for deployment | delete the propagationPolicy and check whether labels and annotations are deleted correctly | |
37+
| Test modify the old propagationPolicy to unbind and create a new one | modify the old propagationPolicy to unbind and create a new one | |
38+
| 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‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -999,22 +999,27 @@ var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() {
999999
}, pollTimeout, pollInterval).Should(gomega.Equal(true))
10001000
})
10011001

1002-
ginkgo.It("delete the propagationPolicy and check whether labels are deleted correctly", func() {
1002+
ginkgo.It("delete the propagationPolicy and check whether labels and annotations are deleted correctly", func() {
10031003
framework.RemovePropagationPolicy(karmadaClient, policy.Namespace, policy.Name)
10041004
framework.WaitDeploymentFitWith(kubeClient, deployment.Namespace, deployment.Name, func(dep *appsv1.Deployment) bool {
1005-
if dep.Labels == nil {
1006-
return true
1005+
if dep.Labels != nil && dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" {
1006+
return false
10071007
}
1008-
return dep.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == ""
1009-
1008+
if dep.Annotations != nil && dep.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] != "" && dep.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] != "" {
1009+
return false
1010+
}
1011+
return true
10101012
})
10111013

10121014
resourceBindingName := names.GenerateBindingName(deployment.Kind, deployment.Name)
10131015
framework.WaitResourceBindingFitWith(karmadaClient, deployment.Namespace, resourceBindingName, func(resourceBinding *workv1alpha2.ResourceBinding) bool {
1014-
if resourceBinding.Labels == nil {
1015-
return true
1016+
if resourceBinding.Labels != nil && resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] != "" {
1017+
return false
1018+
}
1019+
if resourceBinding.Annotations != nil && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNamespaceAnnotation] != "" && resourceBinding.Annotations[policyv1alpha1.PropagationPolicyNameAnnotation] != "" {
1020+
return false
10161021
}
1017-
return resourceBinding.Labels[policyv1alpha1.PropagationPolicyPermanentIDLabel] == ""
1022+
return true
10181023
})
10191024
})
10201025
})

0 commit comments

Comments
 (0)