From 8f665ec4b3b921591dab312fe3fced196c6af75d Mon Sep 17 00:00:00 2001
From: Christian Schlotter <christi.schlotter@gmail.com>
Date: Thu, 17 Aug 2023 13:13:19 +0200
Subject: [PATCH] machineset: adjust preflight check to allow kubelet version
 skew of 3 for clusters running v1.28 and above

---
 .../machineset/machineset_preflight.go        | 13 +++-
 .../machineset/machineset_preflight_test.go   | 61 ++++++++++++++++++-
 2 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/internal/controllers/machineset/machineset_preflight.go b/internal/controllers/machineset/machineset_preflight.go
index 00d5d67fc8f2..591f7b8ca09a 100644
--- a/internal/controllers/machineset/machineset_preflight.go
+++ b/internal/controllers/machineset/machineset_preflight.go
@@ -45,6 +45,8 @@ type preflightCheckErrorMessage *string
 // the preflight checks fail.
 const preflightFailedRequeueAfter = 15 * time.Second
 
+var minVerKubernetesKubeletVersionSkewThree = semver.MustParse("1.28.0")
+
 func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet, action string) (_ ctrl.Result, message string, retErr error) {
 	log := ctrl.LoggerFrom(ctx)
 	// If the MachineSetPreflightChecks feature gate is disabled return early.
@@ -168,13 +170,18 @@ func (r *Reconciler) controlPlaneStablePreflightCheck(controlPlane *unstructured
 func (r *Reconciler) kubernetesVersionPreflightCheck(cpSemver, msSemver semver.Version) preflightCheckErrorMessage {
 	// Check the Kubernetes version skew policy.
 	// => MS minor version cannot be greater than the Control Plane minor version.
-	// => MS minor version cannot be older than 2 minor versions of Control Plane.
+	// => MS minor version cannot be outside of the supported skew.
 	// Kubernetes skew policy: https://kubernetes.io/releases/version-skew-policy/#kubelet
 	if msSemver.Minor > cpSemver.Minor {
 		return pointer.String(fmt.Sprintf("MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy as MachineSet version is higher than ControlPlane version (%q preflight failed)", msSemver.String(), cpSemver.String(), clusterv1.MachineSetPreflightCheckKubernetesVersionSkew))
 	}
-	if msSemver.Minor < cpSemver.Minor-2 {
-		return pointer.String(fmt.Sprintf("MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy as MachineSet version is more than 2 minor versions older than the ControlPlane version (%q preflight failed)", msSemver.String(), cpSemver.String(), clusterv1.MachineSetPreflightCheckKubernetesVersionSkew))
+	minorSkew := uint64(3)
+	// For Control Planes running Kubernetes < v1.28, the version skew policy for kubelets is two.
+	if cpSemver.LT(minVerKubernetesKubeletVersionSkewThree) {
+		minorSkew = 2
+	}
+	if msSemver.Minor < cpSemver.Minor-minorSkew {
+		return pointer.String(fmt.Sprintf("MachineSet version (%s) and ControlPlane version (%s) do not conform to the kubernetes version skew policy as MachineSet version is more than %d minor versions older than the ControlPlane version (%q preflight failed)", msSemver.String(), cpSemver.String(), minorSkew, clusterv1.MachineSetPreflightCheckKubernetesVersionSkew))
 	}
 
 	return nil
diff --git a/internal/controllers/machineset/machineset_preflight_test.go b/internal/controllers/machineset/machineset_preflight_test.go
index 42e911c382fc..cd24ae16d144 100644
--- a/internal/controllers/machineset/machineset_preflight_test.go
+++ b/internal/controllers/machineset/machineset_preflight_test.go
@@ -60,6 +60,13 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) {
 		}).
 		Build()
 
+	controlPlaneStable128 := builder.ControlPlane(ns, "cp1").
+		WithVersion("v1.28.0").
+		WithStatusFields(map[string]interface{}{
+			"status.version": "v1.28.0",
+		}).
+		Build()
+
 	t.Run("should run preflight checks if the feature gate is enabled", func(t *testing.T) {
 		defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineSetPreflightChecks, true)()
 
@@ -267,7 +274,32 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) {
 				wantPass: false,
 			},
 			{
-				name: "kubernetes version preflight check: should fail if the machine set minor version is 2 older than control plane minor version",
+				name: "kubernetes version preflight check: should fail if the machine set minor version is 4 older than control plane minor version for >= v1.28",
+				cluster: &clusterv1.Cluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+					},
+					Spec: clusterv1.ClusterSpec{
+						ControlPlaneRef: contract.ObjToRef(controlPlaneStable128),
+					},
+				},
+				controlPlane: controlPlaneStable128,
+				machineSet: &clusterv1.MachineSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+					},
+					Spec: clusterv1.MachineSetSpec{
+						Template: clusterv1.MachineTemplateSpec{
+							Spec: clusterv1.MachineSpec{
+								Version: pointer.String("v1.24.0"),
+							},
+						},
+					},
+				},
+				wantPass: false,
+			},
+			{
+				name: "kubernetes version preflight check: should fail if the machine set minor version is 3 older than control plane minor version for < v1.28",
 				cluster: &clusterv1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: ns,
@@ -320,7 +352,32 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) {
 				wantPass: true,
 			},
 			{
-				name: "kubernetes version preflight check: should pass if the machine set minor version and control plane version conform to kubernetes version skew policy",
+				name: "kubernetes version preflight check: should pass if the machine set minor version and control plane version conform to kubernetes version skew policy >= v1.28",
+				cluster: &clusterv1.Cluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+					},
+					Spec: clusterv1.ClusterSpec{
+						ControlPlaneRef: contract.ObjToRef(controlPlaneStable128),
+					},
+				},
+				controlPlane: controlPlaneStable128,
+				machineSet: &clusterv1.MachineSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: ns,
+					},
+					Spec: clusterv1.MachineSetSpec{
+						Template: clusterv1.MachineTemplateSpec{
+							Spec: clusterv1.MachineSpec{
+								Version: pointer.String("v1.25.0"),
+							},
+						},
+					},
+				},
+				wantPass: true,
+			},
+			{
+				name: "kubernetes version preflight check: should pass if the machine set minor version and control plane version conform to kubernetes version skew policy < v1.28",
 				cluster: &clusterv1.Cluster{
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace: ns,