Skip to content

Commit

Permalink
OCPBUGS-41184: Add validation for gcp disk and instance types
Browse files Browse the repository at this point in the history
** Validate the disk and instance type combinations for gcp installs.
  • Loading branch information
barbacbd committed Sep 11, 2024
1 parent 0f97bdc commit ed58de1
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 12 deletions.
50 changes: 41 additions & 9 deletions pkg/asset/installconfig/gcp/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,25 @@ 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"))
}

if instanceType != "" {
family, _, _ := strings.Cut(instanceType, "-")
acceptedInstanceTypes := sets.New(validInstanceTypes...)
if !acceptedInstanceTypes.Has(family) {
return field.NotSupported(fldPath.Child("type"), family, sets.List(acceptedInstanceTypes))
}

if arch == types.ArchitectureARM64 && family != "t2a" {
return field.InternalError(fldPath, fmt.Errorf("instance type %s requires %s architecture", instanceType, types.ArchitectureARM64))
}
}
return nil
}

// ValidateInstanceType ensures the instance type has sufficient Vcpu and Memory.
func ValidateInstanceType(client API, fieldPath *field.Path, project, region string, zones []string, diskType string, instanceType string, req resourceRequirements, arch string) field.ErrorList {
allErrs := field.ErrorList{}
Expand All @@ -85,13 +104,17 @@ func ValidateInstanceType(client API, fieldPath *field.Path, project, region str
return append(allErrs, field.InternalError(nil, err))
}

if diskType == "hyperdisk-balanced" {
family, _, _ := strings.Cut(instanceType, "-")
families := sets.NewString("c3", "c3d", "m1", "n4")
if !families.Has(family) {
allErrs = append(allErrs, field.NotSupported(fieldPath.Child("diskType"), family, families.List()))
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)
}

userZones := sets.New(zones...)
if len(userZones) == 0 {
Expand Down Expand Up @@ -149,6 +172,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
allErrs := field.ErrorList{}

defaultInstanceType := ""
defaultDiskType := gcp.PDSSD
defaultZones := []string{}

// Default requirements need to be sufficient to support Control Plane instances.
Expand All @@ -161,6 +185,9 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
if ic.GCP.DefaultMachinePlatform != nil {
defaultZones = ic.GCP.DefaultMachinePlatform.Zones
defaultInstanceType = ic.GCP.DefaultMachinePlatform.InstanceType
if ic.GCP.DefaultMachinePlatform.DiskType != "" {
defaultDiskType = ic.GCP.DefaultMachinePlatform.DiskType
}

if ic.GCP.DefaultMachinePlatform.InstanceType != "" {
allErrs = append(allErrs,
Expand All @@ -170,7 +197,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
ic.GCP.ProjectID,
ic.GCP.Region,
ic.GCP.DefaultMachinePlatform.Zones,
ic.GCP.DefaultMachinePlatform.DiskType,
defaultDiskType,
ic.GCP.DefaultMachinePlatform.InstanceType,
defaultInstanceReq,
unknownArchitecture,
Expand All @@ -181,7 +208,7 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
zones := defaultZones
instanceType := defaultInstanceType
arch := types.ArchitectureAMD64
cpDiskType := ""
cpDiskType := defaultDiskType
if ic.ControlPlane != nil {
arch = string(ic.ControlPlane.Architecture)
if instanceType == "" {
Expand All @@ -194,7 +221,9 @@ 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
if ic.ControlPlane.Platform.GCP.DiskType != "" {
cpDiskType = ic.ControlPlane.Platform.GCP.DiskType
}
}
}
allErrs = append(allErrs,
Expand All @@ -214,9 +243,13 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
fieldPath := field.NewPath("compute").Index(idx)
zones := defaultZones
instanceType := defaultInstanceType
diskType := defaultDiskType
if instanceType == "" {
instanceType = DefaultInstanceTypeForArch(compute.Architecture)
}
if diskType == "" {
diskType = gcp.PDSSD
}
arch := compute.Architecture
if compute.Platform.GCP != nil {
if compute.Platform.GCP.InstanceType != "" {
Expand All @@ -227,7 +260,6 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList
}
}

diskType := ""
if compute.Platform.GCP != nil && compute.Platform.GCP.DiskType != "" {
diskType = compute.Platform.GCP.DiskType
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/installconfig/gcp/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,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\", \"m1\", \"n4\"\]$`,
expectedErrMsg: `[instance.diskType: Unsupported value: "n2": supported values: "c3", "c3d", "c4", m1", "n4"]`,
},
}

Expand Down
24 changes: 22 additions & 2 deletions pkg/types/gcp/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,32 @@ type FeatureSwitch string
// applicable when ConfidentialCompute is Enabled.
type OnHostMaintenanceType string

const (
// PDSSD is the constant string representation for persistent disk ssd disk types.
PDSSD = "pd-ssd"
// PDStandard is the constant string representation for persistent disk standard disk types.
PDStandard = "pd-standard"
// PDBalanced is the constant string representation for persistent disk balanced disk types.
PDBalanced = "pd-balanced"
// HyperDiskBalanced is the constant string representation for hyperdisk balanced disk types.
HyperDiskBalanced = "hyperdisk-balanced"
)

var (
// ControlPlaneSupportedDisks contains the supported disk types for control plane nodes.
ControlPlaneSupportedDisks = sets.New("hyperdisk-balanced", "pd-balanced", "pd-ssd")
ControlPlaneSupportedDisks = sets.New(HyperDiskBalanced, PDBalanced, PDSSD)

// ComputeSupportedDisks contains the supported disk types for control plane nodes.
ComputeSupportedDisks = sets.New("hyperdisk-balanced", "pd-balanced", "pd-ssd", "pd-standard")
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 = 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"},
}
)

const (
Expand Down

0 comments on commit ed58de1

Please sign in to comment.