diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 2be79eb2e62..c0a146f19db 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/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 { diff --git a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go index d9df288dba8..9e0c9cf58db 100644 --- a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go +++ b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go @@ -51,19 +51,19 @@ func (mr *MockAPIMockRecorder) GetNetwork(ctx, network, project interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNetwork", reflect.TypeOf((*MockAPI)(nil).GetNetwork), ctx, network, project) } -// GetMachineTypes mocks base method. -func (m *MockAPI) GetMachineTypes(ctx context.Context, project, filter string) (map[string]*compute.MachineType, error) { +// GetMachineType mocks base method. +func (m *MockAPI) GetMachineType(ctx context.Context, project, zone, machineType string) (*compute.MachineType, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetMachineTypes", ctx, project, filter) - ret0, _ := ret[0].(map[string]*compute.MachineType) + ret := m.ctrl.Call(m, "GetMachineType", ctx, project, zone, machineType) + ret0, _ := ret[0].(*compute.MachineType) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetMachineTypes indicates an expected call of GetMachineTypes. -func (mr *MockAPIMockRecorder) GetMachineTypes(ctx, project, filter interface{}) *gomock.Call { +// GetMachineType indicates an expected call of GetMachineType. +func (mr *MockAPIMockRecorder) GetMachineType(ctx, project, zone, machineType interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMachineTypes", reflect.TypeOf((*MockAPI)(nil).GetMachineTypes), ctx, project, filter) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMachineType", reflect.TypeOf((*MockAPI)(nil).GetMachineType), ctx, project, zone, machineType) } // GetPublicDomains mocks base method. @@ -141,6 +141,21 @@ func (mr *MockAPIMockRecorder) GetRecordSets(ctx, project, zone interface{}) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRecordSets", reflect.TypeOf((*MockAPI)(nil).GetRecordSets), ctx, project, zone) } +// GetZones mocks base method. +func (m *MockAPI) GetZones(ctx context.Context, project, filter string) ([]*compute.Zone, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetZones", ctx, project, filter) + ret0, _ := ret[0].([]*compute.Zone) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetZones indicates an expected call of GetZones. +func (mr *MockAPIMockRecorder) GetZones(ctx, project, filter interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetZones", reflect.TypeOf((*MockAPI)(nil).GetZones), ctx, project, filter) +} + // GetEnabledServices mocks base method. func (m *MockAPI) GetEnabledServices(ctx context.Context, project string) ([]string, error) { m.ctrl.T.Helper() diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index e184e0078dd..080b25747cb 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/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,6 +71,12 @@ 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 @@ -85,17 +84,20 @@ func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList // 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)...) } } diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 741d40628a9..e140ac71f56 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/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()