Merge pull request #4386 from jstuever/bz1898194

Bug 1898194: installconfig/gcp/validation: handle custom machine types

pkg/asset/installconfig/gcp/client.go

@@ -19,12 +19,13 @@ import (
 // API represents the calls made to the API.
 type API interface {
 	GetNetwork(ctx context.Context, network, project string) (*compute.Network, error)
-	GetMachineTypes(ctx context.Context, project, filter string) (map[string]*compute.MachineType, error)
+	GetMachineType(ctx context.Context, project, zone, machineType string) (*compute.MachineType, error)
 	GetPublicDomains(ctx context.Context, project string) ([]string, error)
 	GetPublicDNSZone(ctx context.Context, project, baseDomain string) (*dns.ManagedZone, error)
 	GetSubnetworks(ctx context.Context, network, project, region string) ([]*compute.Subnetwork, error)
 	GetProjects(ctx context.Context) (map[string]string, error)
 	GetRecordSets(ctx context.Context, project, zone string) ([]*dns.ResourceRecordSet, error)
+	GetZones(ctx context.Context, project, filter string) ([]*compute.Zone, error)
 	GetEnabledServices(ctx context.Context, project string) ([]string, error)
 }
 
@@ -49,10 +50,8 @@ func NewClient(ctx context.Context) (*Client, error) {
 	return client, nil
 }
 
-// GetMachineTypes uses the GCP Compute Service API to get a list of machine types from a project.
-func (c *Client) GetMachineTypes(ctx context.Context, project, filter string) (map[string]*compute.MachineType, error) {
-	types := map[string]*compute.MachineType{}
-
+// GetMachineType uses the GCP Compute Service API to get the specified machine type.
+func (c *Client) GetMachineType(ctx context.Context, project, zone, machineType string) (*compute.MachineType, error) {
 	ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
 	defer cancel()
 
@@ -61,22 +60,12 @@ func (c *Client) GetMachineTypes(ctx context.Context, project, filter string) (m
 		return nil, err
 	}
 
-	req := svc.MachineTypes.AggregatedList(project)
-	if filter != "" {
-		req = req.Filter(filter)
+	req, err := svc.MachineTypes.Get(project, zone, machineType).Context(ctx).Do()
+	if err != nil {
+		return nil, err
 	}
 
-	if err := req.Pages(ctx, func(page *compute.MachineTypeAggregatedList) error {
-		for _, scopedList := range page.Items {
-			for _, item := range scopedList.MachineTypes {
-				types[item.Name] = item
-			}
-		}
-		return nil
-	}); err != nil {
-		return nil, errors.Wrapf(err, "failed to get machine types from project %s", project)
-	}
-	return types, nil
+	return req, nil
 }
 
 // GetNetwork uses the GCP Compute Service API to get a network by name from a project.
@@ -233,6 +222,35 @@ func (c *Client) GetProjects(ctx context.Context) (map[string]string, error) {
 	return projects, nil
 }
 
+// GetZones uses the GCP Compute Service API to get a list of zones from a project.
+func (c *Client) GetZones(ctx context.Context, project, filter string) ([]*compute.Zone, error) {
+	zones := []*compute.Zone{}
+
+	ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
+	defer cancel()
+
+	svc, err := c.getComputeService(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	req := svc.Zones.List(project)
+	if filter != "" {
+		req = req.Filter(filter)
+	}
+
+	if err := req.Pages(ctx, func(page *compute.ZoneList) error {
+		for _, zone := range page.Items {
+			zones = append(zones, zone)
+		}
+		return nil
+	}); err != nil {
+		return nil, errors.Wrapf(err, "failed to get zones from project %s", project)
+	}
+
+	return zones, nil
+}
+
 func (c *Client) getCloudResourceService(ctx context.Context) (*cloudresourcemanager.Service, error) {
 	svc, err := cloudresourcemanager.NewService(ctx, option.WithCredentials(c.ssn.Credentials))
 	if err != nil {

pkg/asset/installconfig/gcp/validation.go

@@ -44,30 +44,23 @@ func Validate(client API, ic *types.InstallConfig) error {
 }
 
 // ValidateInstanceType ensures the instance type has sufficient Vcpu and Memory.
-func ValidateInstanceType(client API, ic *types.InstallConfig, fieldPath *field.Path, instanceType string, req resourceRequirements) field.ErrorList {
+func ValidateInstanceType(client API, fieldPath *field.Path, project, zone, instanceType string, req resourceRequirements) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if instanceType == "" {
-		return nil
-	}
-
-	filter := fmt.Sprintf("name = %s", instanceType)
-	instanceTypes, err := client.GetMachineTypes(context.TODO(), ic.GCP.ProjectID, filter)
+	typeMeta, err := client.GetMachineType(context.TODO(), project, zone, instanceType)
 	if err != nil {
-		return append(allErrs, field.InternalError(fieldPath, err))
+		if _, ok := err.(*googleapi.Error); ok {
+			return append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, err.Error()))
+		}
+		return append(allErrs, field.InternalError(nil, err))
 	}
 
-	if typeMeta, ok := instanceTypes[instanceType]; ok {
-		if typeMeta.GuestCpus < req.minimumVCpus {
-			errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d vCPUs", req.minimumVCpus)
-			allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
-		}
-		if typeMeta.MemoryMb < req.minimumMemory {
-			errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d MB Memory", req.minimumMemory)
-			allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
-		}
-	} else {
-		errMsg := fmt.Sprintf("instance type %s not found", instanceType)
+	if typeMeta.GuestCpus < req.minimumVCpus {
+		errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d vCPUs", req.minimumVCpus)
+		allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
+	}
+	if typeMeta.MemoryMb < req.minimumMemory {
+		errMsg := fmt.Sprintf("instance type does not meet minimum resource requirements of %d MB Memory", req.minimumMemory)
 		allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
 	}
 
@@ -78,24 +71,33 @@ func ValidateInstanceType(client API, ic *types.InstallConfig, fieldPath *field.
 func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList {
 	allErrs := field.ErrorList{}
 
+	// Get list of zones in region
+	zones, err := client.GetZones(context.TODO(), ic.GCP.ProjectID, fmt.Sprintf("region eq .*%s", ic.GCP.Region))
+	if err != nil {
+		return append(allErrs, field.InternalError(nil, err))
+	}
+
 	// Default requirements need to be sufficient to support Control Plane instances.
 	defaultInstanceReq := controlPlaneReq
 
 	if ic.ControlPlane != nil && ic.ControlPlane.Platform.GCP != nil && ic.ControlPlane.Platform.GCP.InstanceType != "" {
 		// Default requirements can be relaxed when the controlPlane type is set explicitly.
 		defaultInstanceReq = computeReq
 
-		allErrs = append(allErrs, ValidateInstanceType(client, ic, field.NewPath("controlPlane", "platform", "gcp"), ic.ControlPlane.Platform.GCP.InstanceType, controlPlaneReq)...)
+		allErrs = append(allErrs, ValidateInstanceType(client, field.NewPath("controlPlane", "platform", "gcp"), ic.GCP.ProjectID, zones[0].Name,
+			ic.ControlPlane.Platform.GCP.InstanceType, controlPlaneReq)...)
 	}
 
 	if ic.Platform.GCP.DefaultMachinePlatform != nil && ic.Platform.GCP.DefaultMachinePlatform.InstanceType != "" {
-		allErrs = append(allErrs, ValidateInstanceType(client, ic, field.NewPath("platform", "gcp", "defaultMachinePlatform"), ic.Platform.GCP.DefaultMachinePlatform.InstanceType, defaultInstanceReq)...)
+		allErrs = append(allErrs, ValidateInstanceType(client, field.NewPath("platform", "gcp", "defaultMachinePlatform"), ic.GCP.ProjectID, zones[0].Name,
+			ic.Platform.GCP.DefaultMachinePlatform.InstanceType, defaultInstanceReq)...)
 	}
 
 	for idx, compute := range ic.Compute {
 		fieldPath := field.NewPath("compute").Index(idx)
 		if compute.Platform.GCP != nil && compute.Platform.GCP.InstanceType != "" {
-			allErrs = append(allErrs, ValidateInstanceType(client, ic, fieldPath.Child("platform", "gcp"), compute.Platform.GCP.InstanceType, computeReq)...)
+			allErrs = append(allErrs, ValidateInstanceType(client, fieldPath.Child("platform", "gcp"), ic.GCP.ProjectID, zones[0].Name,
+				compute.Platform.GCP.InstanceType, computeReq)...)
 		}
 	}
 

pkg/asset/installconfig/gcp/validation_test.go

@@ -23,6 +23,7 @@ var (
 	validNetworkName   = "valid-vpc"
 	validProjectName   = "valid-project"
 	validRegion        = "us-east1"
+	validZone          = "us-east1-b"
 	validComputeSubnet = "valid-compute-subnet"
 	validCPSubnet      = "valid-controlplane-subnet"
 	validCIDR          = "10.0.0.0/16"
@@ -189,7 +190,7 @@ func TestGCPInstallConfigValidation(t *testing.T) {
 			name:           "Undefined default machine types",
 			edits:          editFunctions{undefinedDefaultMachineTypes},
 			expectedError:  true,
-			expectedErrMsg: `platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-dne-1": instance type n1-dne-1 not found`,
+			expectedErrMsg: `Internal error: 404`,
 		},
 		{
 			name:           "Invalid region",
@@ -222,8 +223,16 @@ func TestGCPInstallConfigValidation(t *testing.T) {
 	gcpClient := mock.NewMockAPI(mockCtrl)
 	// Should get the list of projects.
 	gcpClient.EXPECT().GetProjects(gomock.Any()).Return(map[string]string{"valid-project": "valid-project"}, nil).AnyTimes()
-	// Should return the list of machine types in machineTypeAPIResult.
-	gcpClient.EXPECT().GetMachineTypes(gomock.Any(), gomock.Any(), gomock.Any()).Return(machineTypeAPIResult, nil).AnyTimes()
+	// Should get the list of zones.
+	gcpClient.EXPECT().GetZones(gomock.Any(), gomock.Any(), gomock.Any()).Return([]*compute.Zone{{Name: validZone}}, nil).AnyTimes()
+
+	// Should return the machine type as specified.
+	for key, value := range machineTypeAPIResult {
+		gcpClient.EXPECT().GetMachineType(gomock.Any(), gomock.Any(), gomock.Any(), key).Return(value, nil).AnyTimes()
+	}
+	// When passed incorrect machine type, the API returns nil.
+	gcpClient.EXPECT().GetMachineType(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("404")).AnyTimes()
+
 	// When passed the correct network & project, return an empty network, which should be enough to validate ok.
 	gcpClient.EXPECT().GetNetwork(gomock.Any(), validNetworkName, validProjectName).Return(&compute.Network{}, nil).AnyTimes()