From 8b8853b6da7decf5ec1eb68b26eaebb54b74a06e Mon Sep 17 00:00:00 2001 From: alejandroEsc Date: Mon, 7 Feb 2022 21:19:45 -0500 Subject: [PATCH 1/6] feat: adding new clusteresourceset strategy ApplyAlways Reuses the Reconcile strategy but is kept for backwards compatibility. --- ....cluster.x-k8s.io_clusterresourcesets.yaml | 2 + .../api/v1beta1/clusterresourceset_types.go | 5 +- .../clusterresourceset_controller.go | 2 +- .../clusterresourceset_controller_test.go | 204 ++++++++++++++++++ .../controllers/clusterresourceset_scope.go | 2 + 5 files changed, 213 insertions(+), 2 deletions(-) diff --git a/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesets.yaml b/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesets.yaml index f00315537a2c..bfffa113f709 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesets.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesets.yaml @@ -112,6 +112,7 @@ spec: Defaults to ApplyOnce. This field is immutable. enum: - ApplyOnce + - ApplyAlways type: string required: - clusterSelector @@ -439,6 +440,7 @@ spec: Defaults to ApplyOnce. This field is immutable. enum: - ApplyOnce + - ApplyAlways - Reconcile type: string required: diff --git a/exp/addons/api/v1beta1/clusterresourceset_types.go b/exp/addons/api/v1beta1/clusterresourceset_types.go index e64dccd29714..6c9b983c3935 100644 --- a/exp/addons/api/v1beta1/clusterresourceset_types.go +++ b/exp/addons/api/v1beta1/clusterresourceset_types.go @@ -46,7 +46,7 @@ type ClusterResourceSetSpec struct { Resources []ResourceRef `json:"resources,omitempty"` // Strategy is the strategy to be used during applying resources. Defaults to ApplyOnce. This field is immutable. - // +kubebuilder:validation:Enum=ApplyOnce;Reconcile + // +kubebuilder:validation:Enum=ApplyOnce;ApplyAlways;Reconcile // +optional Strategy string `json:"strategy,omitempty"` } @@ -83,6 +83,9 @@ const ( // ClusterResourceSetStrategyReconcile reapplies the resources managed by a ClusterResourceSet // if their normalized hash changes. ClusterResourceSetStrategyReconcile ClusterResourceSetStrategy = "Reconcile" + // ClusterResourceSetStrategyApplyAlways is the strategy where changes to the ClusterResourceSet + // are applied always if they exist already in clusters or created if they do not. + ClusterResourceSetStrategyApplyAlways ClusterResourceSetStrategy = "ApplyAlways" ) // SetTypedStrategy sets the Strategy field to the string representation of ClusterResourceSetStrategy. diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go index 3c451d5b6c6b..07f79b2906c1 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller.go @@ -234,7 +234,7 @@ func (r *ClusterResourceSetReconciler) getClustersByClusterResourceSetSelector(c return nil, errors.Wrap(err, "failed to list clusters") } - clusters := []*clusterv1.Cluster{} + clusters := make([]*clusterv1.Cluster, 0) for i := range clusterList.Items { c := &clusterList.Items[i] if c.DeletionTimestamp.IsZero() { diff --git a/exp/addons/internal/controllers/clusterresourceset_controller_test.go b/exp/addons/internal/controllers/clusterresourceset_controller_test.go index 916a1687d6fa..d264c0acd3bd 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller_test.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller_test.go @@ -996,6 +996,210 @@ metadata: return err == nil }, timeout).Should(BeTrue()) }) + + t.Run("Should create ClusterResourceSet with strategy 'AlwaysApply' and reconcile when configmap changes data", func(t *testing.T) { + g := NewWithT(t) + ns := setup(t, g) + defer teardown(t, g, ns) + + t.Log("Updating the cluster with labels") + testCluster.SetLabels(labels) + g.Expect(env.Update(ctx, testCluster)).To(Succeed()) + + clusterResourceSetInstance := &addonsv1.ClusterResourceSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterResourceSetName, + Namespace: ns.Name, + }, + Spec: addonsv1.ClusterResourceSetSpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + Strategy: "ApplyAlways", + Resources: []addonsv1.ResourceRef{{Name: configmapName, Kind: "ConfigMap"}}, + }, + } + // Create the ClusterResourceSet. + g.Expect(env.Create(ctx, clusterResourceSetInstance)).To(Succeed()) + + // Wait until ClusterResourceSetBinding is created for the Cluster + clusterResourceSetBindingKey := client.ObjectKey{ + Namespace: testCluster.Namespace, + Name: testCluster.Name, + } + + t.Log("Getting ClusterResourceSetBinding") + oldHash := "" + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + oldHash = resource.Hash + return resource.Applied + }, timeout).Should(BeTrue()) + + // Get configmap obj, update the configmap + cmKey := client.ObjectKey{ + Namespace: ns.Name, + Name: configmapName, + } + cm := &corev1.ConfigMap{} + g.Expect(env.Get(ctx, cmKey, cm)).To(Succeed()) + + cmUpdate := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cm.GetName(), + Namespace: cm.GetNamespace(), + ResourceVersion: cm.ResourceVersion, + UID: cm.GetUID(), + }, + Data: map[string]string{ + "cm": `kind: ConfigMap +apiVersion: v1 +metadata: + name: resource-configmap + namespace: default +data: + hello: "world!"`, + }, + } + + // update the configmap data + t.Log("Updating the configmap resource") + g.Expect(env.Update(ctx, cmUpdate)).To(Succeed()) + + t.Log("Check if reconciled hash has updated for the changed configmap resource") + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + return resource.Hash != oldHash + }, timeout).Should(BeTrue()) + }) + + t.Run("Should create ClusterResourceSet with strategy 'AlwaysApply' and reconcile when secret changes data", func(t *testing.T) { + g := NewWithT(t) + ns := setup(t, g) + defer teardown(t, g, ns) + + t.Log("Updating the cluster with labels") + testCluster.SetLabels(labels) + g.Expect(env.Update(ctx, testCluster)).To(Succeed()) + + clusterResourceSetInstance := &addonsv1.ClusterResourceSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterResourceSetName, + Namespace: ns.Name, + }, + Spec: addonsv1.ClusterResourceSetSpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + Strategy: "ApplyAlways", + Resources: []addonsv1.ResourceRef{{Name: secretName, Kind: "Secret"}}, + }, + } + // Create the ClusterResourceSet. + g.Expect(env.Create(ctx, clusterResourceSetInstance)).To(Succeed()) + + // Wait until ClusterResourceSetBinding is created for the Cluster + clusterResourceSetBindingKey := client.ObjectKey{ + Namespace: testCluster.Namespace, + Name: testCluster.Name, + } + + t.Log("Getting ClusterResourceSetBinding") + oldHash := "" + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + oldHash = resource.Hash + return resource.Applied + }, timeout).Should(BeTrue()) + + secretKey := client.ObjectKey{ + Namespace: ns.Name, + Name: secretName, + } + secret := &corev1.Secret{} + g.Expect(env.Get(ctx, secretKey, secret)).To(Succeed()) + + secretUpdate := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secret.GetName(), + Namespace: secret.GetNamespace(), + ResourceVersion: secret.ResourceVersion, + UID: secret.GetUID(), + }, + Type: "addons.cluster.x-k8s.io/resource-set", + StringData: map[string]string{ + "cm": `kind: ConfigMap +apiVersion: v1 +metadata: + name: resource-configmap + namespace: default +data: + hello: "world!"`, + }, + } + + // update the configmap data + t.Log("Updating the secret resource") + g.Expect(env.Update(ctx, secretUpdate)).To(Succeed()) + + t.Log("Check if reconciled hash has updated for the changed secret resource") + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + return resource.Hash != oldHash + }, timeout).Should(BeTrue()) + }) } func clusterResourceSetBindingReady(env *envtest.Environment, cluster *clusterv1.Cluster) func() bool { diff --git a/exp/addons/internal/controllers/clusterresourceset_scope.go b/exp/addons/internal/controllers/clusterresourceset_scope.go index 921a752e0d54..396b6dc3c1d7 100644 --- a/exp/addons/internal/controllers/clusterresourceset_scope.go +++ b/exp/addons/internal/controllers/clusterresourceset_scope.go @@ -82,6 +82,8 @@ func newResourceReconcileScope( return &reconcileApplyOnceScope{base} case addonsv1.ClusterResourceSetStrategyReconcile: return &reconcileStrategyScope{base} + case addonsv1.ClusterResourceSetStrategyApplyAlways: + return &reconcileStrategyScope{base} default: return nil } From 4c1b4fc9b59283bbdc70f84583ee64a2d5338fc4 Mon Sep 17 00:00:00 2001 From: alejandroEsc Date: Fri, 6 May 2022 13:54:19 -0700 Subject: [PATCH 2/6] test: Verify that CRS controller does not re-apply a resource if the resource has not changed. Co-authored-by: Daniel Lipovetsky --- .../clusterresourceset_controller_test.go | 184 +++++++++++++++++- 1 file changed, 183 insertions(+), 1 deletion(-) diff --git a/exp/addons/internal/controllers/clusterresourceset_controller_test.go b/exp/addons/internal/controllers/clusterresourceset_controller_test.go index d264c0acd3bd..5ba52d8f7c7d 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller_test.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller_test.go @@ -1177,7 +1177,7 @@ data: }, } - // update the configmap data + // update the secret data t.Log("Updating the secret resource") g.Expect(env.Update(ctx, secretUpdate)).To(Succeed()) @@ -1200,6 +1200,188 @@ data: return resource.Hash != oldHash }, timeout).Should(BeTrue()) }) + + t.Run("Should create ClusterResourceSet with strategy 'AlwaysApply' and reconcile configmap only once if data has not changed", func(t *testing.T) { + g := NewWithT(t) + ns := setup(t, g) + defer teardown(t, g, ns) + + t.Log("Updating the cluster with labels") + testCluster.SetLabels(labels) + g.Expect(env.Update(ctx, testCluster)).To(Succeed()) + + clusterResourceSetInstance := &addonsv1.ClusterResourceSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterResourceSetName, + Namespace: ns.Name, + }, + Spec: addonsv1.ClusterResourceSetSpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + Strategy: "ApplyAlways", + Resources: []addonsv1.ResourceRef{{Name: configmapName, Kind: "ConfigMap"}}, + }, + } + // Create the ClusterResourceSet. + g.Expect(env.Create(ctx, clusterResourceSetInstance)).To(Succeed()) + + // Wait until ClusterResourceSetBinding is created for the Cluster + clusterResourceSetBindingKey := client.ObjectKey{ + Namespace: testCluster.Namespace, + Name: testCluster.Name, + } + + // Wait until ClusterResourceSetBinding is created for the Cluster + t.Log("Waiting for the ClusterResourceSetBinding to be created") + oldHash := "" + var oldLastAppliedTime *metav1.Time + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + oldHash = resource.Hash + oldLastAppliedTime = resource.LastAppliedTime + return resource.Applied + }, timeout).Should(BeTrue()) + + // Get configmap obj, update the configmap + cmKey := client.ObjectKey{ + Namespace: ns.Name, + Name: configmapName, + } + cm := &corev1.ConfigMap{} + g.Expect(env.Get(ctx, cmKey, cm)).To(Succeed()) + + cm.Labels = map[string]string{"foo": "bar"} + + // The CRS controller writes a lastAppliedTime field, which is of type metav1.Time. The precision at most a + // second. Therefore, if the controller re-applies the resource twice within one second, the lastAppliedTime + // value is unlikely to change. Our test below compares the lastAppliedTime values of two reconciles, so we wait + // to prevent the reconciles from running within the same second. Related issue: https://issues.k8s.io/15262 + t.Log("Letting some time pass before updating the resource, so that lastAppliedTime will be different.") + time.Sleep(10 * time.Second) + + // update the configmap data + t.Log("Updating the configmap resource") + g.Expect(env.Update(ctx, cm)).To(Succeed()) + + t.Log("Verifying that resource is not re-applied over a period of time.") + g.Consistently(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // only one resource is applied + resource := bindings[0].Resources[0] + return oldHash == resource.Hash && oldLastAppliedTime.Equal(resource.LastAppliedTime) + }, timeout).Should(BeTrue()) + }) + + t.Run("Should create ClusterResourceSet with strategy 'AlwaysApply' and reconcile secrets only once if data has not changed", func(t *testing.T) { + g := NewWithT(t) + ns := setup(t, g) + defer teardown(t, g, ns) + + t.Log("Updating the cluster with labels") + testCluster.SetLabels(labels) + g.Expect(env.Update(ctx, testCluster)).To(Succeed()) + + clusterResourceSetInstance := &addonsv1.ClusterResourceSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterResourceSetName, + Namespace: ns.Name, + }, + Spec: addonsv1.ClusterResourceSetSpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: labels, + }, + Strategy: "ApplyAlways", + Resources: []addonsv1.ResourceRef{{Name: secretName, Kind: "Secret"}}, + }, + } + // Create the ClusterResourceSet. + g.Expect(env.Create(ctx, clusterResourceSetInstance)).To(Succeed()) + + // Wait until ClusterResourceSetBinding is created for the Cluster + clusterResourceSetBindingKey := client.ObjectKey{ + Namespace: testCluster.Namespace, + Name: testCluster.Name, + } + + t.Log("Waiting for the ClusterResourceSetBinding to be created") + oldHash := "" + var oldLastAppliedTime *metav1.Time + g.Eventually(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // should only have one binding + if len(bindings) != 1 { + return false + } + + // only one resource is applied + resource := bindings[0].Resources[0] + oldHash = resource.Hash + oldLastAppliedTime = resource.LastAppliedTime + return resource.Applied + }, timeout).Should(BeTrue()) + + secretKey := client.ObjectKey{ + Namespace: ns.Name, + Name: secretName, + } + secret := &corev1.Secret{} + g.Expect(env.Get(ctx, secretKey, secret)).To(Succeed()) + + // Overwrite the Secret labels to cause the ClusterResourceSet controller to reconcile any CRS that references + // the Secret. + secret.Labels = map[string]string{"foo": "bar"} + + // The CRS controller writes a lastAppliedTime field, which is of type metav1.Time. The precision at most a + // second. Therefore, if the controller re-applies the resource twice within one second, the lastAppliedTime + // value is unlikely to change. Our test below compares the lastAppliedTime values of two reconciles, so we wait + // to prevent the reconciles from running within the same second. Related issue: https://issues.k8s.io/15262 + t.Log("Letting some time pass before updating the resource, so that lastAppliedTime will be different.") + time.Sleep(10 * time.Second) + + // update the secrete, but not its data + t.Log("Updating the secret resource") + g.Expect(env.Update(ctx, secret)).To(Succeed()) + + t.Log("Verifying that resource is not re-applied over a period of time.") + g.Consistently(func() bool { + binding := &addonsv1.ClusterResourceSetBinding{} + err := env.Get(ctx, clusterResourceSetBindingKey, binding) + if err != nil { + return false + } + + bindings := binding.Spec.Bindings + // only one resource is applied + resource := bindings[0].Resources[0] + return oldHash == resource.Hash && oldLastAppliedTime.Equal(resource.LastAppliedTime) + }, timeout).Should(BeTrue()) + }) } func clusterResourceSetBindingReady(env *envtest.Environment, cluster *clusterv1.Cluster) func() bool { From 8b7c7089441c7170b31027d3b7c3b47baabf13ac Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Mon, 9 May 2022 09:45:50 -0700 Subject: [PATCH 3/6] test: Reduce amount of time to wait before updating object --- .../controllers/clusterresourceset_controller_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exp/addons/internal/controllers/clusterresourceset_controller_test.go b/exp/addons/internal/controllers/clusterresourceset_controller_test.go index 5ba52d8f7c7d..644d9fb6db45 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller_test.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller_test.go @@ -1271,7 +1271,7 @@ data: // value is unlikely to change. Our test below compares the lastAppliedTime values of two reconciles, so we wait // to prevent the reconciles from running within the same second. Related issue: https://issues.k8s.io/15262 t.Log("Letting some time pass before updating the resource, so that lastAppliedTime will be different.") - time.Sleep(10 * time.Second) + time.Sleep(2 * time.Second) // update the configmap data t.Log("Updating the configmap resource") @@ -1362,7 +1362,7 @@ data: // value is unlikely to change. Our test below compares the lastAppliedTime values of two reconciles, so we wait // to prevent the reconciles from running within the same second. Related issue: https://issues.k8s.io/15262 t.Log("Letting some time pass before updating the resource, so that lastAppliedTime will be different.") - time.Sleep(10 * time.Second) + time.Sleep(2 * time.Second) // update the secrete, but not its data t.Log("Updating the secret resource") From cfbf6c91f93f10ecbac5d635d1ea04f548a506f4 Mon Sep 17 00:00:00 2001 From: Daniel Lipovetsky Date: Fri, 30 Sep 2022 16:34:16 -0700 Subject: [PATCH 4/6] chore: Use literal syntax to declare zero-length arrays --- .../internal/controllers/clusterresourceset_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go index 07f79b2906c1..3c451d5b6c6b 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller.go @@ -234,7 +234,7 @@ func (r *ClusterResourceSetReconciler) getClustersByClusterResourceSetSelector(c return nil, errors.Wrap(err, "failed to list clusters") } - clusters := make([]*clusterv1.Cluster, 0) + clusters := []*clusterv1.Cluster{} for i := range clusterList.Items { c := &clusterList.Items[i] if c.DeletionTimestamp.IsZero() { From 67b609095c93a014c7e9e6110eac992a13ec5161 Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Thu, 9 Feb 2023 07:49:43 -0800 Subject: [PATCH 5/6] ci: build and push images at release to mesosphere registry --- .github/workflows/release.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d76225d8b809..0d2f7b60d483 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,6 +28,10 @@ jobs: go-version: ${{ env.go_version }} - name: generate release artifacts run: | + export REGISTRY=docker.io/mesosphere + export PROD_REGISTRY=$REGISTRY + export STAGING_REGISTRY=$REGISTRY + export TAG=${{ env.RELEASE_TAG }} make release - name: Release uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 # tag=v1 @@ -35,3 +39,16 @@ jobs: draft: true files: out/* body: "TODO: Copy release notes shared by the comms team" + - name: Login to Dockerhub Registry + uses: docker/login-action@v2 + with: + username: ${{ secrets.NEXUS_USERNAME }} + password: ${{ secrets.NEXUS_PASSWORD }} + - name: Build and push docker images + run: | + export REGISTRY=docker.io/mesosphere + export PROD_REGISTRY=$REGISTRY + export STAGING_REGISTRY=$REGISTRY + export TAG=${{ env.RELEASE_TAG }} + make ALL_ARCH="amd64 arm64" docker-build-all + make ALL_ARCH="amd64 arm64" docker-push-all From 932099d3b24577fb2b5412454903043bdaa7d7e1 Mon Sep 17 00:00:00 2001 From: Dimitri Koshkin Date: Mon, 9 Oct 2023 10:04:18 -0700 Subject: [PATCH 6/6] feat: allow changing strategy from ApplyAlways to Reconcile (#20) Allows us to migrate ClusterResourceSets to the upstream Reconcile strategy and use CAPI directly with the next release --- .../api/v1beta1/clusterresourceset_webhook.go | 15 +++++++--- .../clusterresourceset_webhook_test.go | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/exp/addons/api/v1beta1/clusterresourceset_webhook.go b/exp/addons/api/v1beta1/clusterresourceset_webhook.go index 61f8f6dc5c02..604a79c3fc5b 100644 --- a/exp/addons/api/v1beta1/clusterresourceset_webhook.go +++ b/exp/addons/api/v1beta1/clusterresourceset_webhook.go @@ -48,6 +48,8 @@ func (m *ClusterResourceSet) Default() { // ClusterResourceSet Strategy defaults to ApplyOnce. if m.Spec.Strategy == "" { m.Spec.Strategy = string(ClusterResourceSetStrategyApplyOnce) + } else if m.Spec.Strategy == string(ClusterResourceSetStrategyApplyAlways) { + m.Spec.Strategy = string(ClusterResourceSetStrategyReconcile) } } @@ -98,10 +100,15 @@ func (m *ClusterResourceSet) validate(old *ClusterResourceSet) error { } if old != nil && old.Spec.Strategy != "" && old.Spec.Strategy != m.Spec.Strategy { - allErrs = append( - allErrs, - field.Invalid(field.NewPath("spec", "strategy"), m.Spec.Strategy, "field is immutable"), - ) + // Allow changing from ApplyAlways (a strategy that was added in this fork) to Reconcile. + // ApplyAlways is an "alias" for Reconcile and migrating to Reconcile will enable us to stop using a fork. + if !(old.Spec.Strategy == string(ClusterResourceSetStrategyApplyAlways) && + m.Spec.Strategy == string(ClusterResourceSetStrategyReconcile)) { + allErrs = append( + allErrs, + field.Invalid(field.NewPath("spec", "strategy"), m.Spec.Strategy, "field is immutable"), + ) + } } if old != nil && !reflect.DeepEqual(old.Spec.ClusterSelector, m.Spec.ClusterSelector) { diff --git a/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go b/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go index 525300b0cc43..2c76bab85003 100644 --- a/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go +++ b/exp/addons/api/v1beta1/clusterresourceset_webhook_test.go @@ -38,6 +38,23 @@ func TestClusterResourcesetDefault(t *testing.T) { g.Expect(clusterResourceSet.Spec.Strategy).To(Equal(string(ClusterResourceSetStrategyApplyOnce))) } +func TestClusterResourcesetDefaultWithClusterResourceSetStrategyApplyAlways(t *testing.T) { + g := NewWithT(t) + clusterResourceSet := &ClusterResourceSet{ + Spec: ClusterResourceSetSpec{ + Strategy: string(ClusterResourceSetStrategyApplyAlways), + }, + } + defaultingValidationCRS := clusterResourceSet.DeepCopy() + defaultingValidationCRS.Spec.ClusterSelector = metav1.LabelSelector{ + MatchLabels: map[string]string{"foo": "bar"}, + } + t.Run("for ClusterResourceSet", utildefaulting.DefaultValidateTest(defaultingValidationCRS)) + clusterResourceSet.Default() + + g.Expect(clusterResourceSet.Spec.Strategy).To(Equal(string(ClusterResourceSetStrategyReconcile))) +} + func TestClusterResourceSetLabelSelectorAsSelectorValidation(t *testing.T) { tests := []struct { name string @@ -104,6 +121,18 @@ func TestClusterResourceSetStrategyImmutable(t *testing.T) { newStrategy: "", expectErr: true, }, + { + name: "when the Strategy has changed, but the old value was ApplyAlways", + oldStrategy: string(ClusterResourceSetStrategyApplyAlways), + newStrategy: string(ClusterResourceSetStrategyReconcile), + expectErr: false, + }, + { + name: "when the Strategy has changed, but the old value was ApplyAlways and the new value is ApplyOnce", + oldStrategy: string(ClusterResourceSetStrategyApplyAlways), + newStrategy: string(ClusterResourceSetStrategyApplyOnce), + expectErr: true, + }, } for _, tt := range tests {