From 58e87f6b6a3066aee663a02b8a107fd3679f3775 Mon Sep 17 00:00:00 2001 From: Brent Barbachem Date: Tue, 20 Aug 2024 10:15:03 -0400 Subject: [PATCH] CORS-3649: Add new disk types GCP Control Plane nodes ** GCP Control Plane Nodes should now be allowed to use pd-balanced and hyperdisk-balanced. The IOPS rating is sufficient for both disk types to be used for control plane nodes, but pd-standard still falls short. ** Update installer explain to provide the correct information --- .../install.openshift.io_installconfigs.yaml | 9 ++++-- pkg/asset/installconfig/gcp/validation.go | 4 ++- pkg/types/gcp/machinepools.go | 12 +++++++- pkg/types/gcp/validation/machinepool.go | 18 ++++-------- pkg/types/validation/machinepools_test.go | 28 +++++++++++++++++++ 5 files changed, 54 insertions(+), 17 deletions(-) diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index fca95d2d29..935942e51c 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -507,7 +507,8 @@ spec: type: integer diskType: description: DiskType defines the type of disk. For - control plane nodes, the valid value is pd-ssd. + control plane nodes, the valid values are pd-balanced, + pd-ssd, and hyperdisk-balanced. enum: - pd-balanced - pd-ssd @@ -1416,7 +1417,8 @@ spec: type: integer diskType: description: DiskType defines the type of disk. For control - plane nodes, the valid value is pd-ssd. + plane nodes, the valid values are pd-balanced, pd-ssd, + and hyperdisk-balanced. enum: - pd-balanced - pd-ssd @@ -3049,7 +3051,8 @@ spec: type: integer diskType: description: DiskType defines the type of disk. For control - plane nodes, the valid value is pd-ssd. + plane nodes, the valid values are pd-balanced, pd-ssd, + and hyperdisk-balanced. enum: - pd-balanced - pd-ssd diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 0b453c32aa..443c4d15af 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -181,6 +181,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList zones := defaultZones instanceType := defaultInstanceType arch := types.ArchitectureAMD64 + cpDiskType := "" if ic.ControlPlane != nil { arch = string(ic.ControlPlane.Architecture) if instanceType == "" { @@ -193,6 +194,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList if len(ic.ControlPlane.Platform.GCP.Zones) > 0 { zones = ic.ControlPlane.Platform.GCP.Zones } + cpDiskType = ic.ControlPlane.Platform.GCP.DiskType } } allErrs = append(allErrs, @@ -202,7 +204,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList ic.GCP.ProjectID, ic.GCP.Region, zones, - "", // the control plane nodes only support one disk type currently + cpDiskType, instanceType, controlPlaneReq, arch, diff --git a/pkg/types/gcp/machinepools.go b/pkg/types/gcp/machinepools.go index 61ad0e39cf..e4f5fd5c28 100644 --- a/pkg/types/gcp/machinepools.go +++ b/pkg/types/gcp/machinepools.go @@ -1,5 +1,7 @@ package gcp +import "k8s.io/apimachinery/pkg/util/sets" + // FeatureSwitch indicates whether the feature is enabled or disabled. type FeatureSwitch string @@ -7,6 +9,14 @@ type FeatureSwitch string // applicable when ConfidentialCompute is Enabled. type OnHostMaintenanceType string +var ( + // ControlPlaneSupportedDisks contains the supported disk types for control plane nodes. + ControlPlaneSupportedDisks = sets.New("hyperdisk-balanced", "pd-balanced", "pd-ssd") + + // ComputeSupportedDisks contains the supported disk types for control plane nodes. + ComputeSupportedDisks = sets.New("hyperdisk-balanced", "pd-balanced", "pd-ssd", "pd-standard") +) + const ( // EnabledFeature indicates that the feature is configured as enabled. EnabledFeature FeatureSwitch = "Enabled" @@ -86,7 +96,7 @@ type MachinePool struct { // OSDisk defines the disk for machines on GCP. type OSDisk struct { // DiskType defines the type of disk. - // For control plane nodes, the valid value is pd-ssd. + // For control plane nodes, the valid values are pd-balanced, pd-ssd, and hyperdisk-balanced. // +optional // +kubebuilder:validation:Enum=pd-balanced;pd-ssd;pd-standard;hyperdisk-balanced DiskType string `json:"diskType"` diff --git a/pkg/types/gcp/validation/machinepool.go b/pkg/types/gcp/validation/machinepool.go index be3f181bf2..7f91cef93d 100644 --- a/pkg/types/gcp/validation/machinepool.go +++ b/pkg/types/gcp/validation/machinepool.go @@ -29,11 +29,8 @@ func ValidateMachinePool(platform *gcp.Platform, p *gcp.MachinePool, fldPath *fi } } - if p.OSDisk.DiskType != "" { - diskTypes := sets.NewString("pd-balanced", "pd-ssd", "pd-standard", "hyperdisk-balanced") - if !diskTypes.Has(p.OSDisk.DiskType) { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.OSDisk.DiskType, diskTypes.List())) - } + if diskType := p.OSDisk.DiskType; diskType != "" && !gcp.ComputeSupportedDisks.Has(diskType) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), diskType, sets.List(gcp.ComputeSupportedDisks))) } if p.ConfidentialCompute == string(gcp.EnabledFeature) && p.OnHostMaintenance != string(gcp.OnHostMaintenanceTerminate) { @@ -74,9 +71,8 @@ func ValidateServiceAccount(platform *gcp.Platform, p *types.MachinePool, fldPat func ValidateMasterDiskType(p *types.MachinePool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if p.Name == "master" && p.Platform.GCP.OSDisk.DiskType != "" { - diskTypes := sets.NewString("pd-ssd") - if !diskTypes.Has(p.Platform.GCP.OSDisk.DiskType) { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.Platform.GCP.OSDisk.DiskType, diskTypes.List())) + if !gcp.ControlPlaneSupportedDisks.Has(p.Platform.GCP.OSDisk.DiskType) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.Platform.GCP.OSDisk.DiskType, sets.List(gcp.ControlPlaneSupportedDisks))) } } return allErrs @@ -87,10 +83,8 @@ func ValidateDefaultDiskType(p *gcp.MachinePool, fldPath *field.Path) field.Erro allErrs := field.ErrorList{} if p != nil && p.OSDisk.DiskType != "" { - diskTypes := sets.NewString("pd-ssd") - - if !diskTypes.Has(p.OSDisk.DiskType) { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.OSDisk.DiskType, diskTypes.List())) + if !gcp.ControlPlaneSupportedDisks.Has(p.OSDisk.DiskType) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("diskType"), p.OSDisk.DiskType, sets.List(gcp.ControlPlaneSupportedDisks))) } } diff --git a/pkg/types/validation/machinepools_test.go b/pkg/types/validation/machinepools_test.go index f8a05c3aab..8acbbabe27 100644 --- a/pkg/types/validation/machinepools_test.go +++ b/pkg/types/validation/machinepools_test.go @@ -178,6 +178,34 @@ func TestValidateMachinePool(t *testing.T) { }(), valid: false, }, + { + name: "valid GCP disk type master", + platform: &types.Platform{GCP: &gcp.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool("master") + p.Platform = types.MachinePoolPlatform{ + GCP: &gcp.MachinePool{}, + } + p.Platform.GCP.OSDisk.DiskSizeGB = 100 + p.Platform.GCP.OSDisk.DiskType = "hyperdisk-balanced" + return p + }(), + valid: true, + }, + { + name: "invalid GCP disk type master", + platform: &types.Platform{GCP: &gcp.Platform{Region: "us-east-1"}}, + pool: func() *types.MachinePool { + p := validMachinePool("master") + p.Platform = types.MachinePoolPlatform{ + GCP: &gcp.MachinePool{}, + } + p.Platform.GCP.OSDisk.DiskSizeGB = 100 + p.Platform.GCP.OSDisk.DiskType = "pd-standard" + return p + }(), + valid: false, + }, { name: "valid GCP service account use", platform: &types.Platform{GCP: &gcp.Platform{Region: "us-east-1", NetworkProjectID: "ExampleNetworkProject"}},