Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/asset/installconfig/azure/mock/azureclient_generated.go

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

31 changes: 31 additions & 0 deletions pkg/asset/installconfig/gcp/client.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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
jstuever marked this conversation as resolved.
Show resolved Hide resolved
}
}
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)
Expand Down
54 changes: 35 additions & 19 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.

75 changes: 75 additions & 0 deletions pkg/asset/installconfig/gcp/validation.go
Expand Up @@ -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))
}

jstuever marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
83 changes: 77 additions & 6 deletions pkg/asset/installconfig/gcp/validation_test.go
Expand Up @@ -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" }
Expand All @@ -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,
Expand All @@ -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{},
},
}},
}
}

Expand Down Expand Up @@ -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`,
},
jstuever marked this conversation as resolved.
Show resolved Hide resolved
{
name: "Invalid region",
edits: editFunctions{invalidateRegion},
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down