diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index f3734811b6b..91355090d0b 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -73,20 +73,29 @@ func Validate(client API, ic *types.InstallConfig) error { return allErrs.ToAggregate() } -func validateInstanceType(fldPath *field.Path, instanceType string, validInstanceTypes []string, arch string) *field.Error { - if len(validInstanceTypes) == 0 { - return field.InternalError(fldPath, fmt.Errorf("no valid instance types found")) - } - +func validateInstanceAndDiskType(fldPath *field.Path, diskType, instanceType, arch string) *field.Error { + diskTypes := []string{} + ok := false if instanceType != "" { family, _, _ := strings.Cut(instanceType, "-") - acceptedInstanceTypes := sets.New(validInstanceTypes...) - if !acceptedInstanceTypes.Has(family) { - return field.NotSupported(fldPath.Child("type"), family, sets.List(acceptedInstanceTypes)) + diskTypes, ok = gcp.DiskTypeToInstanceTypeMap[family] + if !ok { + return field.NotFound(fldPath.Child("type"), family) + } + + acceptedArmFamilies := sets.New("t2a") + if arch == types.ArchitectureARM64 && !acceptedArmFamilies.Has(family) { + return field.NotSupported(fldPath.Child("type"), family, sets.List(acceptedArmFamilies)) } + } - if arch == types.ArchitectureARM64 && family != "t2a" { - return field.InternalError(fldPath, fmt.Errorf("instance type %s requires %s architecture", instanceType, types.ArchitectureARM64)) + if len(diskTypes) > 0 && diskType != "" { + if !sets.New(diskTypes...).Has(diskType) { + return field.Invalid( + fldPath.Child("diskType"), + diskType, + fmt.Sprintf("%s instance requires one of the following disk types: %v", instanceType, diskTypes), + ) } } return nil @@ -104,16 +113,8 @@ func ValidateInstanceType(client API, fieldPath *field.Path, project, region str return append(allErrs, field.InternalError(nil, err)) } - instanceTypes := []string{} - ok := false - if diskType != "" { - instanceTypes, ok = gcp.DiskTypeToInstanceTypeMap[diskType] - if !ok { - return append(allErrs, field.NotFound(fieldPath.Child("diskType"), diskType)) - } - } - if err := validateInstanceType(fieldPath, instanceType, instanceTypes, arch); err != nil { - allErrs = append(allErrs, err) + if fieldErr := validateInstanceAndDiskType(fieldPath, diskType, instanceType, arch); fieldErr != nil { + return append(allErrs, fieldErr) } userZones := sets.New(zones...) @@ -226,18 +227,28 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList } } } - allErrs = append(allErrs, - ValidateInstanceType( - client, - field.NewPath("controlPlane", "platform", "gcp"), - ic.GCP.ProjectID, - ic.GCP.Region, - zones, - cpDiskType, - instanceType, - controlPlaneReq, - arch, - )...) + + // The IOPS minimum Control plane requirements are not met for pd-standard machines. + if cpDiskType == "pd-standard" { + allErrs = append(allErrs, + field.NotSupported(field.NewPath("controlPlane").Child("type"), + cpDiskType, + sets.List(gcp.ControlPlaneSupportedDisks)), + ) + } else { + allErrs = append(allErrs, + ValidateInstanceType( + client, + field.NewPath("controlPlane", "platform", "gcp"), + ic.GCP.ProjectID, + ic.GCP.Region, + zones, + cpDiskType, + instanceType, + controlPlaneReq, + arch, + )...) + } for idx, compute := range ic.Compute { fieldPath := field.NewPath("compute").Index(idx) diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 4cccaecc404..da2ae913294 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -93,6 +93,10 @@ var ( ic.Platform.GCP.DefaultMachinePlatform.InstanceType = "n1-dne-1" } + invalidateControlPlaneDiskTypes = func(ic *types.InstallConfig) { + ic.ControlPlane.Platform.GCP.DiskType = "pd-standard" + } + invalidateNetwork = func(ic *types.InstallConfig) { ic.GCP.Network = "invalid-vpc" } invalidateComputeSubnet = func(ic *types.InstallConfig) { ic.GCP.ComputeSubnet = "invalid-compute-subnet" } invalidateCPSubnet = func(ic *types.InstallConfig) { ic.GCP.ControlPlaneSubnet = "invalid-cp-subnet" } @@ -234,6 +238,12 @@ func TestGCPInstallConfigValidation(t *testing.T) { expectedError: true, expectedErrMsg: `\[platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 4 vCPUs, platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 15360 MB Memory, controlPlane.platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 4 vCPUs, controlPlane.platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 15360 MB Memory, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 2 vCPUs, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 7680 MB Memory\]`, }, + { + name: "Invalid control plane machine disk types", + edits: editFunctions{validMachineTypes, invalidateControlPlaneDiskTypes}, + expectedError: true, + expectedErrMsg: `controlPlane.type: Unsupported value: "pd-standard": supported values: "hyperdisk-balanced", "pd-balanced", "pd-ssd"`, + }, { name: "Invalid control plane machine types", edits: editFunctions{invalidateControlPlaneMachineTypes}, @@ -851,7 +861,7 @@ func TestValidateInstanceType(t *testing.T) { instanceType: "n2-standard-4", diskType: "hyperdisk-balanced", expectedError: true, - expectedErrMsg: `[instance.diskType: Unsupported value: "n2": supported values: "c3", "c3d", "c4", m1", "n4"]`, + expectedErrMsg: `^\[instance.diskType: Invalid value: "hyperdisk\-balanced": n2\-standard\-4 instance requires one of the following disk types: \[pd\-standard pd\-ssd pd\-balanced\]\]$`, }, } diff --git a/pkg/types/gcp/machinepools.go b/pkg/types/gcp/machinepools.go index f9390226cb8..e5538e4b2ff 100644 --- a/pkg/types/gcp/machinepools.go +++ b/pkg/types/gcp/machinepools.go @@ -27,13 +27,23 @@ var ( // ComputeSupportedDisks contains the supported disk types for control plane nodes. ComputeSupportedDisks = sets.New(HyperDiskBalanced, PDBalanced, PDSSD, PDStandard) - // DiskTypeToInstanceTypeMap contains a map where the key is the Disk Type, and the values are a list of - // instance types that are supported by the installer and correlate to the Disk Type. + // DiskTypeToInstanceTypeMap contains a map where the key is the Instance Type, and the + // values are a list of disk types that are supported by the installer and correlate to the Instance Type. DiskTypeToInstanceTypeMap = map[string][]string{ - PDStandard: {"a2", "e2", "n1", "n2", "n2d", "t2a", "t2d"}, - PDSSD: {"a2", "a3", "c3", "c3d", "e2", "m1", "n1", "n2", "n2d", "t2a", "t2d"}, - PDBalanced: {"a2", "a3", "c3", "c3d", "e2", "m1", "n1", "n2", "n2d", "t2a", "t2d"}, - HyperDiskBalanced: {"c3", "c3d", "m1", "n4"}, + "a2": {PDStandard, PDSSD, PDBalanced}, + "a3": {PDSSD, PDBalanced}, + "c2": {PDStandard, PDSSD, PDBalanced}, + "c2d": {PDStandard, PDSSD, PDBalanced}, + "c3": {PDSSD, PDBalanced, HyperDiskBalanced}, + "c3d": {PDSSD, PDBalanced, HyperDiskBalanced}, + "e2": {PDStandard, PDSSD, PDBalanced}, + "m1": {PDSSD, PDBalanced, HyperDiskBalanced}, + "n1": {PDStandard, PDSSD, PDBalanced}, + "n2": {PDStandard, PDSSD, PDBalanced}, + "n2d": {PDStandard, PDSSD, PDBalanced}, + "n4": {HyperDiskBalanced}, + "t2a": {PDStandard, PDSSD, PDBalanced}, + "t2d": {PDStandard, PDSSD, PDBalanced}, } )