From fb79ff05831ee33186c825ad6ab6524ed13bedb5 Mon Sep 17 00:00:00 2001 From: Rahul Ganesh Date: Tue, 26 Sep 2023 16:02:15 -0700 Subject: [PATCH] Facilitate modular upgrades for tinkerbell provider Signed-off-by: Rahul Ganesh --- pkg/api/v1alpha1/cluster_webhook.go | 7 --- pkg/api/v1alpha1/cluster_webhook_test.go | 52 +++++++++---------- pkg/api/v1alpha1/tinkerbellmachineconfig.go | 7 +++ .../v1alpha1/tinkerbellmachineconfig_types.go | 1 + pkg/providers/tinkerbell/assert.go | 5 ++ pkg/providers/tinkerbell/cluster.go | 1 + pkg/providers/tinkerbell/template.go | 24 +++++++-- pkg/providers/tinkerbell/validate.go | 23 +++++++- 8 files changed, 83 insertions(+), 37 deletions(-) diff --git a/pkg/api/v1alpha1/cluster_webhook.go b/pkg/api/v1alpha1/cluster_webhook.go index 703fbcd0ef44d..0806932234349 100644 --- 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 index feefe38afae19..73efd21cefd80 100644 --- 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 index 20cd5a56e4e3a..1750e721d7bd3 100644 --- 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 index 059452823297e..e81649705938d 100644 --- 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 index 99018e6c72b95..f5d4aef3f256d 100644 --- 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 index fcc61f5bc36ff..f14caf28f3205 100644 --- 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 index e3ca2a529cbf5..a345f021ef704 100644 --- 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 index 2ddcccc97d810..1a1a22c1ab125 100644 --- 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,