Controller changes to enable modular upgrades for tinkerbell provider (…

…aws#6805) * Enable modular upgrades for tinkerbell provider in CLI Signed-off-by: Rahul Ganesh <[email protected]> * Enable modular upgrades through controller for Tinkerbell Provider. Add a K8s version check to ensure the k8s version matches between the osImageURL andthe K8s version associated with the machine config. Signed-off-by: Rahul Ganesh <[email protected]> --------- Signed-off-by: Rahul Ganesh <[email protected]>
ahreehong · Oct 17, 2023 · 51636f0 · 51636f0
1 parent 4238f47
commit 51636f0
Show file tree

Hide file tree

Showing 55 changed files with 469 additions and 79 deletions.
diff --git a/config/crd/bases/anywhere.eks.amazonaws.com_tinkerbellmachineconfigs.yaml b/config/crd/bases/anywhere.eks.amazonaws.com_tinkerbellmachineconfigs.yaml
@@ -155,7 +155,6 @@ spec:
             required:
             - hardwareSelector
             - osFamily
-            - osImageURL
             type: object
           status:
             description: TinkerbellMachineConfigStatus defines the observed state

diff --git a/internal/test/testdata.go b/internal/test/testdata.go
@@ -205,6 +205,11 @@ func Bundle() *releasev1.Bundles {
 						Name:           "test",
 						EksDReleaseUrl: "embed:///testdata/release.yaml",
 						KubeVersion:    "1.22",
+						Raw: releasev1.OSImageBundle{
+							Bottlerocket: releasev1.Archive{
+								URI: "http://tinkerbell-example:8080/bottlerocket-2004-kube-v1.22.5.gz",
+							},
+						},
 					},
 					CertManager:                releasev1.CertManagerBundle{},
 					ClusterAPI:                 releasev1.CoreClusterAPI{},

diff --git a/pkg/api/v1alpha1/tinkerbellmachineconfig_types.go b/pkg/api/v1alpha1/tinkerbellmachineconfig_types.go
@@ -10,10 +10,11 @@ import (
 
 // TinkerbellMachineConfigSpec defines the desired state of TinkerbellMachineConfig.
 type TinkerbellMachineConfigSpec struct {
-	HardwareSelector    HardwareSelector     `json:"hardwareSelector"`
-	TemplateRef         Ref                  `json:"templateRef,omitempty"`
-	OSFamily            OSFamily             `json:"osFamily"`
-	OSImageURL          string               `json:"osImageURL"`
+	HardwareSelector HardwareSelector `json:"hardwareSelector"`
+	TemplateRef      Ref              `json:"templateRef,omitempty"`
+	OSFamily         OSFamily         `json:"osFamily"`
+	//+optional
+	OSImageURL          string               `json:"osImageURL,omitempty"`
 	Users               []UserConfiguration  `json:"users,omitempty"`
 	HostOSConfiguration *HostOSConfiguration `json:"hostOSConfiguration,omitempty"`
 }

diff --git a/pkg/api/v1alpha1/tinkerbelltemplateconfig_defaults.go b/pkg/api/v1alpha1/tinkerbelltemplateconfig_defaults.go
@@ -100,17 +100,8 @@ func GetDefaultActionsFromBundle(clusterSpec *Cluster, b v1alpha1.VersionsBundle
 	return actions
 }
 
-func withStreamImageAction(b v1alpha1.VersionsBundle, disk, osImageOverride string, additionalEnvVar map[string]string) ActionOpt {
+func withStreamImageAction(b v1alpha1.VersionsBundle, disk, imageURL string, additionalEnvVar map[string]string) ActionOpt {
 	return func(a *[]tinkerbell.Action) {
-		var imageURL string
-
-		switch {
-		case osImageOverride != "":
-			imageURL = osImageOverride
-		default:
-			imageURL = b.EksD.Raw.Bottlerocket.URI
-		}
-
 		env := map[string]string{
 			"DEST_DISK":  disk,
 			"IMG_URL":    imageURL,

diff --git a/pkg/api/v1alpha1/tinkerbelltemplateconfig_defaults_test.go b/pkg/api/v1alpha1/tinkerbelltemplateconfig_defaults_test.go
@@ -32,9 +32,10 @@ warnings:
 		wantActions     []tinkerbell.Action
 	}{
 		{
-			testName:    "Bottlerocket-sda",
-			osFamily:    Bottlerocket,
-			clusterSpec: &Cluster{},
+			testName:        "Bottlerocket-sda",
+			osFamily:        Bottlerocket,
+			osImageOverride: "http://tinkerbell-example:8080/bottlerocket-2004-kube-v1.21.5.gz",
+			clusterSpec:     &Cluster{},
 			wantActions: []tinkerbell.Action{
 				{
 					Name:    "stream-image",
@@ -105,9 +106,10 @@ warnings:
 			},
 		},
 		{
-			testName:    "Bottlerocket-nvme",
-			osFamily:    Bottlerocket,
-			clusterSpec: &Cluster{},
+			testName:        "Bottlerocket-nvme",
+			osFamily:        Bottlerocket,
+			osImageOverride: "http://tinkerbell-example:8080/bottlerocket-2004-kube-v1.21.5.gz",
+			clusterSpec:     &Cluster{},
 			wantActions: []tinkerbell.Action{
 				{
 					Name:    "stream-image",

diff --git a/pkg/providers/tinkerbell/assert_test.go b/pkg/providers/tinkerbell/assert_test.go
@@ -16,6 +16,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 
+	"github.com/aws/eks-anywhere/internal/test"
 	eksav1alpha1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1"
 	"github.com/aws/eks-anywhere/pkg/clusterapi"
 	"github.com/aws/eks-anywhere/pkg/networkutils/mocks"
@@ -148,14 +149,194 @@ func TestAssertMachineConfigOSImageURLSpecified_Succeed(t *testing.T) {
 	clusterSpec := builder.Build()
 	clusterSpec.Spec.Cluster.Spec.ExternalEtcdConfiguration = nil
 	clusterSpec.DatacenterConfig.Spec.OSImageURL = ""
+	clusterSpec.Spec.Cluster.Spec.KubernetesVersion = "1.22"
 	// set OsImageURL at machineConfig level but not for all machine configs
-	clusterSpec.MachineConfigs[builder.ControlPlaneMachineName].Spec.OSImageURL = "test-url"
-	clusterSpec.MachineConfigs[builder.ExternalEtcdMachineName].Spec.OSImageURL = "test-url"
-	clusterSpec.MachineConfigs[builder.WorkerNodeGroupMachineName].Spec.OSImageURL = "test-url"
+	clusterSpec.MachineConfigs[builder.ControlPlaneMachineName].Spec.OSImageURL = "test-url-122"
+	clusterSpec.MachineConfigs[builder.ExternalEtcdMachineName].Spec.OSImageURL = "test-url-122"
+	clusterSpec.MachineConfigs[builder.WorkerNodeGroupMachineName].Spec.OSImageURL = "test-url-122"
 	err := tinkerbell.AssertOSImageURL(clusterSpec)
 	g.Expect(err).To(gomega.Succeed())
 }
 
+func TestK8sVersionInDataCenterOSImageURL_Succeed(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	for name, spec := range map[string]func(*tinkerbell.ClusterSpec){
+		"validate'.'specifier": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url-1.22"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+		"validate'-'specifier": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url-1-22"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+		"validate'_'specifier": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url-1_22"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+		"validate''specifier": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url-122"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			cluster := NewDefaultValidClusterSpecBuilder().Build()
+			spec(cluster)
+			g.Expect(tinkerbell.AssertOSImageURL(cluster)).To(gomega.Succeed())
+		})
+	}
+}
+
+func TestK8sVersionInDataCenterOSImageURL_Error(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	for name, spec := range map[string]func(*tinkerbell.ClusterSpec){
+		"noK8sVersion": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+		"invalidSpecifierinK8sVersion": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = "test-url-1/22"
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+		"emptyImageURL": func(c *tinkerbell.ClusterSpec) {
+			c.DatacenterConfig.Spec.OSImageURL = ""
+			c.Cluster.Spec.KubernetesVersion = kube122
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			cluster := NewDefaultValidClusterSpecBuilder().Build()
+			spec(cluster)
+			g.Expect(tinkerbell.AssertOSImageURL(cluster)).ToNot(gomega.Succeed())
+		})
+	}
+}
+
+func TestK8sVersionInMachineConfigOSImageURL_Succeed(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	kube123 := eksav1alpha1.Kube123
+	for name, spec := range map[string]func(*tinkerbell.ClusterSpec){
+		"validate'.'specifier": func(c *tinkerbell.ClusterSpec) {
+			for _, mcRef := range c.Cluster.MachineConfigRefs() {
+				c.MachineConfigs[mcRef.Name].Spec.OSImageURL = "test-url-1.22"
+				c.Cluster.Spec.KubernetesVersion = kube122
+			}
+		},
+		"validate'-'specifier": func(c *tinkerbell.ClusterSpec) {
+			for _, mcRef := range c.Cluster.MachineConfigRefs() {
+				c.MachineConfigs[mcRef.Name].Spec.OSImageURL = "test-url-1-22"
+				c.Cluster.Spec.KubernetesVersion = kube122
+			}
+		},
+		"validate'_'specifier": func(c *tinkerbell.ClusterSpec) {
+			for _, mcRef := range c.Cluster.MachineConfigRefs() {
+				c.MachineConfigs[mcRef.Name].Spec.OSImageURL = "test-url-1-22"
+				c.Cluster.Spec.KubernetesVersion = kube122
+			}
+		},
+		"validate''specifier": func(c *tinkerbell.ClusterSpec) {
+			for _, mcRef := range c.Cluster.MachineConfigRefs() {
+				c.MachineConfigs[mcRef.Name].Spec.OSImageURL = "test-url-122"
+				c.Cluster.Spec.KubernetesVersion = kube122
+			}
+		},
+		"validateCPWorkerDiffVersion": func(c *tinkerbell.ClusterSpec) {
+			c.Cluster.Spec.KubernetesVersion = kube123
+			c.Cluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube122
+			c.ControlPlaneMachineConfig().Spec.OSImageURL = "test-url-123"
+			c.ExternalEtcdMachineConfig().Spec.OSImageURL = "test-url"
+			wngRef := c.WorkerNodeGroupConfigurations()[0].MachineGroupRef.Name
+			c.MachineConfigs[wngRef].Spec.OSImageURL = "test-url-122"
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			cluster := NewDefaultValidClusterSpecBuilder().Build()
+			cluster.DatacenterConfig.Spec.OSImageURL = ""
+			spec(cluster)
+			g.Expect(tinkerbell.AssertOSImageURL(cluster)).To(gomega.Succeed())
+		})
+	}
+}
+
+func TestK8sVersionInMachineConfigOSImageURL_Error(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	kube123 := eksav1alpha1.Kube123
+	for name, spec := range map[string]func(*tinkerbell.ClusterSpec){
+		"validateCPVersionError": func(c *tinkerbell.ClusterSpec) {
+			c.Cluster.Spec.KubernetesVersion = kube123
+			c.Cluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube122
+			c.ControlPlaneMachineConfig().Spec.OSImageURL = "test-url-122"
+			wngRef := c.WorkerNodeGroupConfigurations()[0].MachineGroupRef.Name
+			c.MachineConfigs[wngRef].Spec.OSImageURL = "test-url-122"
+		},
+		"validateWorkerVersionError": func(c *tinkerbell.ClusterSpec) {
+			c.Cluster.Spec.KubernetesVersion = kube123
+			c.Cluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube123
+			c.ControlPlaneMachineConfig().Spec.OSImageURL = "test-url-123"
+			wngRef := c.WorkerNodeGroupConfigurations()[0].MachineGroupRef.Name
+			c.MachineConfigs[wngRef].Spec.OSImageURL = "test-url-122"
+		},
+		"validateCPWorkerSameVersionError": func(c *tinkerbell.ClusterSpec) {
+			c.Cluster.Spec.KubernetesVersion = kube123
+			c.ControlPlaneMachineConfig().Spec.OSImageURL = "test-url-123"
+			wngRef := c.WorkerNodeGroupConfigurations()[0].MachineGroupRef.Name
+			c.MachineConfigs[wngRef].Spec.OSImageURL = "test-url-122"
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			cluster := NewDefaultValidClusterSpecBuilder().Build()
+			spec(cluster)
+			g.Expect(tinkerbell.AssertOSImageURL(cluster)).ToNot(gomega.Succeed())
+		})
+	}
+}
+
+func TestK8sVersionInCPMachineConfigOSImageURL_Error(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	builder := NewDefaultValidClusterSpecBuilder()
+	clusterSpec := builder.Build()
+	clusterSpec.Spec.Cluster.Spec.KubernetesVersion = kube122
+	clusterSpec.DatacenterConfig.Spec.OSImageURL = ""
+	clusterSpec.MachineConfigs[builder.ControlPlaneMachineName].Spec.OSImageURL = "test-url"
+	clusterSpec.MachineConfigs[builder.ExternalEtcdMachineName].Spec.OSImageURL = "test-url-122"
+	clusterSpec.MachineConfigs[builder.WorkerNodeGroupMachineName].Spec.OSImageURL = "test-url-122"
+	g.Expect(tinkerbell.AssertOSImageURL(clusterSpec)).To(gomega.MatchError(gomega.ContainSubstring("missing kube version from control plane machine config OSImageURL:")))
+}
+
+func TestK8sVersionInWorkerMachineConfigOSImageURL_Error(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube122 := eksav1alpha1.Kube122
+	builder := NewDefaultValidClusterSpecBuilder()
+	clusterSpec := builder.Build()
+	clusterSpec.Spec.Cluster.Spec.KubernetesVersion = kube122
+	clusterSpec.DatacenterConfig.Spec.OSImageURL = ""
+	clusterSpec.MachineConfigs[builder.ControlPlaneMachineName].Spec.OSImageURL = "test-url-122"
+	clusterSpec.MachineConfigs[builder.ExternalEtcdMachineName].Spec.OSImageURL = "test-url-122"
+	clusterSpec.MachineConfigs[builder.WorkerNodeGroupMachineName].Spec.OSImageURL = "test-url"
+	g.Expect(tinkerbell.AssertOSImageURL(clusterSpec)).To(gomega.MatchError(gomega.ContainSubstring("missing kube version from worker node group machine config OSImageURL:")))
+}
+
+func TestK8sVersionForBRAutoImport_Succeed(t *testing.T) {
+	g := gomega.NewWithT(t)
+	kube123 := eksav1alpha1.Kube123
+	kube122 := eksav1alpha1.Kube122
+	builder := NewDefaultValidClusterSpecBuilder()
+	clusterSpec := builder.Build()
+	clusterSpec.Spec.Cluster.Spec.KubernetesVersion = kube123
+	clusterSpec.Spec.Cluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube122
+	clusterSpec.DatacenterConfig.Spec.OSImageURL = ""
+	for _, mc := range clusterSpec.MachineConfigs {
+		mc.Spec.OSFamily = eksav1alpha1.Bottlerocket
+	}
+	clusterSpec.VersionsBundles = test.VersionsBundlesMap()
+	clusterSpec.VersionsBundle(kube122).EksD.Raw.Bottlerocket.URI = "br-122"
+	clusterSpec.VersionsBundle(kube123).EksD.Raw.Bottlerocket.URI = "br-123"
+	g.Expect(tinkerbell.AssertOSImageURL(clusterSpec)).To(gomega.Succeed())
+}
+
 func TestAssertEtcdMachineRefExists_Exists(t *testing.T) {
 	g := gomega.NewWithT(t)
 	clusterSpec := NewDefaultValidClusterSpecBuilder().Build()

diff --git a/pkg/providers/tinkerbell/reconciler/reconciler.go b/pkg/providers/tinkerbell/reconciler/reconciler.go
@@ -182,6 +182,16 @@ func (r *Reconciler) DetectOperation(ctx context.Context, log logr.Logger, tinke
 		return K8sVersionUpgradeOperation, nil
 	}
 
+	for _, wg := range tinkerbellScope.Workers.Groups {
+		machineDeployment, err := controller.GetMachineDeployment(ctx, r.client, wg.MachineDeployment.GetName())
+		if err != nil {
+			return "", errors.Wrap(err, "failed to get workernode group machinedeployment")
+		}
+		if machineDeployment != nil && (*machineDeployment.Spec.Template.Spec.Version != *wg.MachineDeployment.Spec.Template.Spec.Version) {
+			log.Info("Operation detected", "operation", K8sVersionUpgradeOperation)
+			return K8sVersionUpgradeOperation, nil
+		}
+	}
 	log.Info("Operation detected", "operation", NoChange)
 	return NoChange, nil
 }

diff --git a/pkg/providers/tinkerbell/reconciler/reconciler_test.go b/pkg/providers/tinkerbell/reconciler/reconciler_test.go
@@ -63,16 +63,18 @@ func TestReconcilerReconcileSuccess(t *testing.T) {
 	tt.eksaSupportObjs = append(tt.eksaSupportObjs, tinkHardware("hw1", "cp"))
 	tt.eksaSupportObjs = append(tt.eksaSupportObjs, tinkHardware("hw2", "worker"))
 	tt.createAllObjs()
-
 	logger := test.NewNullLogger()
 	remoteClient := env.Client()
-
 	tt.ipValidator.EXPECT().ValidateControlPlaneIP(tt.ctx, logger, tt.buildSpec()).Return(controller.Result{}, nil)
 
 	tt.remoteClientRegistry.EXPECT().GetClient(
 		tt.ctx, client.ObjectKey{Name: workloadClusterName, Namespace: constants.EksaSystemNamespace},
 	).Return(remoteClient, nil)
-	tt.cniReconciler.EXPECT().Reconcile(tt.ctx, logger, remoteClient, tt.buildSpec())
+	spec := tt.buildSpec()
+	for _, mc := range spec.TinkerbellMachineConfigs {
+		mc.Spec.OSImageURL = "http://tinkerbell-example:8080/bottlerocket-2004-kube-v1.22.5.gz"
+	}
+	tt.cniReconciler.EXPECT().Reconcile(tt.ctx, logger, remoteClient, spec)
 
 	result, err := tt.reconciler().Reconcile(tt.ctx, logger, tt.cluster)
 
@@ -671,6 +673,44 @@ func TestReconcilerDetectOperationK8sVersionUpgrade(t *testing.T) {
 	tt.cleanup()
 }
 
+func TestReconcilerDetectOperationK8sVersionUpgradeCPOnly(t *testing.T) {
+	tt := newReconcilerTest(t)
+	tt.createAllObjs()
+
+	logger := test.NewNullLogger()
+	scope := tt.buildScope()
+	_, err := tt.reconciler().GenerateSpec(tt.ctx, logger, scope)
+	tt.Expect(err).NotTo(HaveOccurred())
+	scope.ControlPlane = tinkerbellCP(tt.cluster.Name)
+	kube123 := "v1.23.8"
+	scope.ControlPlane.KubeadmControlPlane.Spec.Version = kube123
+	op, err := tt.reconciler().DetectOperation(tt.ctx, logger, scope)
+	tt.Expect(err).NotTo(HaveOccurred())
+	tt.Expect(op).To(Equal(reconciler.K8sVersionUpgradeOperation))
+	tt.cleanup()
+}
+
+func TestReconcilerDetectOperationK8sVersionUpgradeWorkerOnly(t *testing.T) {
+	tt := newReconcilerTest(t)
+	tt.createAllObjs()
+
+	logger := test.NewNullLogger()
+	scope := tt.buildScope()
+
+	_, err := tt.reconciler().GenerateSpec(tt.ctx, logger, scope)
+	kube123 := "v1.23.8"
+	scope.Workers = tinkWorker(tt.cluster.Name, func(w *tinkerbell.Workers) {
+		w.Groups[0].MachineDeployment.Spec.Template.Spec.Version = &kube123
+	})
+
+	tt.Expect(err).NotTo(HaveOccurred())
+
+	op, err := tt.reconciler().DetectOperation(tt.ctx, logger, scope)
+	tt.Expect(err).NotTo(HaveOccurred())
+	tt.Expect(op).To(Equal(reconciler.K8sVersionUpgradeOperation))
+	tt.cleanup()
+}
+
 func TestReconcilerDetectOperationExistingWorkerNodeGroupScaleUpdate(t *testing.T) {
 	tt := newReconcilerTest(t)
 	tt.createAllObjs()
@@ -813,7 +853,7 @@ func newReconcilerTest(t testing.TB) *reconcilerTest {
 
 	bundle := test.Bundle()
 	version := test.DevEksaVersion()
-
+	kube122 := anywherev1.Kube122
 	managementClusterDatacenter := dataCenter(func(d *anywherev1.TinkerbellDatacenterConfig) {
 		d.Name = "management-datacenter"
 	})
@@ -878,11 +918,12 @@ func newReconcilerTest(t testing.TB) *reconcilerTest {
 					Kind: anywherev1.TinkerbellMachineConfigKind,
 					Name: machineConfigWN.Name,
 				},
-				Name:   "md-0",
-				Labels: nil,
+				Name:              "md-0",
+				Labels:            nil,
+				KubernetesVersion: &kube122,
 			},
 		)
-
+		c.Spec.KubernetesVersion = kube122
 		c.Spec.EksaVersion = &version
 	})