From 1a7409d81578e3cbdd9ee8a37fab7d5674f4f99a Mon Sep 17 00:00:00 2001 From: Amir Alavi Date: Sat, 26 Oct 2024 12:26:12 -0400 Subject: [PATCH] chore: unify karmada labels/annotations usage in execution controller and test Signed-off-by: Amir Alavi --- .../execution/execution_controller.go | 20 +------ pkg/util/constants.go | 25 +++++++++ test/e2e/migration_and_rollback_test.go | 56 ++++++------------- 3 files changed, 44 insertions(+), 57 deletions(-) diff --git a/pkg/controllers/execution/execution_controller.go b/pkg/controllers/execution/execution_controller.go index 6c104699a231..813e140a4930 100644 --- a/pkg/controllers/execution/execution_controller.go +++ b/pkg/controllers/execution/execution_controller.go @@ -40,7 +40,6 @@ import ( clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1" workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1" - workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" "github.com/karmada-io/karmada/pkg/detector" "github.com/karmada-io/karmada/pkg/events" "github.com/karmada-io/karmada/pkg/metrics" @@ -209,23 +208,8 @@ func (c *Controller) cleanupPolicyClaimMetadata(ctx context.Context, work *workv } else { detector.CleanupPPClaimMetadata(workload) } - util.RemoveLabels( - workload, - workv1alpha2.ResourceBindingPermanentIDLabel, - workv1alpha2.WorkPermanentIDLabel, - util.ManagedByKarmadaLabel, - ) - util.RemoveAnnotations( - workload, - workv1alpha2.ManagedAnnotation, - workv1alpha2.ManagedLabels, - workv1alpha2.ResourceBindingNamespaceAnnotationKey, - workv1alpha2.ResourceBindingNameAnnotationKey, - workv1alpha2.ResourceTemplateUIDAnnotation, - workv1alpha2.ResourceTemplateGenerationAnnotationKey, - workv1alpha2.WorkNameAnnotation, - workv1alpha2.WorkNamespaceAnnotation, - ) + util.RemoveLabels(workload, util.ManagedResourceLabels...) + util.RemoveAnnotations(workload, util.ManagedResourceAnnotations...) if err := c.ObjectWatcher.Update(ctx, cluster.Name, workload, clusterObj); err != nil { klog.Errorf("Failed to update metadata in the given member cluster %v, err is %v", cluster.Name, err) diff --git a/pkg/util/constants.go b/pkg/util/constants.go index 6204b76d4466..6a7b9d1d463f 100644 --- a/pkg/util/constants.go +++ b/pkg/util/constants.go @@ -20,6 +20,8 @@ import ( "time" discoveryv1 "k8s.io/api/discovery/v1" + + workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" ) // Define labels used by karmada system. @@ -252,3 +254,26 @@ const ( // DefaultFilePerm default file perm DefaultFilePerm = 0640 ) + +var ( + // ManagedResourceLabels is the list of labels that are applied to + // resources in member clusters. + ManagedResourceLabels = []string{ + workv1alpha2.ResourceBindingPermanentIDLabel, + workv1alpha2.WorkPermanentIDLabel, + ManagedByKarmadaLabel, + } + + // ManagedResourceAnnotations is the list of annotations that are applied to + // resources in member clusters. + ManagedResourceAnnotations = []string{ + workv1alpha2.ManagedAnnotation, + workv1alpha2.ManagedLabels, + workv1alpha2.ResourceBindingNamespaceAnnotationKey, + workv1alpha2.ResourceBindingNameAnnotationKey, + workv1alpha2.ResourceTemplateUIDAnnotation, + workv1alpha2.ResourceTemplateGenerationAnnotationKey, + workv1alpha2.WorkNameAnnotation, + workv1alpha2.WorkNamespaceAnnotation, + } +) diff --git a/test/e2e/migration_and_rollback_test.go b/test/e2e/migration_and_rollback_test.go index 80f301b63dfc..eaff50ea59ad 100644 --- a/test/e2e/migration_and_rollback_test.go +++ b/test/e2e/migration_and_rollback_test.go @@ -34,7 +34,7 @@ import ( policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" - "github.com/karmada-io/karmada/pkg/util" + pkgutil "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/names" "github.com/karmada-io/karmada/test/e2e/framework" "github.com/karmada-io/karmada/test/helper" @@ -43,17 +43,6 @@ import ( var _ = ginkgo.Describe("Seamless migration and rollback testing", func() { var member1 string var member1Client kubernetes.Interface - karmadaLabels := []string{workv1alpha2.ResourceBindingPermanentIDLabel, workv1alpha2.WorkPermanentIDLabel, util.ManagedByKarmadaLabel} - karmadaAnnotations := []string{ - workv1alpha2.ManagedAnnotation, - workv1alpha2.ManagedLabels, - workv1alpha2.ResourceBindingNamespaceAnnotationKey, - workv1alpha2.ResourceBindingNameAnnotationKey, - workv1alpha2.ResourceTemplateUIDAnnotation, - workv1alpha2.ResourceTemplateGenerationAnnotationKey, - workv1alpha2.WorkNameAnnotation, - workv1alpha2.WorkNamespaceAnnotation, - } ginkgo.BeforeEach(func() { member1 = framework.ClusterNames()[0] @@ -144,12 +133,7 @@ var _ = ginkgo.Describe("Seamless migration and rollback testing", func() { framework.WaitForWorkToDisappear(karmadaClient, workNamespace, workName) // Check member cluster resource is preserved - framework.WaitDeploymentPresentOnClusterFitWith(member1, deployment.Namespace, deployment.Name, - func(memberDeploy *appsv1.Deployment) bool { - // ensure resource exist in member cluster while related labels/annotations is cleared - return allFlagsCleared(memberDeploy.Labels, karmadaLabels) && - allFlagsCleared(memberDeploy.Annotations, karmadaAnnotations) - }) + framework.WaitDeploymentPresentOnClusterFitWith(member1, deployment.Namespace, deployment.Name, isResourceNotManagedByKarmada) }) }) @@ -225,12 +209,7 @@ var _ = ginkgo.Describe("Seamless migration and rollback testing", func() { framework.WaitForWorkToDisappear(karmadaClient, workNamespace, workName) // Check member cluster resource is preserved - framework.WaitClusterRolePresentOnClusterFitWith(member1, clusterRole.Name, - func(memberClusterRole *rbacv1.ClusterRole) bool { - // ensure resource exist in member cluster while related labels/annotations is cleared - return allFlagsCleared(memberClusterRole.Labels, karmadaLabels) && - allFlagsCleared(memberClusterRole.Annotations, karmadaAnnotations) - }) + framework.WaitClusterRolePresentOnClusterFitWith(member1, clusterRole.Name, isResourceNotManagedByKarmada) }) }) @@ -299,12 +278,7 @@ var _ = ginkgo.Describe("Seamless migration and rollback testing", func() { framework.WaitForWorkToDisappear(karmadaClient, workNamespace, workName) // Check member cluster resource is preserved - framework.WaitServicePresentOnClusterFitWith(member1, service.Namespace, service.Name, - func(memberService *corev1.Service) bool { - // ensure resource exist in member cluster while related labels/annotations is cleared - return allFlagsCleared(memberService.Labels, karmadaLabels) && - allFlagsCleared(memberService.Annotations, karmadaAnnotations) - }) + framework.WaitServicePresentOnClusterFitWith(member1, service.Namespace, service.Name, isResourceNotManagedByKarmada) }) }) }) @@ -390,22 +364,26 @@ var _ = ginkgo.Describe("Seamless migration and rollback testing", func() { framework.WaitForWorkToDisappear(karmadaClient, workNamespace, workName) // Check member cluster secret is preserved - framework.WaitSecretPresentOnClusterFitWith(member1, secret.Namespace, secret.Name, - func(memberSecret *corev1.Secret) bool { - // ensure resource exist in member cluster while related labels/annotations is cleared - return allFlagsCleared(memberSecret.Labels, karmadaLabels) && - allFlagsCleared(memberSecret.Annotations, karmadaAnnotations) - }) + framework.WaitSecretPresentOnClusterFitWith(member1, secret.Namespace, secret.Name, isResourceNotManagedByKarmada) }) }) }) }) -func allFlagsCleared(flags map[string]string, checkingKeys []string) bool { - for _, key := range checkingKeys { - if _, exist := flags[key]; exist { +// isResourceNotManagedByKarmada checks if resource is missing all karmada managed labels/annotations +// which indicates that it's not managed by Karmada. +func isResourceNotManagedByKarmada[T metav1.Object](obj T) bool { + for _, key := range pkgutil.ManagedResourceLabels { + if _, exist := obj.GetLabels()[key]; exist { return false } } + + for _, key := range pkgutil.ManagedResourceAnnotations { + if _, exist := obj.GetAnnotations()[key]; exist { + return false + } + } + return true }