Facilitate modular upgrades for tinkerbell provider

Signed-off-by: Rahul Ganesh <[email protected]>
aws · Sep 26, 2023 · a554fe6 · a554fe6
1 parent 4ba4843
commit a554fe6
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 37 deletions.
diff --git a/pkg/api/v1alpha1/cluster_webhook.go b/pkg/api/v1alpha1/cluster_webhook.go
@@ -473,8 +473,6 @@ func validateKubeVersionSkew(newVersion, oldVersion KubernetesVersion, path *fie
 // ValidateWorkerKubernetesVersionSkew validates worker node group Kubernetes version skew between upgrades.
 func ValidateWorkerKubernetesVersionSkew(new, old *Cluster) field.ErrorList {
 	var allErrs field.ErrorList
-	path := field.NewPath("spec").Child("WorkerNodeConfiguration.kubernetesVersion")
-
 	newClusterVersion := new.Spec.KubernetesVersion
 	oldClusterVersion := old.Spec.KubernetesVersion
 
@@ -485,11 +483,6 @@ func ValidateWorkerKubernetesVersionSkew(new, old *Cluster) field.ErrorList {
 	for _, nodeGroupNewSpec := range new.Spec.WorkerNodeGroupConfigurations {
 		newVersion := nodeGroupNewSpec.KubernetesVersion
 
-		if newVersion != nil && nodeGroupNewSpec.MachineGroupRef.Kind == TinkerbellMachineConfigKind {
-			allErrs = append(allErrs, field.Forbidden(path, "worker node group level kubernetesVersion is not supported for Tinkerbell"))
-			return allErrs
-		}
-
 		if workerNodeGrpOldSpec, ok := workerNodeGroupMap[nodeGroupNewSpec.Name]; ok {
 			oldVersion := workerNodeGrpOldSpec.KubernetesVersion
 			allErrs = append(allErrs, performWorkerKubernetesValidations(oldVersion, newVersion, oldClusterVersion, newClusterVersion)...)

diff --git a/pkg/api/v1alpha1/cluster_webhook_test.go b/pkg/api/v1alpha1/cluster_webhook_test.go
@@ -2317,29 +2317,29 @@ func TestValidateWorkerVersionSkewAddNodeGroup(t *testing.T) {
 	g.Expect(err).To(Succeed())
 }
 
-func TestValidateWorkerVersionBlockTinkerbell(t *testing.T) {
-	kube119 := v1alpha1.KubernetesVersion("1.19")
-
-	newCluster := baseCluster()
-	newCluster.Spec.KubernetesVersion = kube119
-	newCluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube119
-	newCluster.Spec.WorkerNodeGroupConfigurations[0].MachineGroupRef.Kind = v1alpha1.TinkerbellMachineConfigKind
-	newWorker := v1alpha1.WorkerNodeGroupConfiguration{
-		Name:  "md-1",
-		Count: ptr.Int(1),
-		MachineGroupRef: &v1alpha1.Ref{
-			Kind: v1alpha1.TinkerbellMachineConfigKind,
-			Name: "eksa-unit-test",
-		},
-		KubernetesVersion: &kube119,
-	}
-	newCluster.Spec.WorkerNodeGroupConfigurations = append(newCluster.Spec.WorkerNodeGroupConfigurations, newWorker)
-
-	oldCluster := baseCluster()
-	oldCluster.Spec.KubernetesVersion = kube119
-	oldCluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube119
-
-	err := newCluster.ValidateUpdate(oldCluster)
-	g := NewWithT(t)
-	g.Expect(err).ToNot(BeNil())
-}
+// func TestValidateWorkerVersionBlockTinkerbell(t *testing.T) {
+// 	kube119 := v1alpha1.KubernetesVersion("1.19")
+
+// 	newCluster := baseCluster()
+// 	newCluster.Spec.KubernetesVersion = kube119
+// 	newCluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube119
+// 	newCluster.Spec.WorkerNodeGroupConfigurations[0].MachineGroupRef.Kind = v1alpha1.TinkerbellMachineConfigKind
+// 	newWorker := v1alpha1.WorkerNodeGroupConfiguration{
+// 		Name:  "md-1",
+// 		Count: ptr.Int(1),
+// 		MachineGroupRef: &v1alpha1.Ref{
+// 			Kind: v1alpha1.TinkerbellMachineConfigKind,
+// 			Name: "eksa-unit-test",
+// 		},
+// 		KubernetesVersion: &kube119,
+// 	}
+// 	newCluster.Spec.WorkerNodeGroupConfigurations = append(newCluster.Spec.WorkerNodeGroupConfigurations, newWorker)
+
+// 	oldCluster := baseCluster()
+// 	oldCluster.Spec.KubernetesVersion = kube119
+// 	oldCluster.Spec.WorkerNodeGroupConfigurations[0].KubernetesVersion = &kube119
+
+// 	err := newCluster.ValidateUpdate(oldCluster)
+// 	g := NewWithT(t)
+// 	g.Expect(err).ToNot(BeNil())
+// }
diff --git a/pkg/api/v1alpha1/tinkerbellmachineconfig.go b/pkg/api/v1alpha1/tinkerbellmachineconfig.go
@@ -2,6 +2,7 @@ package v1alpha1
 
 import (
 	"fmt"
+	"net/url"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -91,6 +92,12 @@ func validateTinkerbellMachineConfig(config *TinkerbellMachineConfig) error {
 		)
 	}
 
+	if config.Spec.OSImageURL != "" {
+		if _, err := url.ParseRequestURI(config.Spec.OSImageURL); err != nil {
+			return fmt.Errorf("parsing osImageOverride: %v", err)
+		}
+	}
+
 	if len(config.Spec.Users) == 0 {
 		return fmt.Errorf("TinkerbellMachineConfig: missing spec.Users: %s", config.Name)
 	}

diff --git a/pkg/api/v1alpha1/tinkerbellmachineconfig_types.go b/pkg/api/v1alpha1/tinkerbellmachineconfig_types.go
@@ -13,6 +13,7 @@ type TinkerbellMachineConfigSpec struct {
 	HardwareSelector    HardwareSelector     `json:"hardwareSelector"`
 	TemplateRef         Ref                  `json:"templateRef,omitempty"`
 	OSFamily            OSFamily             `json:"osFamily"`
+	OSImageURL          string               `json:"osImageURL"`
 	Users               []UserConfiguration  `json:"users,omitempty"`
 	HostOSConfiguration *HostOSConfiguration `json:"hostOSConfiguration,omitempty"`
 }

diff --git a/pkg/providers/tinkerbell/assert.go b/pkg/providers/tinkerbell/assert.go
@@ -90,6 +90,11 @@ func AssertOsFamilyValid(spec *ClusterSpec) error {
 	return validateOsFamily(spec)
 }
 
+// AssertOSImageURLDontOverlap ensures that the OSImageURL value is either set at the datacenter config level or set for each machine config and not at both levels.
+func AssertOSImageURLDontOverlap(spec *ClusterSpec) error {
+	return validateOSImageURLDontOverlap(spec)
+}
+
 // AssertcontrolPlaneIPNotInUse ensures the endpoint host for the control plane isn't in use.
 // The check may be unreliable due to its implementation.
 func NewIPNotInUseAssertion(client networkutils.NetClient) ClusterSpecAssertion {

diff --git a/pkg/providers/tinkerbell/cluster.go b/pkg/providers/tinkerbell/cluster.go
@@ -108,6 +108,7 @@ func NewClusterSpecValidator(assertions ...ClusterSpecAssertion) *ClusterSpecVal
 		AssertMachineConfigsValid,
 		AssertMachineConfigNamespaceMatchesDatacenterConfig,
 		AssertOsFamilyValid,
+		AssertOSImageURLDontOverlap,
 		AssertTinkerbellIPAndControlPlaneIPNotSame,
 		AssertHookRetrievableWithoutProxy,
 	)

diff --git a/pkg/providers/tinkerbell/template.go b/pkg/providers/tinkerbell/template.go
@@ -69,9 +69,16 @@ func (tb *TemplateBuilder) GenerateCAPISpecControlPlane(clusterSpec *cluster.Spe
 	if err != nil {
 		return nil, err
 	}
+	var OSImageURL string
+	if clusterSpec.TinkerbellDatacenter.Spec.OSImageURL != "" {
+		OSImageURL = clusterSpec.TinkerbellDatacenter.Spec.OSImageURL
+	} else {
+		OSImageURL = tb.controlPlaneMachineSpec.OSImageURL
+	}
+
 	if cpTemplateConfig == nil {
 		versionBundle := bundle.VersionsBundle
-		cpTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, tb.datacenterSpec.OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, tb.controlPlaneMachineSpec.OSFamily)
+		cpTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, tb.controlPlaneMachineSpec.OSFamily)
 	}
 
 	cpTemplateString, err := cpTemplateConfig.ToTemplateString()
@@ -81,12 +88,16 @@ func (tb *TemplateBuilder) GenerateCAPISpecControlPlane(clusterSpec *cluster.Spe
 
 	var etcdMachineSpec v1alpha1.TinkerbellMachineConfigSpec
 	var etcdTemplateString string
+
 	if clusterSpec.Cluster.Spec.ExternalEtcdConfiguration != nil {
 		etcdMachineSpec = *tb.etcdMachineSpec
+		if etcdMachineSpec.OSImageURL != "" {
+			OSImageURL = etcdMachineSpec.OSImageURL
+		}
 		etcdTemplateConfig := clusterSpec.TinkerbellTemplateConfigs[tb.etcdMachineSpec.TemplateRef.Name]
 		if etcdTemplateConfig == nil {
 			versionBundle := bundle.VersionsBundle
-			etcdTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, tb.datacenterSpec.OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, tb.etcdMachineSpec.OSFamily)
+			etcdTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, tb.etcdMachineSpec.OSFamily)
 		}
 		etcdTemplateString, err = etcdTemplateConfig.ToTemplateString()
 		if err != nil {
@@ -111,12 +122,19 @@ func (tb *TemplateBuilder) GenerateCAPISpecControlPlane(clusterSpec *cluster.Spe
 func (tb *TemplateBuilder) GenerateCAPISpecWorkers(clusterSpec *cluster.Spec, workloadTemplateNames, kubeadmconfigTemplateNames map[string]string) (content []byte, err error) {
 	workerSpecs := make([][]byte, 0, len(clusterSpec.Cluster.Spec.WorkerNodeGroupConfigurations))
 	bundle := clusterSpec.RootVersionsBundle()
+	var OSImageURL string
+	if clusterSpec.TinkerbellDatacenter.Spec.OSImageURL != "" {
+		OSImageURL = clusterSpec.TinkerbellDatacenter.Spec.OSImageURL
+	}
 	for _, workerNodeGroupConfiguration := range clusterSpec.Cluster.Spec.WorkerNodeGroupConfigurations {
 		workerNodeMachineSpec := tb.WorkerNodeGroupMachineSpecs[workerNodeGroupConfiguration.MachineGroupRef.Name]
 		wTemplateConfig := clusterSpec.TinkerbellTemplateConfigs[workerNodeMachineSpec.TemplateRef.Name]
 		if wTemplateConfig == nil {
 			versionBundle := bundle.VersionsBundle
-			wTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, tb.datacenterSpec.OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, workerNodeMachineSpec.OSFamily)
+			if workerNodeMachineSpec.OSImageURL != "" {
+				OSImageURL = workerNodeMachineSpec.OSImageURL
+			}
+			wTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, workerNodeMachineSpec.OSFamily)
 		}
 
 		wTemplateString, err := wTemplateConfig.ToTemplateString()

diff --git a/pkg/providers/tinkerbell/validate.go b/pkg/providers/tinkerbell/validate.go
@@ -29,12 +29,33 @@ func validateOsFamily(spec *ClusterSpec) error {
 		}
 	}
 
-	if controlPlaneOsFamily != v1alpha1.Bottlerocket && spec.DatacenterConfig.Spec.OSImageURL == "" {
+	if controlPlaneOsFamily != v1alpha1.Bottlerocket && spec.DatacenterConfig.Spec.OSImageURL == "" && spec.ControlPlaneMachineConfig().Spec.OSImageURL == "" {
 		return fmt.Errorf("please use bottlerocket as osFamily for auto-importing or provide a valid osImageURL")
 	}
 	return nil
 }
 
+func validateOSImageURLDontOverlap(spec *ClusterSpec) error {
+	var dcOSImageURLfound bool
+	if spec.TinkerbellDatacenter.Spec.OSImageURL != "" {
+		dcOSImageURLfound = true
+	}
+	return validateMachineCfgOSImageURL(spec.TinkerbellMachineConfigs, dcOSImageURLfound)
+}
+
+func validateMachineCfgOSImageURL(machineConfigs map[string]*v1alpha1.TinkerbellMachineConfig, dataCenterOSImageURLfound bool) error {
+	for _, mc := range machineConfigs {
+		if mc.Spec.OSImageURL != "" && dataCenterOSImageURLfound {
+			return fmt.Errorf("overlapping OSImageURL found, OSImageURL can either be set at datacenter config or for each machine config not both")
+		}
+		if mc.Spec.OSImageURL == "" && !dataCenterOSImageURLfound && mc.Spec.OSFamily != v1alpha1.Bottlerocket {
+			return fmt.Errorf("OSImageURL should be set for each machine config not found for: %s", mc.ObjectMeta.Name)
+		}
+	}
+
+	return nil
+}
+
 func validateMachineRefExists(
 	ref *v1alpha1.Ref,
 	machineConfigs map[string]*v1alpha1.TinkerbellMachineConfig,