Skip to content

Commit

Permalink
installconfig/gcp/validation: handle custom machine types
Browse files Browse the repository at this point in the history
Prior to this change, the validation on gcp instance types would return
an error that the type did not exist. This change adds the logic to
properly handle custom machine types while validating their CPU and
Memeory values.
  • Loading branch information
jstuever committed Nov 20, 2020
1 parent 8d9d7cb commit 60a5705
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 51 deletions.
56 changes: 37 additions & 19 deletions pkg/asset/installconfig/gcp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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()

Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 22 additions & 7 deletions pkg/asset/installconfig/gcp/mock/gcpclient_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 24 additions & 22 deletions pkg/asset/installconfig/gcp/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand All @@ -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)...)
}
}

Expand Down
15 changes: 12 additions & 3 deletions pkg/asset/installconfig/gcp/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 60a5705

Please sign in to comment.