From c21ccfb3f2e92f39c639cb1147504f0687ffa1af Mon Sep 17 00:00:00 2001 From: Yuvaraj Kakaraparthi Date: Sat, 20 May 2023 00:15:40 -0700 Subject: [PATCH] add MachineSetPreflightChecks feature flag --- config/manager/manager.yaml | 2 +- feature/feature.go | 6 + .../machineset/machineset_preflight.go | 6 + .../machineset/machineset_preflight_test.go | 650 ++++++++++-------- 4 files changed, 361 insertions(+), 303 deletions(-) diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 4d1b70b16622..7218898cfdeb 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -22,7 +22,7 @@ spec: args: - "--leader-elect" - "--metrics-bind-addr=localhost:8080" - - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=false},ClusterResourceSet=${EXP_CLUSTER_RESOURCE_SET:=false},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},LazyRestmapper=${EXP_LAZY_RESTMAPPER:=false}" + - "--feature-gates=MachinePool=${EXP_MACHINE_POOL:=false},ClusterResourceSet=${EXP_CLUSTER_RESOURCE_SET:=false},ClusterTopology=${CLUSTER_TOPOLOGY:=false},RuntimeSDK=${EXP_RUNTIME_SDK:=false},LazyRestmapper=${EXP_LAZY_RESTMAPPER:=false},MachineSetPreflightChecks=${MACHINESET_PREFLIGHT_CHECKS:=false}" image: controller:latest name: manager env: diff --git a/feature/feature.go b/feature/feature.go index 416594118122..d186d2cd1fb9 100644 --- a/feature/feature.go +++ b/feature/feature.go @@ -60,6 +60,11 @@ const ( // // alpha: v1.4 LazyRestmapper featuregate.Feature = "LazyRestmapper" + + // MachineSetPreflightChecks is a feature fate for the MachineSet preflight checks functionality. + // + // alpha: v1.5 + MachineSetPreflightChecks featuregate.Feature = "MachineSetPreflightChecks" ) func init() { @@ -76,4 +81,5 @@ var defaultClusterAPIFeatureGates = map[featuregate.Feature]featuregate.FeatureS KubeadmBootstrapFormatIgnition: {Default: false, PreRelease: featuregate.Alpha}, RuntimeSDK: {Default: false, PreRelease: featuregate.Alpha}, LazyRestmapper: {Default: false, PreRelease: featuregate.Alpha}, + MachineSetPreflightChecks: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/internal/controllers/machineset/machineset_preflight.go b/internal/controllers/machineset/machineset_preflight.go index d89297687653..2e42e18e5828 100644 --- a/internal/controllers/machineset/machineset_preflight.go +++ b/internal/controllers/machineset/machineset_preflight.go @@ -34,6 +34,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/util" ) @@ -43,6 +44,11 @@ import ( const preflightFailedRequeueAfter = 15 * time.Second func (r *Reconciler) runPreflightChecks(ctx context.Context, cluster *clusterv1.Cluster, ms *clusterv1.MachineSet) (ctrl.Result, error) { + // If the MachineSetPreflightChecks feature gate is disabled return early. + if !feature.Gates.Enabled(feature.MachineSetPreflightChecks) { + return ctrl.Result{}, nil + } + skipped := skippedPreflightChecks(ms) // If all the preflight checks are skipped then return early. if skipped.Has(clusterv1.MachineSetPreflightCheckAll) { diff --git a/internal/controllers/machineset/machineset_preflight_test.go b/internal/controllers/machineset/machineset_preflight_test.go index 28e0886c62ab..30d7350c3a97 100644 --- a/internal/controllers/machineset/machineset_preflight_test.go +++ b/internal/controllers/machineset/machineset_preflight_test.go @@ -23,12 +23,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + utilfeature "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" "sigs.k8s.io/cluster-api/internal/test/builder" ) @@ -55,372 +57,416 @@ func TestMachineSetReconciler_runPreflightChecks(t *testing.T) { }). Build() - tests := []struct { - name string - cluster *clusterv1.Cluster - controlPlane *unstructured.Unstructured - machineSet *clusterv1.MachineSet - wantPass bool - }{ - { - name: "should pass if cluster has no control plane", - cluster: &clusterv1.Cluster{}, - wantPass: true, - }, - { - name: "should pass if the control plane version is not defined", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneWithNoVersion), - }, - }, - controlPlane: controlPlaneWithNoVersion, - wantPass: true, - }, - { - name: "should pass if all preflight checks are skipped", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), - }, + 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)() + + tests := []struct { + name string + cluster *clusterv1.Cluster + controlPlane *unstructured.Unstructured + machineSet *clusterv1.MachineSet + wantPass bool + }{ + { + name: "should pass if cluster has no control plane", + cluster: &clusterv1.Cluster{}, + machineSet: &clusterv1.MachineSet{}, + wantPass: true, }, - controlPlane: controlPlaneUpgrading, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Annotations: map[string]string{ - clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckAll), + { + name: "should pass if the control plane version is not defined", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneWithNoVersion), }, }, + controlPlane: controlPlaneWithNoVersion, + machineSet: &clusterv1.MachineSet{}, + wantPass: true, }, - wantPass: true, - }, - { - name: "control plane preflight check: should fail if the control plane is provisioning", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "should pass if all preflight checks are skipped", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), + }, }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneProvisioning), + controlPlane: controlPlaneUpgrading, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Annotations: map[string]string{ + clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckAll), + }, + }, }, + wantPass: true, }, - controlPlane: controlPlaneProvisioning, - wantPass: false, - }, - { - name: "control plane preflight check: should fail if the control plane is upgrading", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), + { + name: "control plane preflight check: should fail if the control plane is provisioning", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneProvisioning), + }, }, + controlPlane: controlPlaneProvisioning, + machineSet: &clusterv1.MachineSet{}, + wantPass: false, }, - controlPlane: controlPlaneUpgrading, - wantPass: false, - }, - { - name: "control plane preflight check: should pass if the control plane is upgrading but the preflight check is skipped", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), + { + name: "control plane preflight check: should fail if the control plane is upgrading", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), + }, }, + controlPlane: controlPlaneUpgrading, + machineSet: &clusterv1.MachineSet{}, + wantPass: false, }, - controlPlane: controlPlaneUpgrading, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Annotations: map[string]string{ - clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckControlPlaneStable), + { + name: "control plane preflight check: should pass if the control plane is upgrading but the preflight check is skipped", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.26.2"), - Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{Kind: "KubeadmConfigTemplate"}}, + controlPlane: controlPlaneUpgrading, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Annotations: map[string]string{ + clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckControlPlaneStable), + }, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.2"), + Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{Kind: "KubeadmConfigTemplate"}}, + }, }, }, }, + wantPass: true, }, - wantPass: true, - }, - { - name: "control plane preflight check: should pass if the control plane is stable", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{}, - wantPass: true, - }, - { - name: "should pass if the machine set version is not defined", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "control plane preflight check: should pass if the control plane is stable", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.MachineSetSpec{}, + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{}, + wantPass: true, }, - wantPass: true, - }, - { - name: "kubernetes version preflight check: should fail if the machine set minor version is greater than control plane minor version", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "should pass if the machine set version is not defined", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{}, }, + wantPass: true, }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubernetes version preflight check: should fail if the machine set minor version is greater than control plane minor version", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.27.0"), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.27.0"), + }, }, }, }, + wantPass: false, }, - wantPass: false, - }, - { - name: "kubernetes version preflight check: should fail if the machine set minor version is 2 older than control plane minor version", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubernetes version preflight check: should fail if the machine set minor version is 2 older than control plane minor version", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.23.0"), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.23.0"), + }, }, }, }, + wantPass: false, }, - wantPass: false, - }, - { - name: "kubernetes version preflight check: should pass if the machine set minor version is greater than control plane minor version but the preflight check is skipped", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Annotations: map[string]string{ - clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckKubernetesVersion), + { + name: "kubernetes version preflight check: should pass if the machine set minor version is greater than control plane minor version but the preflight check is skipped", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.27.0"), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Annotations: map[string]string{ + clusterv1.MachineSetSkipPreflightChecksAnnotation: string(clusterv1.MachineSetPreflightCheckKubernetesVersion), }, }, - }, - }, - 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", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.25.6"), + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.27.0"), + }, }, }, }, + wantPass: true, }, - wantPass: true, - }, - { - name: "kubeadm version preflight check: should fail if the machine set version is not equal to control plane version when using kubeadm bootstrap provider", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, - }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubernetes version preflight check: should pass if the machine set minor version and control plane version conform to kubernetes version skew policy", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.26.5"), - Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ - APIVersion: bootstrapv1.GroupVersion.String(), - Kind: "KubeadmConfigTemplate", - }}, + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.25.6"), + }, }, }, }, + wantPass: true, }, - wantPass: false, - }, - { - name: "kubeadm version preflight check: should pass if the machine set is not using kubeadm bootstrap provider", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubeadm version preflight check: should fail if the machine set version is not equal to control plane version when using kubeadm bootstrap provider", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.5"), + Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfigTemplate", + }}, + }, + }, + }, }, + wantPass: false, }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubeadm version preflight check: should pass if the machine set is not using kubeadm bootstrap provider", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.26.0"), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.0"), + }, }, }, }, + wantPass: true, }, - wantPass: true, - }, - { - name: "kubeadm version preflight check: should pass if the machine set version and control plane version do not conform to kubeadm version skew policy but the preflight check is skipped", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, + { + name: "kubeadm version preflight check: should pass if the machine set version and control plane version do not conform to kubeadm version skew policy but the preflight check is skipped", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + }, }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Annotations: map[string]string{ + clusterv1.MachineSetSkipPreflightChecksAnnotation: "foobar," + string(clusterv1.MachineSetPreflightCheckKubeadmVersion), + }, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.0"), + Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfigTemplate", + }}, + }, + }, + }, }, + wantPass: true, }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - Annotations: map[string]string{ - clusterv1.MachineSetSkipPreflightChecksAnnotation: "foobar," + string(clusterv1.MachineSetPreflightCheckKubeadmVersion), + { + name: "kubeadm version preflight check: should pass if the machine set version and control plane version conform to kubeadm version skew when using kubeadm bootstrap provider", + cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneStable), }, }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.26.0"), - Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ - APIVersion: bootstrapv1.GroupVersion.String(), - Kind: "KubeadmConfigTemplate", - }}, + controlPlane: controlPlaneStable, + machineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.2"), + Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfigTemplate", + }}, + }, }, }, }, + wantPass: true, }, - wantPass: true, - }, - { - name: "kubeadm version preflight check: should pass if the machine set version and control plane version conform to kubeadm version skew when using kubeadm bootstrap provider", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.ClusterSpec{ - ControlPlaneRef: contract.ObjToRef(controlPlaneStable), - }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + objs := []client.Object{} + if tt.controlPlane != nil { + objs = append(objs, tt.controlPlane) + } + fakeClient := fake.NewClientBuilder().WithObjects(objs...).Build() + r := &Reconciler{Client: fakeClient} + result, err := r.runPreflightChecks(ctx, tt.cluster, tt.machineSet) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.IsZero()).To(Equal(tt.wantPass)) + }) + } + }) + + t.Run("should not run the preflight checks if the feature gate is disabled", func(t *testing.T) { + defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.MachineSetPreflightChecks, false)() + + g := NewWithT(t) + cluster := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, }, - controlPlane: controlPlaneStable, - machineSet: &clusterv1.MachineSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ns, - }, - Spec: clusterv1.MachineSetSpec{ - Template: clusterv1.MachineTemplateSpec{ - Spec: clusterv1.MachineSpec{ - Version: pointer.String("v1.26.2"), - Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ - APIVersion: bootstrapv1.GroupVersion.String(), - Kind: "KubeadmConfigTemplate", - }}, - }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: contract.ObjToRef(controlPlaneUpgrading), + }, + } + controlPlane := controlPlaneUpgrading + machineSet := &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + }, + Spec: clusterv1.MachineSetSpec{ + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + Version: pointer.String("v1.26.0"), + Bootstrap: clusterv1.Bootstrap{ConfigRef: &corev1.ObjectReference{ + APIVersion: bootstrapv1.GroupVersion.String(), + Kind: "KubeadmConfigTemplate", + }}, }, }, }, - wantPass: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - objs := []client.Object{} - if tt.controlPlane != nil { - objs = append(objs, tt.controlPlane) - } - fakeClient := fake.NewClientBuilder().WithObjects(objs...).Build() - r := &Reconciler{Client: fakeClient} - result, err := r.runPreflightChecks(ctx, tt.cluster, tt.machineSet) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(result.IsZero()).To(Equal(tt.wantPass)) - }) - } + } + fakeClient := fake.NewClientBuilder().WithObjects(controlPlane).Build() + r := &Reconciler{Client: fakeClient} + result, err := r.runPreflightChecks(ctx, cluster, machineSet) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result.IsZero()).To(BeTrue()) + }) }