Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tinkerbell modular upgrades #6733

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions pkg/api/v1alpha1/cluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)...)
Expand Down
52 changes: 26 additions & 26 deletions pkg/api/v1alpha1/cluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
// }
7 changes: 7 additions & 0 deletions pkg/api/v1alpha1/tinkerbellmachineconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"fmt"
"net/url"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -91,6 +92,12 @@
)
}

if config.Spec.OSImageURL != "" {
if _, err := url.ParseRequestURI(config.Spec.OSImageURL); err != nil {
return fmt.Errorf("parsing osImageOverride: %v", err)
}

Check warning on line 98 in pkg/api/v1alpha1/tinkerbellmachineconfig.go

View check run for this annotation

Codecov / codecov/patch

pkg/api/v1alpha1/tinkerbellmachineconfig.go#L96-L98

Added lines #L96 - L98 were not covered by tests
}

if len(config.Spec.Users) == 0 {
return fmt.Errorf("TinkerbellMachineConfig: missing spec.Users: %s", config.Name)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/api/v1alpha1/tinkerbellmachineconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
75 changes: 75 additions & 0 deletions pkg/executables/govc.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"net/http"
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -39,6 +40,7 @@
DeployOptsFile = "deploy-opts.json"
disk1 = "Hard disk 1"
disk2 = "Hard disk 2"
MemoryAvailable = "Memory_Available"
)

var requiredEnvs = []string{govcUsernameKey, govcPasswordKey, govcURLKey, govcInsecure, govcDatacenterKey}
Expand Down Expand Up @@ -1143,3 +1145,76 @@

return nil
}

type resourcePoolInfo struct {
ResourcePoolIdentifier *resourcePool
}

type resourcePool struct {
memoryUsage string
memoryLimit string
}

// GetResourcePoolInfo returns the pool info for the provided resource pool.
func (g *Govc) GetResourcePoolInfo(ctx context.Context, datacenter, resourcepool string, args ...string) (map[string]int, error) {
params := []string{"pool.info", "-dc", datacenter, resourcepool}
params = append(params, args...)
response, err := g.exec(ctx, params...)
if err != nil {
return nil, fmt.Errorf("getting resource pool information: %v", err)
}

scanner := bufio.NewScanner(strings.NewReader(response.String()))
var resourcePoolInfoResponse resourcePoolInfo
resourcePoolInfoResponse.ResourcePoolIdentifier = new(resourcePool)
for scanner.Scan() {
metaData := scanner.Text()
if strings.Contains(metaData, "Mem Usage") {
resourcePoolInfoResponse.ResourcePoolIdentifier.memoryUsage = strings.Split(metaData, ":")[1]
}
if strings.Contains(metaData, "Mem Limit") {
resourcePoolInfoResponse.ResourcePoolIdentifier.memoryLimit = strings.Split(metaData, ":")[1]
}
}

if err := scanner.Err(); err != nil {
return nil, fmt.Errorf("failure reading memory allocation for resource pool")
}

Check warning on line 1182 in pkg/executables/govc.go

View check run for this annotation

Codecov / codecov/patch

pkg/executables/govc.go#L1181-L1182

Added lines #L1181 - L1182 were not covered by tests

poolInfo, err := getPoolInfo(resourcePoolInfoResponse.ResourcePoolIdentifier)
if err != nil {
return nil, err
}
return poolInfo, nil
}

// getPoolInfo parses resource pool response and returns memory requirements.
func getPoolInfo(rp *resourcePool) (map[string]int, error) {
memoryUsed, err := getValueFromString(rp.memoryUsage)
if err != nil {
return nil, fmt.Errorf("unable to obtain memory usage for resource pool %s: %v", rp.memoryUsage, err)
}
memoryLimit, err := getValueFromString(rp.memoryLimit)
if err != nil {
return nil, fmt.Errorf("unable to obtain memory limit for resource pool %s: %v", rp.memoryLimit, err)
}
poolInfo := make(map[string]int)
if memoryLimit != -1 {
poolInfo[MemoryAvailable] = memoryLimit - memoryUsed
} else {
poolInfo[MemoryAvailable] = memoryLimit
}
return poolInfo, nil
}

// getValueFromString cleans the input string and returns the extracted numerical value.
func getValueFromString(str string) (int, error) {
splitResponse := strings.Split(strings.TrimSpace(str), " ")
nonNumericRegex := regexp.MustCompile(`[^0-9- ]+`)
cleanedString := nonNumericRegex.ReplaceAllString(splitResponse[0], "")
numValue, err := strconv.Atoi(cleanedString)
if err != nil {
return 0, err
}
return numValue, nil
}
79 changes: 79 additions & 0 deletions pkg/executables/govc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1643,3 +1643,82 @@ func TestGovcGetHardDiskSizeError(t *testing.T) {
})
}
}

func TestGovcGetResourcePoolInfo(t *testing.T) {
datacenter := "SDDC-Datacenter"
resourcePool := "*/Resources/Test-ResourcePool"
govcErr := errors.New("error PoolInfo()")
ctx := context.Background()
_, g, executable, env := setup(t)

tests := []struct {
testName string
response string
govcErr error
wantErr error
wantMemInfo map[string]int
}{
{
testName: "pool_info_memory_limit_set",
response: `Name: Test-ResourcePool
Path: /SDDC-Datacenter/host/Cluster-1/Resources/Test-ResourcePool
Mem Usage: 100MB (11.3%)
Mem Shares: normal
Mem Reservation: 0MB (expandable=true)
Mem Limit: 1000MB`,
govcErr: nil,
wantErr: nil,
wantMemInfo: map[string]int{executables.MemoryAvailable: 900},
},
{
testName: "pool_info_memory_limit_unset",
response: `Name: Test-ResourcePool
Path: /SDDC-Datacenter/host/Cluster-1/Resources/Test-ResourcePool
Mem Usage: 100MB (11.3%)
Mem Shares: normal
Mem Reservation: 0MB (expandable=true)
Mem Limit: -1MB`,
govcErr: nil,
wantErr: nil,
wantMemInfo: map[string]int{executables.MemoryAvailable: -1},
},
{
testName: "pool_info_memory_usage_corrupt",
response: `Name: Test-ResourcePool
Mem Usage:corrupt-val
Mem Limit:-1MB`,
govcErr: nil,
wantErr: fmt.Errorf("unable to obtain memory usage for resource pool corrupt-val: strconv.Atoi: parsing \"-\": invalid syntax"),
wantMemInfo: nil,
},
{
testName: "pool_info_memory_limit_corrupt",
response: `Name: Test-ResourcePool
Mem Usage:100
Mem Limit:corrupt-val`,
govcErr: nil,
wantErr: fmt.Errorf("unable to obtain memory limit for resource pool corrupt-val: strconv.Atoi: parsing \"-\": invalid syntax"),
wantMemInfo: nil,
},
{
testName: "pool_info_error",
response: "",
govcErr: govcErr,
wantErr: fmt.Errorf("getting resource pool information: %v", govcErr),
wantMemInfo: nil,
},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
gt := NewWithT(t)
responseBytes := bytes.NewBuffer([]byte(tt.response))
executable.EXPECT().ExecuteWithEnv(ctx, env, "pool.info", "-dc", datacenter, resourcePool).Return(*responseBytes, tt.govcErr)
poolMemInfo, err := g.GetResourcePoolInfo(ctx, datacenter, resourcePool)
if tt.wantErr != nil {
gt.Expect(err.Error()).To(Equal(tt.wantErr.Error()))
}
gt.Expect(poolMemInfo).To(Equal(tt.wantMemInfo))
})
}
}
5 changes: 5 additions & 0 deletions pkg/providers/tinkerbell/assert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/providers/tinkerbell/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func NewClusterSpecValidator(assertions ...ClusterSpecAssertion) *ClusterSpecVal
AssertMachineConfigsValid,
AssertMachineConfigNamespaceMatchesDatacenterConfig,
AssertOsFamilyValid,
AssertOSImageURLDontOverlap,
AssertTinkerbellIPAndControlPlaneIPNotSame,
AssertHookRetrievableWithoutProxy,
)
Expand Down
24 changes: 21 additions & 3 deletions pkg/providers/tinkerbell/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,16 @@
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()
Expand All @@ -81,12 +88,16 @@

var etcdMachineSpec v1alpha1.TinkerbellMachineConfigSpec
var etcdTemplateString string

if clusterSpec.Cluster.Spec.ExternalEtcdConfiguration != nil {
etcdMachineSpec = *tb.etcdMachineSpec
if etcdMachineSpec.OSImageURL != "" {
OSImageURL = etcdMachineSpec.OSImageURL
}

Check warning on line 96 in pkg/providers/tinkerbell/template.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/tinkerbell/template.go#L94-L96

Added lines #L94 - L96 were not covered by tests
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)

Check warning on line 100 in pkg/providers/tinkerbell/template.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/tinkerbell/template.go#L100

Added line #L100 was not covered by tests
}
etcdTemplateString, err = etcdTemplateConfig.ToTemplateString()
if err != nil {
Expand All @@ -111,12 +122,19 @@
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
}

Check warning on line 136 in pkg/providers/tinkerbell/template.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/tinkerbell/template.go#L135-L136

Added lines #L135 - L136 were not covered by tests
wTemplateConfig = v1alpha1.NewDefaultTinkerbellTemplateConfigCreate(clusterSpec.Cluster, *versionBundle, OSImageURL, tb.tinkerbellIP, tb.datacenterSpec.TinkerbellIP, workerNodeMachineSpec.OSFamily)
}

wTemplateString, err := wTemplateConfig.ToTemplateString()
Expand Down
23 changes: 22 additions & 1 deletion pkg/providers/tinkerbell/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,33 @@
}
}

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")
}

Check warning on line 50 in pkg/providers/tinkerbell/validate.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/tinkerbell/validate.go#L49-L50

Added lines #L49 - L50 were not covered by tests
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)
}

Check warning on line 53 in pkg/providers/tinkerbell/validate.go

View check run for this annotation

Codecov / codecov/patch

pkg/providers/tinkerbell/validate.go#L52-L53

Added lines #L52 - L53 were not covered by tests
}

return nil
}

func validateMachineRefExists(
ref *v1alpha1.Ref,
machineConfigs map[string]*v1alpha1.TinkerbellMachineConfig,
Expand Down
Loading