From 5a767f4f219c8181aaffeaa0b5fbfb732bd38c55 Mon Sep 17 00:00:00 2001 From: changzhen Date: Thu, 22 Sep 2022 17:06:36 +0800 Subject: [PATCH 1/2] allow to update the resourceSelector of PropagationPolicy/ClusterPropagationPolicy Signed-off-by: changzhen --- pkg/detector/detector.go | 51 +++++-- pkg/detector/policy.go | 124 ++++++++++++++++++ pkg/util/helper/binding.go | 16 +++ pkg/util/helper/binding_test.go | 70 ++++++++++ pkg/util/helper/policy.go | 3 - pkg/util/label.go | 17 ++- pkg/util/label_test.go | 103 +++++++++++++++ .../clusterpropagationpolicy/validating.go | 14 -- pkg/webhook/propagationpolicy/validating.go | 14 -- 9 files changed, 366 insertions(+), 46 deletions(-) diff --git a/pkg/detector/detector.go b/pkg/detector/detector.go index 5d77931d6..6603ab3b4 100644 --- a/pkg/detector/detector.go +++ b/pkg/detector/detector.go @@ -734,7 +734,13 @@ func (d *ResourceDetector) OnPropagationPolicyAdd(obj interface{}) { // OnPropagationPolicyUpdate handles object update event and push the object to queue. func (d *ResourceDetector) OnPropagationPolicyUpdate(oldObj, newObj interface{}) { - // currently do nothing, since a policy's resource selector can not be updated. + key, err := ClusterWideKeyFunc(newObj) + if err != nil { + return + } + + klog.V(2).Infof("Update PropagationPolicy(%s) resourceSelectors", key) + d.policyReconcileWorker.Add(key) } // OnPropagationPolicyDelete handles object delete event and push the object to queue. @@ -776,7 +782,7 @@ func (d *ResourceDetector) ReconcilePropagationPolicy(key util.QueueKey) error { klog.Errorf("Failed to convert PropagationPolicy(%s) from unstructured object: %v", ckey.NamespaceKey(), err) return err } - return d.HandlePropagationPolicyCreation(propagationObject) + return d.HandlePropagationPolicyCreationOrUpdate(propagationObject) } // OnClusterPropagationPolicyAdd handles object add event and push the object to queue. @@ -792,7 +798,13 @@ func (d *ResourceDetector) OnClusterPropagationPolicyAdd(obj interface{}) { // OnClusterPropagationPolicyUpdate handles object update event and push the object to queue. func (d *ResourceDetector) OnClusterPropagationPolicyUpdate(oldObj, newObj interface{}) { - // currently do nothing, since a policy's resource selector can not be updated. + key, err := ClusterWideKeyFunc(newObj) + if err != nil { + return + } + + klog.V(2).Infof("Update ClusterPropagationPolicy(%s) resourceSelectors", key) + d.clusterPolicyReconcileWorker.Add(key) } // OnClusterPropagationPolicyDelete handles object delete event and push the object to queue. @@ -835,7 +847,7 @@ func (d *ResourceDetector) ReconcileClusterPropagationPolicy(key util.QueueKey) klog.Errorf("Failed to convert ClusterPropagationPolicy(%s) from unstructured object: %v", ckey.NamespaceKey(), err) return err } - return d.HandleClusterPropagationPolicyCreation(propagationObject) + return d.HandleClusterPropagationPolicyCreationOrUpdate(propagationObject) } // HandlePropagationPolicyDeletion handles PropagationPolicy delete event. @@ -934,10 +946,17 @@ func (d *ResourceDetector) HandleClusterPropagationPolicyDeletion(policyName str return errors.NewAggregate(errs) } -// HandlePropagationPolicyCreation handles PropagationPolicy add event. -// When a new policy arrives, should check if object in waiting list matches the policy, if yes remove the object +// HandlePropagationPolicyCreationOrUpdate handles PropagationPolicy add and update event. +// When a new policy arrives, should first check whether existing objects are no longer +// matched by the current policy, if yes, clean the labels on the object. +// And then check if object in waiting list matches the policy, if yes remove the object // from waiting list and throw the object to it's reconcile queue. If not, do nothing. -func (d *ResourceDetector) HandlePropagationPolicyCreation(policy *policyv1alpha1.PropagationPolicy) error { +func (d *ResourceDetector) HandlePropagationPolicyCreationOrUpdate(policy *policyv1alpha1.PropagationPolicy) error { + err := d.cleanUnmatchedResourceBindings(policy.Namespace, policy.Name, policy.Spec.ResourceSelectors) + if err != nil { + return err + } + matchedKeys := d.GetMatching(policy.Spec.ResourceSelectors) klog.Infof("Matched %d resources by policy(%s/%s)", len(matchedKeys), policy.Namespace, policy.Name) @@ -958,10 +977,22 @@ func (d *ResourceDetector) HandlePropagationPolicyCreation(policy *policyv1alpha return nil } -// HandleClusterPropagationPolicyCreation handles ClusterPropagationPolicy add event. -// When a new policy arrives, should check if object in waiting list matches the policy, if yes remove the object +// HandleClusterPropagationPolicyCreationOrUpdate handles ClusterPropagationPolicy add and update event. +// When a new policy arrives, should first check whether existing objects are no longer +// matched by the current policy, if yes, clean the labels on the object. +// And then check if object in waiting list matches the policy, if yes remove the object // from waiting list and throw the object to it's reconcile queue. If not, do nothing. -func (d *ResourceDetector) HandleClusterPropagationPolicyCreation(policy *policyv1alpha1.ClusterPropagationPolicy) error { +func (d *ResourceDetector) HandleClusterPropagationPolicyCreationOrUpdate(policy *policyv1alpha1.ClusterPropagationPolicy) error { + err := d.cleanUnmatchedResourceBindings("", policy.Name, policy.Spec.ResourceSelectors) + if err != nil { + return err + } + + err = d.cleanUnmatchedClusterResourceBinding(policy.Name, policy.Spec.ResourceSelectors) + if err != nil { + return err + } + matchedKeys := d.GetMatching(policy.Spec.ResourceSelectors) klog.Infof("Matched %d resources by policy(%s)", len(matchedKeys), policy.Name) diff --git a/pkg/detector/policy.go b/pkg/detector/policy.go index bf018d32f..a3e887fac 100644 --- a/pkg/detector/policy.go +++ b/pkg/detector/policy.go @@ -1,11 +1,16 @@ package detector import ( + "context" "fmt" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1" workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" @@ -118,3 +123,122 @@ func (d *ResourceDetector) getAndApplyClusterPolicy(object *unstructured.Unstruc return d.ApplyClusterPolicy(object, objectKey, matchedClusterPropagationPolicy) } + +func (d *ResourceDetector) cleanUnmatchedResourceBindings(policyNamespace, policyName string, selectors []policyv1alpha1.ResourceSelector) error { + var ls labels.Set + var removeLabels []string + if len(policyNamespace) == 0 { + ls = labels.Set{policyv1alpha1.ClusterPropagationPolicyLabel: policyName} + removeLabels = []string{policyv1alpha1.ClusterPropagationPolicyLabel} + } else { + ls = labels.Set{ + policyv1alpha1.PropagationPolicyNamespaceLabel: policyNamespace, + policyv1alpha1.PropagationPolicyNameLabel: policyName, + } + removeLabels = []string{ + policyv1alpha1.PropagationPolicyNamespaceLabel, + policyv1alpha1.PropagationPolicyNameLabel, + } + } + + bindings := &workv1alpha2.ResourceBindingList{} + listOpt := &client.ListOptions{LabelSelector: labels.SelectorFromSet(ls)} + err := d.Client.List(context.TODO(), bindings, listOpt) + if err != nil { + klog.Errorf("Failed to list ResourceBinding with policy(%s/%s), error: %v", policyNamespace, policyName, err) + } + + var errs []error + for _, binding := range bindings.Items { + removed, err := d.removeResourceLabelsIfNotMatch(binding.Spec.Resource, selectors, removeLabels...) + if err != nil { + klog.Errorf("Failed to remove resource labels when resource not match with policy selectors, err: %v", err) + errs = append(errs, err) + continue + } + if !removed { + continue + } + + bindingCopy := binding.DeepCopy() + for _, l := range removeLabels { + delete(bindingCopy.Labels, l) + } + err = d.Client.Update(context.TODO(), bindingCopy) + if err != nil { + klog.Errorf("Failed to update resourceBinding(%s/%s), err: %v", binding.Namespace, binding.Name, err) + errs = append(errs, err) + } + } + + if len(errs) > 0 { + return errors.NewAggregate(errs) + } + return nil +} + +func (d *ResourceDetector) cleanUnmatchedClusterResourceBinding(policyName string, selectors []policyv1alpha1.ResourceSelector) error { + bindings := &workv1alpha2.ClusterResourceBindingList{} + listOpt := &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(labels.Set{ + policyv1alpha1.ClusterPropagationPolicyLabel: policyName, + })} + err := d.Client.List(context.TODO(), bindings, listOpt) + if err != nil { + klog.Errorf("Failed to list ClusterResourceBinding with policy(%s), error: %v", policyName, err) + } + + var errs []error + for _, binding := range bindings.Items { + removed, err := d.removeResourceLabelsIfNotMatch(binding.Spec.Resource, selectors, []string{policyv1alpha1.ClusterPropagationPolicyLabel}...) + if err != nil { + klog.Errorf("Failed to remove resource labels when resource not match with policy selectors, err: %v", err) + errs = append(errs, err) + continue + } + if !removed { + continue + } + + bindingCopy := binding.DeepCopy() + delete(bindingCopy.Labels, policyv1alpha1.ClusterPropagationPolicyLabel) + err = d.Client.Update(context.TODO(), bindingCopy) + if err != nil { + klog.Errorf("Failed to update clusterResourceBinding(%s), err: %v", binding.Name, err) + errs = append(errs, err) + } + } + + if len(errs) > 0 { + return errors.NewAggregate(errs) + } + return nil +} + +func (d *ResourceDetector) removeResourceLabelsIfNotMatch(objectReference workv1alpha2.ObjectReference, selectors []policyv1alpha1.ResourceSelector, labelKeys ...string) (bool, error) { + objectKey, err := helper.ConstructClusterWideKey(objectReference) + if err != nil { + return false, err + } + + object, err := d.GetUnstructuredObject(objectKey) + if err != nil { + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + } + + if util.ResourceMatchSelectors(object, selectors...) { + return false, nil + } + + for _, labelKey := range labelKeys { + util.RemoveLabel(object, labelKey) + } + err = d.Client.Update(context.TODO(), object) + if err != nil { + return false, err + } + return true, nil +} diff --git a/pkg/util/helper/binding.go b/pkg/util/helper/binding.go index 04be1c725..ebd080fd8 100644 --- a/pkg/util/helper/binding.go +++ b/pkg/util/helper/binding.go @@ -22,6 +22,7 @@ import ( workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" "github.com/karmada-io/karmada/pkg/util" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" + "github.com/karmada-io/karmada/pkg/util/fedinformer/keys" "github.com/karmada-io/karmada/pkg/util/names" "github.com/karmada-io/karmada/pkg/util/restmapper" ) @@ -246,3 +247,18 @@ func GenerateReplicaRequirements(podTemplate *corev1.PodTemplateSpec) *workv1alp return nil } + +// ConstructClusterWideKey construct resource ClusterWideKey from binding's objectReference. +func ConstructClusterWideKey(resource workv1alpha2.ObjectReference) (keys.ClusterWideKey, error) { + gv, err := schema.ParseGroupVersion(resource.APIVersion) + if err != nil { + return keys.ClusterWideKey{}, err + } + return keys.ClusterWideKey{ + Group: gv.Group, + Version: gv.Version, + Kind: resource.Kind, + Namespace: resource.Namespace, + Name: resource.Name, + }, nil +} diff --git a/pkg/util/helper/binding_test.go b/pkg/util/helper/binding_test.go index 0f475875a..bf4f86a12 100644 --- a/pkg/util/helper/binding_test.go +++ b/pkg/util/helper/binding_test.go @@ -21,6 +21,7 @@ import ( 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/util/fedinformer/genericmanager" + "github.com/karmada-io/karmada/pkg/util/fedinformer/keys" "github.com/karmada-io/karmada/pkg/util/gclient" "github.com/karmada-io/karmada/pkg/util/names" ) @@ -899,3 +900,72 @@ func TestGenerateReplicaRequirements(t *testing.T) { }) } } + +func TestConstructClusterWideKey(t *testing.T) { + type args struct { + resource workv1alpha2.ObjectReference + } + tests := []struct { + name string + args args + want keys.ClusterWideKey + wantErr bool + }{ + { + name: "wrong APIVersion", + args: args{resource: workv1alpha2.ObjectReference{ + APIVersion: "a/b/c", + Kind: "Foo", + Namespace: "test", + Name: "foo", + }}, + want: keys.ClusterWideKey{}, + wantErr: true, + }, + { + name: "APIVersion: v1", + args: args{resource: workv1alpha2.ObjectReference{ + APIVersion: "v1", + Kind: "Foo", + Namespace: "test", + Name: "foo", + }}, + want: keys.ClusterWideKey{ + Version: "v1", + Kind: "Foo", + Namespace: "test", + Name: "foo", + }, + wantErr: false, + }, + { + name: "APIVersion: test/v1", + args: args{resource: workv1alpha2.ObjectReference{ + APIVersion: "test/v1", + Kind: "Foo", + Namespace: "test", + Name: "foo", + }}, + want: keys.ClusterWideKey{ + Group: "test", + Version: "v1", + Kind: "Foo", + Namespace: "test", + Name: "foo", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ConstructClusterWideKey(tt.args.resource) + if (err != nil) != tt.wantErr { + t.Errorf("ConstructClusterWideKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ConstructClusterWideKey() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/util/helper/policy.go b/pkg/util/helper/policy.go index 73f330670..897e25f87 100644 --- a/pkg/util/helper/policy.go +++ b/pkg/util/helper/policy.go @@ -15,9 +15,6 @@ import ( "github.com/karmada-io/karmada/pkg/util/names" ) -// DenyReasonResourceSelectorsModify constructs a reason indicating that modify ResourceSelectors is not allowed. -const DenyReasonResourceSelectorsModify = "modify ResourceSelectors is forbidden" - // SetDefaultSpreadConstraints set default spread constraints if both 'SpreadByField' and 'SpreadByLabel' not set. func SetDefaultSpreadConstraints(spreadConstraints []policyv1alpha1.SpreadConstraint) { for i := range spreadConstraints { diff --git a/pkg/util/label.go b/pkg/util/label.go index 9d743b1f5..fe64309e1 100644 --- a/pkg/util/label.go +++ b/pkg/util/label.go @@ -15,12 +15,19 @@ func GetLabelValue(labels map[string]string, labelKey string) string { // MergeLabel adds label for the given object. func MergeLabel(obj *unstructured.Unstructured, labelKey string, labelValue string) { - workloadLabel := obj.GetLabels() - if workloadLabel == nil { - workloadLabel = make(map[string]string, 1) + labels := obj.GetLabels() + if labels == nil { + labels = make(map[string]string, 1) } - workloadLabel[labelKey] = labelValue - obj.SetLabels(workloadLabel) + labels[labelKey] = labelValue + obj.SetLabels(labels) +} + +// RemoveLabel removes the label from the given object. +func RemoveLabel(obj *unstructured.Unstructured, labelKey string) { + labels := obj.GetLabels() + delete(labels, labelKey) + obj.SetLabels(labels) } // DedupeAndMergeLabels merges the new labels into exist labels. diff --git a/pkg/util/label_test.go b/pkg/util/label_test.go index cc418a5bc..843cefaef 100644 --- a/pkg/util/label_test.go +++ b/pkg/util/label_test.go @@ -208,3 +208,106 @@ func TestDedupeAndMergeLabels(t *testing.T) { }) } } + +func TestRemoveLabel(t *testing.T) { + type args struct { + obj *unstructured.Unstructured + labelKey string + } + tests := []struct { + name string + args args + expected *unstructured.Unstructured + }{ + { + name: "nil object labels", + args: args{ + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + labelKey: "foo", + }, + expected: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + }, + { + name: "same labelKey", + args: args{ + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + "labels": map[string]interface{}{"foo": "bar"}, + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + labelKey: "foo", + }, + expected: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + "labels": map[string]interface{}{}, + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + }, + { + name: "different labelKey", + args: args{ + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + "labels": map[string]interface{}{"foo": "bar"}, + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + labelKey: "foo1", + }, + expected: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "demo-deployment", + "labels": map[string]interface{}{"foo": "bar"}, + }, + "spec": map[string]interface{}{ + "replicas": 2, + }}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RemoveLabel(tt.args.obj, tt.args.labelKey) + if !reflect.DeepEqual(tt.args.obj, tt.expected) { + t.Errorf("RemoveLabel() = %v, want %v", tt.args.obj, tt.expected) + } + }) + } +} diff --git a/pkg/webhook/clusterpropagationpolicy/validating.go b/pkg/webhook/clusterpropagationpolicy/validating.go index e05e1c824..cb6de13d1 100644 --- a/pkg/webhook/clusterpropagationpolicy/validating.go +++ b/pkg/webhook/clusterpropagationpolicy/validating.go @@ -4,8 +4,6 @@ import ( "context" "net/http" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -34,18 +32,6 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request) } klog.V(2).Infof("Validating ClusterPropagationPolicy(%s) for request: %s", policy.Name, req.Operation) - if req.Operation == admissionv1.Update { - oldPolicy := &policyv1alpha1.ClusterPropagationPolicy{} - err := v.decoder.DecodeRaw(req.OldObject, oldPolicy) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - if !equality.Semantic.DeepEqual(policy.Spec.ResourceSelectors, oldPolicy.Spec.ResourceSelectors) { - klog.Error(helper.DenyReasonResourceSelectorsModify) - return admission.Denied(helper.DenyReasonResourceSelectorsModify) - } - } - if err := helper.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil { klog.Error(err) return admission.Denied(err.Error()) diff --git a/pkg/webhook/propagationpolicy/validating.go b/pkg/webhook/propagationpolicy/validating.go index 3169f14c4..ba070c73e 100644 --- a/pkg/webhook/propagationpolicy/validating.go +++ b/pkg/webhook/propagationpolicy/validating.go @@ -4,8 +4,6 @@ import ( "context" "net/http" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -34,18 +32,6 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request) } klog.V(2).Infof("Validating PropagationPolicy(%s/%s) for request: %s", policy.Namespace, policy.Name, req.Operation) - if req.Operation == admissionv1.Update { - oldPolicy := &policyv1alpha1.PropagationPolicy{} - err := v.decoder.DecodeRaw(req.OldObject, oldPolicy) - if err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - if !equality.Semantic.DeepEqual(policy.Spec.ResourceSelectors, oldPolicy.Spec.ResourceSelectors) { - klog.Info(helper.DenyReasonResourceSelectorsModify) - return admission.Denied(helper.DenyReasonResourceSelectorsModify) - } - } - if err := helper.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil { klog.Error(err) return admission.Denied(err.Error()) From ce0236e546834cec858e547bbe711d4eee221a08 Mon Sep 17 00:00:00 2001 From: changzhen Date: Wed, 9 Nov 2022 16:41:15 +0800 Subject: [PATCH 2/2] add e2e for editing policy resoureceSelectors Signed-off-by: changzhen --- test/e2e/clusterpropagationpolicy_test.go | 171 +++++++++++++++++- .../e2e/framework/clusterpropagationpolicy.go | 12 ++ test/e2e/framework/deployment.go | 13 ++ test/e2e/framework/propagationpolicy.go | 12 ++ test/e2e/framework/rbac.go | 13 ++ test/e2e/propagationpolicy_test.go | 93 +++++++++- 6 files changed, 311 insertions(+), 3 deletions(-) diff --git a/test/e2e/clusterpropagationpolicy_test.go b/test/e2e/clusterpropagationpolicy_test.go index 3d39cd655..3af8a9050 100644 --- a/test/e2e/clusterpropagationpolicy_test.go +++ b/test/e2e/clusterpropagationpolicy_test.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/onsi/ginkgo/v2" + appsv1 "k8s.io/api/apps/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/util/rand" @@ -13,7 +14,7 @@ import ( testhelper "github.com/karmada-io/karmada/test/helper" ) -var _ = ginkgo.Describe("[BasicClusterPropagation] basic cluster propagation testing", func() { +var _ = ginkgo.Describe("[BasicClusterPropagation] propagation testing", func() { ginkgo.Context("CustomResourceDefinition propagation testing", func() { var crdGroup string var randStr string @@ -149,3 +150,171 @@ var _ = ginkgo.Describe("[BasicClusterPropagation] basic cluster propagation tes }) }) }) + +var _ = ginkgo.Describe("[AdvancedClusterPropagation] propagation testing", func() { + ginkgo.Context("Edit ClusterPropagationPolicy ResourceSelectors", func() { + ginkgo.When("propagate namespace scope resource", func() { + var policy *policyv1alpha1.ClusterPropagationPolicy + var deployment01, deployment02 *appsv1.Deployment + var targetMember string + + ginkgo.BeforeEach(func() { + targetMember = framework.ClusterNames()[0] + policyName := deploymentNamePrefix + rand.String(RandomStrLength) + + deployment01 = testhelper.NewDeployment(testNamespace, policyName+"01") + deployment02 = testhelper.NewDeployment(testNamespace, policyName+"02") + + policy = testhelper.NewClusterPropagationPolicy(policyName, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment01.APIVersion, + Kind: deployment01.Kind, + Name: deployment01.Name, + }}, policyv1alpha1.Placement{ + ClusterAffinity: &policyv1alpha1.ClusterAffinity{ + ClusterNames: []string{targetMember}, + }, + }) + }) + + ginkgo.BeforeEach(func() { + framework.CreateClusterPropagationPolicy(karmadaClient, policy) + framework.CreateDeployment(kubeClient, deployment01) + framework.CreateDeployment(kubeClient, deployment02) + ginkgo.DeferCleanup(func() { + framework.RemoveClusterPropagationPolicy(karmadaClient, policy.Name) + framework.RemoveDeployment(kubeClient, deployment01.Namespace, deployment01.Name) + framework.RemoveDeployment(kubeClient, deployment02.Namespace, deployment02.Name) + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment01.Namespace, deployment01.Name, + func(deployment *appsv1.Deployment) bool { return true }) + }) + + ginkgo.It("add resourceSelectors item", func() { + framework.UpdateClusterPropagationPolicy(karmadaClient, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment01.APIVersion, + Kind: deployment01.Kind, + Name: deployment01.Name, + }, + { + APIVersion: deployment02.APIVersion, + Kind: deployment02.Kind, + Name: deployment02.Name, + }, + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment02.Namespace, deployment02.Name, + func(deployment *appsv1.Deployment) bool { return true }) + }) + + ginkgo.It("update resourceSelectors item", func() { + framework.UpdateClusterPropagationPolicy(karmadaClient, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment02.APIVersion, + Kind: deployment02.Kind, + Name: deployment02.Name, + }, + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment02.Namespace, deployment02.Name, + func(deployment *appsv1.Deployment) bool { return true }) + framework.WaitDeploymentGetByClientFitWith(kubeClient, deployment01.Namespace, deployment01.Name, + func(deployment *appsv1.Deployment) bool { + if deployment.Labels == nil { + return true + } + + _, exist := deployment.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] + return !exist + }) + }) + }) + + ginkgo.When("propagate cluster scope resource", func() { + var policy *policyv1alpha1.ClusterPropagationPolicy + var clusterRole01, clusterRole02 *rbacv1.ClusterRole + var targetMember string + + ginkgo.BeforeEach(func() { + policyName := clusterRoleNamePrefix + rand.String(RandomStrLength) + targetMember = framework.ClusterNames()[0] + + clusterRole01 = testhelper.NewClusterRole(fmt.Sprintf("system:test-%s-01", policyName), nil) + clusterRole02 = testhelper.NewClusterRole(fmt.Sprintf("system:test-%s-02", policyName), nil) + policy = testhelper.NewClusterPropagationPolicy(policyName, []policyv1alpha1.ResourceSelector{ + { + APIVersion: clusterRole01.APIVersion, + Kind: clusterRole01.Kind, + Name: clusterRole01.Name, + }, + }, policyv1alpha1.Placement{ + ClusterAffinity: &policyv1alpha1.ClusterAffinity{ + ClusterNames: []string{targetMember}, + }, + }) + }) + + ginkgo.BeforeEach(func() { + framework.CreateClusterPropagationPolicy(karmadaClient, policy) + framework.CreateClusterRole(kubeClient, clusterRole01) + framework.CreateClusterRole(kubeClient, clusterRole02) + ginkgo.DeferCleanup(func() { + framework.RemoveClusterPropagationPolicy(karmadaClient, policy.Name) + framework.RemoveClusterRole(kubeClient, clusterRole01.Name) + framework.RemoveClusterRole(kubeClient, clusterRole02.Name) + }) + + framework.WaitClusterRolePresentOnClusterFitWith(targetMember, clusterRole01.Name, + func(role *rbacv1.ClusterRole) bool { + return true + }) + }) + + ginkgo.It("add resourceSelectors item", func() { + framework.UpdateClusterPropagationPolicy(karmadaClient, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: clusterRole01.APIVersion, + Kind: clusterRole01.Kind, + Name: clusterRole01.Name, + }, + { + APIVersion: clusterRole02.APIVersion, + Kind: clusterRole02.Kind, + Name: clusterRole02.Name, + }, + }) + + framework.WaitClusterRolePresentOnClusterFitWith(targetMember, clusterRole02.Name, + func(role *rbacv1.ClusterRole) bool { + return true + }) + }) + + ginkgo.It("update resourceSelectors item", func() { + framework.UpdateClusterPropagationPolicy(karmadaClient, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: clusterRole02.APIVersion, + Kind: clusterRole02.Kind, + Name: clusterRole02.Name, + }, + }) + + framework.WaitClusterRolePresentOnClusterFitWith(targetMember, clusterRole02.Name, + func(role *rbacv1.ClusterRole) bool { + return true + }) + framework.WaitClusterRoleGetByClientFitWith(kubeClient, clusterRole01.Name, + func(clusterRole *rbacv1.ClusterRole) bool { + if clusterRole.Labels == nil { + return true + } + + _, exist := clusterRole.Labels[policyv1alpha1.ClusterPropagationPolicyLabel] + return !exist + }) + }) + }) + }) +}) diff --git a/test/e2e/framework/clusterpropagationpolicy.go b/test/e2e/framework/clusterpropagationpolicy.go index 6d8762e15..1ffaf51bf 100644 --- a/test/e2e/framework/clusterpropagationpolicy.go +++ b/test/e2e/framework/clusterpropagationpolicy.go @@ -27,3 +27,15 @@ func RemoveClusterPropagationPolicy(client karmada.Interface, name string) { gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) }) } + +// UpdateClusterPropagationPolicy update ClusterPropagationPolicy resourceSelectors with karmada client. +func UpdateClusterPropagationPolicy(client karmada.Interface, name string, resourceSelectors []policyv1alpha1.ResourceSelector) { + ginkgo.By(fmt.Sprintf("Updating ClusterPropagationPolicy(%s)", name), func() { + newPolicy, err := client.PolicyV1alpha1().ClusterPropagationPolicies().Get(context.TODO(), name, metav1.GetOptions{}) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + newPolicy.Spec.ResourceSelectors = resourceSelectors + _, err = client.PolicyV1alpha1().ClusterPropagationPolicies().Update(context.TODO(), newPolicy, metav1.UpdateOptions{}) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + }) +} diff --git a/test/e2e/framework/deployment.go b/test/e2e/framework/deployment.go index 7e9f7c234..f94f50a59 100644 --- a/test/e2e/framework/deployment.go +++ b/test/e2e/framework/deployment.go @@ -166,3 +166,16 @@ func CheckDeploymentReadyStatus(deployment *appsv1.Deployment, wantedReplicas in } return false } + +// WaitDeploymentGetByClientFitWith wait deployment get by client fit with func. +func WaitDeploymentGetByClientFitWith(client kubernetes.Interface, namespace, name string, fit func(deployment *appsv1.Deployment) bool) { + ginkgo.By(fmt.Sprintf("Check deployment(%s/%s) labels fit with function", namespace, name), func() { + gomega.Eventually(func() bool { + dep, err := client.AppsV1().Deployments(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return false + } + return fit(dep) + }, pollTimeout, pollInterval).Should(gomega.Equal(true)) + }) +} diff --git a/test/e2e/framework/propagationpolicy.go b/test/e2e/framework/propagationpolicy.go index 21a7f58a2..0fe4b0fdc 100644 --- a/test/e2e/framework/propagationpolicy.go +++ b/test/e2e/framework/propagationpolicy.go @@ -40,3 +40,15 @@ func PatchPropagationPolicy(client karmada.Interface, namespace, name string, pa gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) }) } + +// UpdatePropagationPolicy update PropagationPolicy resourceSelectors with karmada client. +func UpdatePropagationPolicy(client karmada.Interface, namespace, name string, resourceSelectors []policyv1alpha1.ResourceSelector) { + ginkgo.By(fmt.Sprintf("Updating PropagationPolicy(%s/%s)", namespace, name), func() { + newPolicy, err := client.PolicyV1alpha1().PropagationPolicies(namespace).Get(context.TODO(), name, metav1.GetOptions{}) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + + newPolicy.Spec.ResourceSelectors = resourceSelectors + _, err = client.PolicyV1alpha1().PropagationPolicies(namespace).Update(context.TODO(), newPolicy, metav1.UpdateOptions{}) + gomega.Expect(err).ShouldNot(gomega.HaveOccurred()) + }) +} diff --git a/test/e2e/framework/rbac.go b/test/e2e/framework/rbac.go index 4b9438e39..31492c4d2 100644 --- a/test/e2e/framework/rbac.go +++ b/test/e2e/framework/rbac.go @@ -158,6 +158,19 @@ func WaitClusterRoleDisappearOnCluster(cluster, name string) { }, pollTimeout, pollInterval).Should(gomega.Equal(true)) } +// WaitClusterRoleGetByClientFitWith wait clusterRole get by client fit with func. +func WaitClusterRoleGetByClientFitWith(client kubernetes.Interface, name string, fit func(clusterRole *rbacv1.ClusterRole) bool) { + ginkgo.By(fmt.Sprintf("Check clusterRole(%s) labels fit with function", name), func() { + gomega.Eventually(func() bool { + clusterRole, err := client.RbacV1().ClusterRoles().Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return false + } + return fit(clusterRole) + }, pollTimeout, pollInterval).Should(gomega.Equal(true)) + }) +} + // CreateRoleBinding create roleBinding. func CreateRoleBinding(client kubernetes.Interface, roleBinding *rbacv1.RoleBinding) { ginkgo.By(fmt.Sprintf("Creating RoleBinding(%s/%s)", roleBinding.Namespace, roleBinding.Name), func() { diff --git a/test/e2e/propagationpolicy_test.go b/test/e2e/propagationpolicy_test.go index 72fefad17..a72b29b73 100644 --- a/test/e2e/propagationpolicy_test.go +++ b/test/e2e/propagationpolicy_test.go @@ -29,7 +29,7 @@ import ( ) // BasicPropagation focus on basic propagation functionality testing. -var _ = ginkgo.Describe("[BasicPropagation] basic propagation testing", func() { +var _ = ginkgo.Describe("[BasicPropagation] propagation testing", func() { ginkgo.Context("Deployment propagation testing", func() { var policyNamespace, policyName string var deploymentNamespace, deploymentName string @@ -495,7 +495,7 @@ var _ = ginkgo.Describe("[BasicPropagation] basic propagation testing", func() { }) // ImplicitPriority more than one PP matches the object, we should choose the most suitable one. -var _ = ginkgo.Describe("[ImplicitPriority] basic propagation testing", func() { +var _ = ginkgo.Describe("[ImplicitPriority] propagation testing", func() { ginkgo.Context("priorityMatchName propagation testing", func() { var policyNamespace, priorityMatchName, priorityMatchLabelSelector, priorityMatchAll string var deploymentNamespace, deploymentName string @@ -573,6 +573,7 @@ var _ = ginkgo.Describe("[ImplicitPriority] basic propagation testing", func() { }) }) }) + ginkgo.Context("policyMatchLabelSelector propagation testing", func() { var policyNamespace, priorityMatchLabelSelector, priorityMatchAll string var deploymentNamespace, deploymentName string @@ -638,6 +639,7 @@ var _ = ginkgo.Describe("[ImplicitPriority] basic propagation testing", func() { }) }) }) + ginkgo.Context("priorityMatchAll propagation testing", func() { var policyNamespace, priorityMatchAll string var deploymentNamespace, deploymentName string @@ -688,3 +690,90 @@ var _ = ginkgo.Describe("[ImplicitPriority] basic propagation testing", func() { }) }) }) + +// AdvancedPropagation focus on some advanced propagation testing. +var _ = ginkgo.Describe("[AdvancedPropagation] propagation testing", func() { + ginkgo.Context("Edit PropagationPolicy ResourceSelectors", func() { + var policy *policyv1alpha1.PropagationPolicy + var deployment01, deployment02 *appsv1.Deployment + var targetMember string + + ginkgo.BeforeEach(func() { + targetMember = framework.ClusterNames()[0] + policyNamespace := testNamespace + policyName := deploymentNamePrefix + rand.String(RandomStrLength) + + deployment01 = testhelper.NewDeployment(testNamespace, policyName+"01") + deployment02 = testhelper.NewDeployment(testNamespace, policyName+"02") + + policy = testhelper.NewPropagationPolicy(policyNamespace, policyName, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment01.APIVersion, + Kind: deployment01.Kind, + Name: deployment01.Name, + }}, policyv1alpha1.Placement{ + ClusterAffinity: &policyv1alpha1.ClusterAffinity{ + ClusterNames: []string{targetMember}, + }, + }) + }) + + ginkgo.BeforeEach(func() { + framework.CreatePropagationPolicy(karmadaClient, policy) + framework.CreateDeployment(kubeClient, deployment01) + framework.CreateDeployment(kubeClient, deployment02) + ginkgo.DeferCleanup(func() { + framework.RemovePropagationPolicy(karmadaClient, policy.Namespace, policy.Name) + framework.RemoveDeployment(kubeClient, deployment01.Namespace, deployment01.Name) + framework.RemoveDeployment(kubeClient, deployment02.Namespace, deployment02.Name) + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment01.Namespace, deployment01.Name, + func(deployment *appsv1.Deployment) bool { return true }) + }) + + ginkgo.It("add resourceSelectors item", func() { + framework.UpdatePropagationPolicy(karmadaClient, policy.Namespace, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment01.APIVersion, + Kind: deployment01.Kind, + Name: deployment01.Name, + }, + { + APIVersion: deployment02.APIVersion, + Kind: deployment02.Kind, + Name: deployment02.Name, + }, + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment02.Namespace, deployment02.Name, + func(deployment *appsv1.Deployment) bool { return true }) + }) + + ginkgo.It("update resourceSelectors item", func() { + framework.UpdatePropagationPolicy(karmadaClient, policy.Namespace, policy.Name, []policyv1alpha1.ResourceSelector{ + { + APIVersion: deployment02.APIVersion, + Kind: deployment02.Kind, + Name: deployment02.Name, + }, + }) + + framework.WaitDeploymentPresentOnClusterFitWith(targetMember, deployment02.Namespace, deployment02.Name, + func(deployment *appsv1.Deployment) bool { return true }) + framework.WaitDeploymentGetByClientFitWith(kubeClient, deployment01.Namespace, deployment01.Name, + func(deployment *appsv1.Deployment) bool { + if deployment.Labels == nil { + return true + } + + _, namespaceExist := deployment.Labels[policyv1alpha1.PropagationPolicyNamespaceLabel] + _, nameExist := deployment.Labels[policyv1alpha1.PropagationPolicyNameLabel] + if namespaceExist || nameExist { + return false + } + return true + }) + }) + }) +})