Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

e2e: Remove annotaions when pp/cpp is deleted #4561

Merged
merged 1 commit into from
Apr 28, 2024
Merged

e2e: Remove annotaions when pp/cpp is deleted #4561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/detector/detector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
zhzhuang-zju marked this conversation as resolved.
Show resolved Hide resolved
waitingObjects map[keys.ClusterWideKey]struct{}
// waitingLock is the lock for waitingObjects operation.
waitingLock sync.RWMutex
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
44 changes: 28 additions & 16 deletions test/e2e/clusterpropagationpolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
})
})
})
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/coverage_docs/clusterpropagationpolicy_test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
22 changes: 13 additions & 9 deletions test/e2e/coverage_docs/propagationpolicy_test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) |
Expand All @@ -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 | |
21 changes: 13 additions & 8 deletions test/e2e/propagationpolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
})
})
Expand Down
Loading