OCPBUGS-41184: GCP Validate Disk and Instance Type

** Update the error text and the way that disks and instances are validated. Before, the
error message was backwards on indicating if the instance type or the disk type was the problem.
Now, the disk type is validated against the instance type (rather than the opposite).

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)
 		}
 
-		if arch == types.ArchitectureARM64 && family != "t2a" {
-			return field.InternalError(fldPath, fmt.Errorf("instance type %s requires %s architecture", instanceType, types.ArchitectureARM64))
+		acceptedArmFamilies := sets.New("t2a")
+		if arch == types.ArchitectureARM64 && !acceptedArmFamilies.Has(family) {
+			return field.NotSupported(fldPath.Child("type"), family, sets.List(acceptedArmFamilies))
+		}
+	}
+
+	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...)

pkg/asset/installconfig/gcp/validation_test.go

@@ -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", "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\]\]$`,
 		},
 	}
 

pkg/types/gcp/machinepools.go

@@ -27,13 +27,21 @@ 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},
+		"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},
 	}
 )