Merge pull request #4438 from Nordix/fix-max-surge-mutability-issue-0.3

🐛 Fix KCP maxSurge field to be mutable

controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go

@@ -139,7 +139,7 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) error {
 		{spec, "version"},
 		{spec, "upgradeAfter"},
 		{spec, "nodeDrainTimeout"},
-		{spec, "rolloutStrategy"},
+		{spec, "rolloutStrategy", "*"},
 	}
 
 	allErrs := in.validateCommon()

controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go

@@ -189,6 +189,14 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 				Name:      "infraTemplate",
 			},
 			Replicas: pointer.Int32Ptr(1),
+			RolloutStrategy: &RolloutStrategy{
+				Type: RollingUpdateStrategyType,
+				RollingUpdate: &RollingUpdate{
+					MaxSurge: &intstr.IntOrString{
+						IntVal: 1,
+					},
+				},
+			},
 			KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{
 				InitConfiguration: &kubeadmv1beta1.InitConfiguration{
 					LocalAPIEndpoint: kubeadmv1beta1.APIEndpoint{
@@ -242,6 +250,13 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 		},
 	}
 
+	updateMaxSurgeVal := before.DeepCopy()
+	updateMaxSurgeVal.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal = int32(0)
+	updateMaxSurgeVal.Spec.Replicas = pointer.Int32Ptr(3)
+
+	wrongReplicaCountForScaleIn := before.DeepCopy()
+	wrongReplicaCountForScaleIn.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal = int32(0)
+
 	invalidUpdateKubeadmConfigInit := before.DeepCopy()
 	invalidUpdateKubeadmConfigInit.Spec.KubeadmConfigSpec.InitConfiguration = &kubeadmv1beta1.InitConfiguration{}
 
@@ -745,6 +760,18 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			before:    disallowedUpgrade118Prev,
 			kcp:       disallowedUpgrade119Version,
 		},
+		{
+			name:      "should not return an error when maxSurge value is updated to 0",
+			expectErr: false,
+			before:    before,
+			kcp:       updateMaxSurgeVal,
+		},
+		{
+			name:      "should return an error when maxSurge value is updated to 0, but replica count is < 3",
+			expectErr: true,
+			before:    before,
+			kcp:       wrongReplicaCountForScaleIn,
+		},
 	}
 
 	for _, tt := range tests {
@@ -761,6 +788,56 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 	}
 }
 
+func TestKubeadmControlPlaneValidateUpdateAfterDefaulting(t *testing.T) {
+	before := &KubeadmControlPlane{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "foo",
+		},
+		Spec: KubeadmControlPlaneSpec{
+			Version: "v1.19.0",
+			InfrastructureTemplate: corev1.ObjectReference{
+				Namespace: "foo",
+				Name:      "infraTemplate",
+			},
+		},
+	}
+
+	afterDefault := before.DeepCopy()
+	afterDefault.Default()
+
+	tests := []struct {
+		name      string
+		expectErr bool
+		before    *KubeadmControlPlane
+		kcp       *KubeadmControlPlane
+	}{
+		{
+			name:      "update should succeed after defaulting",
+			expectErr: false,
+			before:    before,
+			kcp:       afterDefault,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			err := tt.kcp.ValidateUpdate(tt.before.DeepCopy())
+			if tt.expectErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).To(Succeed())
+				g.Expect(tt.kcp.Spec.InfrastructureTemplate.Namespace).To(Equal(tt.before.Namespace))
+				g.Expect(tt.kcp.Spec.Version).To(Equal("v1.19.0"))
+				g.Expect(tt.kcp.Spec.RolloutStrategy.Type).To(Equal(RollingUpdateStrategyType))
+				g.Expect(tt.kcp.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal).To(Equal(int32(1)))
+				g.Expect(tt.kcp.Spec.Replicas).To(Equal(pointer.Int32Ptr(1)))
+			}
+		})
+	}
+}
+
 func TestPathsMatch(t *testing.T) {
 	tests := []struct {
 		name          string