feat: allow changing strategy from ApplyAlways to Reconcile

Allows us to migrate ClusterResourceSets to the upstream Reconcile strategy and use CAPI directly with the next release
mesosphere · Sep 26, 2023 · 6597d12 · 6597d12
1 parent 72247b1
commit 6597d12
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git 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
@@ -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 {