Merge pull request #4329 from jstuever/cors1548

asset/installconfig/gcp: Validate install-config instance types

pkg/asset/installconfig/gcp/client.go

@@ -19,6 +19,7 @@ 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)
 	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)
@@ -48,6 +49,36 @@ 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{}
+
+	ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
+	defer cancel()
+
+	svc, err := c.getComputeService(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	req := svc.MachineTypes.AggregatedList(project)
+	if filter != "" {
+		req = req.Filter(filter)
+	}
+
+	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
+}
+
 // GetNetwork uses the GCP Compute Service API to get a network by name from a project.
 func (c *Client) GetNetwork(ctx context.Context, network, project string) (*compute.Network, error) {
 	ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)

pkg/asset/installconfig/gcp/validation.go

@@ -17,16 +17,91 @@ import (
 	"github.com/openshift/installer/pkg/types"
 )
 
+type resourceRequirements struct {
+	minimumVCpus  int64
+	minimumMemory int64
+}
+
+var controlPlaneReq = resourceRequirements{
+	minimumVCpus:  4,
+	minimumMemory: 15360,
+}
+
+var computeReq = resourceRequirements{
+	minimumVCpus:  2,
+	minimumMemory: 7680,
+}
+
 // Validate executes platform-specific validation.
 func Validate(client API, ic *types.InstallConfig) error {
 	allErrs := field.ErrorList{}
 
 	allErrs = append(allErrs, validateProject(client, ic, field.NewPath("platform").Child("gcp"))...)
 	allErrs = append(allErrs, validateNetworks(client, ic, field.NewPath("platform").Child("gcp"))...)
+	allErrs = append(allErrs, validateInstanceTypes(client, ic)...)
 
 	return allErrs.ToAggregate()
 }
 
+// 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 {
+	allErrs := field.ErrorList{}
+
+	if instanceType == "" {
+		return nil
+	}
+
+	filter := fmt.Sprintf("name = %s", instanceType)
+	instanceTypes, err := client.GetMachineTypes(context.TODO(), ic.GCP.ProjectID, filter)
+	if err != nil {
+		return append(allErrs, field.InternalError(fieldPath, 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)
+		allErrs = append(allErrs, field.Invalid(fieldPath.Child("type"), instanceType, errMsg))
+	}
+
+	return allErrs
+}
+
+// validateInstanceTypes checks that the user-provided instance types are valid.
+func validateInstanceTypes(client API, ic *types.InstallConfig) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	// 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)...)
+	}
+
+	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)...)
+	}
+
+	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)...)
+		}
+	}
+
+	return allErrs
+}
+
 // ValidatePreExitingPublicDNS ensure no pre-existing DNS record exists in the public
 // DNS zone for cluster's Kubernetes API.
 func ValidatePreExitingPublicDNS(client API, ic *types.InstallConfig) error {

pkg/asset/installconfig/gcp/validation_test.go

@@ -34,6 +34,28 @@ var (
 		}
 	}
 
+	validMachineTypes = func(ic *types.InstallConfig) {
+		ic.Platform.GCP.DefaultMachinePlatform.InstanceType = "n1-standard-2"
+		ic.ControlPlane.Platform.GCP.InstanceType = "n1-standard-4"
+		ic.Compute[0].Platform.GCP.InstanceType = "n1-standard-2"
+	}
+
+	invalidateDefaultMachineTypes = func(ic *types.InstallConfig) {
+		ic.Platform.GCP.DefaultMachinePlatform.InstanceType = "n1-standard-1"
+	}
+
+	invalidateControlPlaneMachineTypes = func(ic *types.InstallConfig) {
+		ic.ControlPlane.Platform.GCP.InstanceType = "n1-standard-1"
+	}
+
+	invalidateComputeMachineTypes = func(ic *types.InstallConfig) {
+		ic.Compute[0].Platform.GCP.InstanceType = "n1-standard-1"
+	}
+
+	undefinedDefaultMachineTypes = func(ic *types.InstallConfig) {
+		ic.Platform.GCP.DefaultMachinePlatform.InstanceType = "n1-dne-1"
+	}
+
 	invalidateNetwork       = func(ic *types.InstallConfig) { ic.GCP.Network = "invalid-vpc" }
 	invalidateComputeSubnet = func(ic *types.InstallConfig) { ic.GCP.ComputeSubnet = "invalid-compute-subnet" }
 	invalidateCPSubnet      = func(ic *types.InstallConfig) { ic.GCP.ControlPlaneSubnet = "invalid-cp-subnet" }
@@ -42,6 +64,12 @@ var (
 	removeVPC               = func(ic *types.InstallConfig) { ic.GCP.Network = "" }
 	removeSubnets           = func(ic *types.InstallConfig) { ic.GCP.ComputeSubnet, ic.GCP.ControlPlaneSubnet = "", "" }
 
+	machineTypeAPIResult = map[string]*compute.MachineType{
+		"n1-standard-1": {GuestCpus: 1, MemoryMb: 3840},
+		"n1-standard-2": {GuestCpus: 2, MemoryMb: 7680},
+		"n1-standard-4": {GuestCpus: 4, MemoryMb: 15360},
+	}
+
 	subnetAPIResult = []*compute.Subnetwork{
 		{
 			Name:        validCPSubnet,
@@ -63,13 +91,24 @@ func validInstallConfig() *types.InstallConfig {
 		},
 		Platform: types.Platform{
 			GCP: &gcp.Platform{
-				ProjectID:          validProjectName,
-				Region:             validRegion,
-				Network:            validNetworkName,
-				ComputeSubnet:      validComputeSubnet,
-				ControlPlaneSubnet: validCPSubnet,
+				DefaultMachinePlatform: &gcp.MachinePool{},
+				ProjectID:              validProjectName,
+				Region:                 validRegion,
+				Network:                validNetworkName,
+				ComputeSubnet:          validComputeSubnet,
+				ControlPlaneSubnet:     validCPSubnet,
 			},
 		},
+		ControlPlane: &types.MachinePool{
+			Platform: types.MachinePoolPlatform{
+				GCP: &gcp.MachinePool{},
+			},
+		},
+		Compute: []types.MachinePool{{
+			Platform: types.MachinePoolPlatform{
+				GCP: &gcp.MachinePool{},
+			},
+		}},
 	}
 }
 
@@ -122,6 +161,36 @@ func TestGCPInstallConfigValidation(t *testing.T) {
 			expectedError:  true,
 			expectedErrMsg: "computeSubnet: Invalid value.*controlPlaneSubnet: Invalid value",
 		},
+		{
+			name:           "Valid machine types",
+			edits:          editFunctions{validMachineTypes},
+			expectedError:  false,
+			expectedErrMsg: "",
+		},
+		{
+			name:           "Invalid default machine type",
+			edits:          editFunctions{invalidateDefaultMachineTypes},
+			expectedError:  true,
+			expectedErrMsg: `\[platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 4 vCPUs, platform.gcp.defaultMachinePlatform.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 15360 MB Memory\]`,
+		},
+		{
+			name:           "Invalid control plane machine types",
+			edits:          editFunctions{invalidateControlPlaneMachineTypes},
+			expectedError:  true,
+			expectedErrMsg: `[controlPlane.platform.gcp.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 4 vCPUs, controlPlane.platform.gcp.type: Invalid value: "n1\-standard\-1": instance type does not meet minimum resource requirements of 15361 MB Memory]`,
+		},
+		{
+			name:           "Invalid compute machine types",
+			edits:          editFunctions{invalidateComputeMachineTypes},
+			expectedError:  true,
+			expectedErrMsg: `\[compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 2 vCPUs, compute\[0\].platform.gcp.type: Invalid value: "n1-standard-1": instance type does not meet minimum resource requirements of 7680 MB Memory\]`,
+		},
+		{
+			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`,
+		},
 		{
 			name:           "Invalid region",
 			edits:          editFunctions{invalidateRegion},
@@ -153,6 +222,8 @@ 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()
 	// 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()
 
@@ -261,7 +332,7 @@ func TestGCPEnabledServicesList(t *testing.T) {
 			defer mockCtrl.Finish()
 			gcpClient := mock.NewMockAPI(mockCtrl)
 
-			gcpClient.EXPECT().GetEnabledServices(gomock.Any()).Return(test.services, nil).AnyTimes()
+			gcpClient.EXPECT().GetEnabledServices(gomock.Any(), gomock.Any()).Return(test.services, nil).AnyTimes()
 			err := ValidateEnabledServices(nil, gcpClient, "")
 			if test.err == "" {
 				assert.NoError(t, err)