From ce5ab72133f808c94ce8d98ba90977430c97f403 Mon Sep 17 00:00:00 2001
From: jan-est <jan.tilles@est.tech>
Date: Wed, 7 Apr 2021 11:30:08 +0300
Subject: [PATCH] This PR is fixing issue 4437. Webhook defaulting in v0.3.15
 causing an error during KCP upgrade after upgrading cluster from v0.3.x to
 v0.3.15. This PR is changing every field under spec.rolloutStrategy mutable.

---
 .../v1alpha3/kubeadm_control_plane_webhook.go |  2 +-
 .../kubeadm_control_plane_webhook_test.go     | 77 +++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
index bc2155f9a78b..967deb70e072 100644
--- a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook.go
+++ b/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()
diff --git a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go
index 39604180ec4b..6141154cc6d4 100644
--- a/controlplane/kubeadm/api/v1alpha3/kubeadm_control_plane_webhook_test.go
+++ b/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