Merge pull request #4163 from abhinavdahiya/bz-invalid-parse-machine-…

…type

Bug 1878108: asset/quota/gcp: use GCP api to find CPU count for constraint and guess only on failure

pkg/asset/quota/gcp/client.go

@@ -0,0 +1,37 @@
+package gcp
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	computev1 "google.golang.org/api/compute/v1"
+	"google.golang.org/api/option"
+
+	gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
+)
+
+// MachineTypeGetter returns the machine type info for a type in a zone using GCP API.
+type MachineTypeGetter interface {
+	GetMachineType(zone string, machineType string) (*computev1.MachineType, error)
+}
+
+// Client is GCP client for calculating quota constraint.
+type Client struct {
+	computeSvc *computev1.Service
+
+	projectID string
+}
+
+// NewClient returns Client using the context and session.
+func NewClient(ctx context.Context, sess *gcpconfig.Session, projectID string) (*Client, error) {
+	svc, err := computev1.NewService(ctx, option.WithCredentials(sess.Credentials))
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to create compute service")
+	}
+	return &Client{computeSvc: svc, projectID: projectID}, nil
+}
+
+// GetMachineType returns the machine type info for a type in a zone using the client.
+func (c *Client) GetMachineType(zone string, machineType string) (*computev1.MachineType, error) {
+	return c.computeSvc.MachineTypes.Get(c.projectID, zone, machineType).Context(context.TODO()).Do()
+}

pkg/asset/quota/gcp/gcp.go

@@ -16,7 +16,7 @@ import (
 // Constraints returns a list of quota constraints based on the InstallConfig.
 // These constraints can be used to check if there is enough quota for creating a cluster
 // for the isntall config.
-func Constraints(config *types.InstallConfig, controlPlanes []machineapi.Machine, computes []machineapi.MachineSet) []quota.Constraint {
+func Constraints(client *Client, config *types.InstallConfig, controlPlanes []machineapi.Machine, computes []machineapi.MachineSet) []quota.Constraint {
 	ctrplConfigs := make([]*gcpprovider.GCPMachineProviderSpec, len(controlPlanes))
 	for i, m := range controlPlanes {
 		ctrplConfigs[i] = m.Spec.ProviderSpec.Value.Object.(*gcpprovider.GCPMachineProviderSpec)
@@ -33,8 +33,8 @@ func Constraints(config *types.InstallConfig, controlPlanes []machineapi.Machine
 		network(config),
 		apiExternal(config),
 		apiInternal(config),
-		controlPlane(config, ctrplConfigs),
-		compute(config, computeReplicas, computeConfigs),
+		controlPlane(client, config, ctrplConfigs),
+		compute(client, config, computeReplicas, computeConfigs),
 		others,
 	} {
 		ret = append(ret, gen()...)
@@ -140,11 +140,11 @@ func apiInternal(config *types.InstallConfig) func() []quota.Constraint {
 	}
 }
 
-func controlPlane(config *types.InstallConfig, machines []*gcpprovider.GCPMachineProviderSpec) func() []quota.Constraint {
+func controlPlane(client MachineTypeGetter, config *types.InstallConfig, machines []*gcpprovider.GCPMachineProviderSpec) func() []quota.Constraint {
 	return func() []quota.Constraint {
 		var ret []quota.Constraint
 		for _, m := range machines {
-			q := machineTypeToQuota(m.MachineType)
+			q := machineTypeToQuota(client, m.Zone, m.MachineType)
 			q.Region = config.Platform.GCP.Region
 			ret = append(ret, q)
 		}
@@ -158,11 +158,11 @@ func controlPlane(config *types.InstallConfig, machines []*gcpprovider.GCPMachin
 	}
 }
 
-func compute(config *types.InstallConfig, replicas []int64, machines []*gcpprovider.GCPMachineProviderSpec) func() []quota.Constraint {
+func compute(client MachineTypeGetter, config *types.InstallConfig, replicas []int64, machines []*gcpprovider.GCPMachineProviderSpec) func() []quota.Constraint {
 	return func() []quota.Constraint {
 		var ret []quota.Constraint
 		for idx, m := range machines {
-			q := machineTypeToQuota(m.MachineType)
+			q := machineTypeToQuota(client, m.Zone, m.MachineType)
 			q.Count = q.Count * replicas[idx]
 			q.Region = config.Platform.GCP.Region
 			ret = append(ret, q)
@@ -189,30 +189,45 @@ func others() []quota.Constraint {
 	}}
 }
 
-func machineTypeToQuota(t string) quota.Constraint {
-	var class string
-	var count int64
-	split := strings.Split(t, "-")
+func machineTypeToQuota(client MachineTypeGetter, zone string, machineType string) quota.Constraint {
+	var name string
+	class := strings.SplitN(machineType, "-", 2)[0]
+	switch class {
+	case "c2", "m1", "m2", "n2", "n2d":
+		name = fmt.Sprintf("compute.googleapis.com/%s_cpus", class)
+	default:
+		name = "compute.googleapis.com/cpus"
+	}
+
+	info, err := client.GetMachineType(zone, machineType)
+	if err != nil {
+		return quota.Constraint{Name: name, Count: guessMachineCPUCount(machineType)}
+	}
+	return quota.Constraint{Name: name, Count: info.GuestCpus}
+}
+
+// the guess is based on https://cloud.google.com/compute/docs/machine-types
+func guessMachineCPUCount(machineType string) int64 {
+	split := strings.Split(machineType, "-")
 	switch len(split) {
-	case 3, 4:
-		class = split[0]
+	case 4:
 		if c, err := strconv.ParseInt(split[2], 10, 0); err == nil {
-			count = c
+			return c
+		}
+	case 3:
+		switch split[0] {
+		case "c2", "m1", "m2", "n1", "n2", "n2d", "e2":
+			if c, err := strconv.ParseInt(split[2], 10, 0); err == nil {
+				return c
+			}
 		}
 	case 2:
-		class = split[0]
-	}
-	switch class {
-	case "c2", "m1", "m2", "n2", "n2d":
-		return quota.Constraint{Name: fmt.Sprintf("compute.googleapis.com/%s_cpus", class), Count: count}
-	case "e2":
-		if count == 0 {
-			count = 2
+		switch split[0] {
+		case "e2":
+			return 2
+		case "f1", "g1":
+			return 1
 		}
-		return quota.Constraint{Name: "compute.googleapis.com/cpus", Count: count}
-	case "f1", "g1":
-		return quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 1}
-	default:
-		return quota.Constraint{Name: "compute.googleapis.com/cpus", Count: count}
 	}
+	return 0
 }

pkg/asset/quota/gcp/gcp_test.go

@@ -1,11 +1,14 @@
 package gcp
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 
-	"github.com/openshift/installer/pkg/quota"
 	"github.com/stretchr/testify/assert"
+	computev1 "google.golang.org/api/compute/v1"
+
+	"github.com/openshift/installer/pkg/quota"
 )
 
 func Test_aggregate(t *testing.T) {
@@ -56,96 +59,197 @@ func Test_aggregate(t *testing.T) {
 	}
 }
 
-func Test_machineTypeToQuota(t *testing.T) {
+func Test_guessMachineCPUCount(t *testing.T) {
 	cases := []struct {
 		machineType string
-		expected    quota.Constraint
+		expected    int64
 	}{{
 		machineType: "e2-standard-2",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+		expected:    2,
 	}, {
 		machineType: "e2-standard-8-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 8},
+		expected:    8,
 	}, {
 		machineType: "e2-highmem-2",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+		expected:    2,
 	}, {
 		machineType: "e2-highmem-8-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 8},
+		expected:    8,
 	}, {
 		machineType: "e2-highcpu-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "e2-highcpu-16-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n2-standard-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2-standard-16-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n2-standard-16.5-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 0},
+		expected:    0,
 	}, {
 		machineType: "n2-highmem-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2-highmem-32-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 32},
+		expected:    32,
 	}, {
 		machineType: "n2-highcpu-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2-highcpu-16",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n2d-standard-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2d-standard-16",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n2d-highmem-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2d-highmem-16",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n2d-highcpu-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "n2d-highcpu-16",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/n2d_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "n1-standard-2",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+		expected:    2,
 	}, {
 		machineType: "n1-standard-8-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 8},
+		expected:    8,
 	}, {
 		machineType: "c2-standard-4",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/c2_cpus", Count: 4},
+		expected:    4,
 	}, {
 		machineType: "c2-standard-16-4096",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/c2_cpus", Count: 16},
+		expected:    16,
 	}, {
 		machineType: "e2-micro",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+		expected:    2,
 	}, {
 		machineType: "e2-medium",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+		expected:    2,
 	}, {
 		machineType: "f1-micro",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 1},
+		expected:    1,
 	}, {
 		machineType: "g1-small",
-		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 1},
+		expected:    1,
 	}}
 	for idx, test := range cases {
 		t.Run(fmt.Sprintf("test %d", idx), func(t *testing.T) {
-			got := machineTypeToQuota(test.machineType)
+			got := guessMachineCPUCount(test.machineType)
+			assert.EqualValues(t, test.expected, got)
+		})
+	}
+}
+
+func Test_machineTypeToQuota(t *testing.T) {
+	fake := newFakeMachineTypeGetter([]*computev1.MachineType{{
+		Zone:      "a",
+		Name:      "n1-standard-2",
+		GuestCpus: 2,
+	}, {
+		Zone:      "a",
+		Name:      "n1-custom-2-1024",
+		GuestCpus: 2,
+	}, {
+		Zone:      "a",
+		Name:      "custom-2-1024",
+		GuestCpus: 2,
+	}, {
+		Zone:      "a",
+		Name:      "n2-standard-4",
+		GuestCpus: 4,
+	}, {
+		Zone:      "a",
+		Name:      "n2-custom-4-1024",
+		GuestCpus: 4,
+	}})
+
+	tests := []struct {
+		zone        string
+		machineType string
+		expected    quota.Constraint
+	}{{
+		zone:        "a",
+		machineType: "n1-standard-2",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+	}, {
+		zone:        "a",
+		machineType: "n1-custom-2-1024",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+	}, {
+		zone:        "a",
+		machineType: "n1-custom-2-2048",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+	}, {
+		zone:        "b",
+		machineType: "n1-standard-2",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+	}, {
+		zone:        "a",
+		machineType: "custom-2-1024",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 2},
+	}, {
+		zone:        "a",
+		machineType: "custom-2-2048",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 0},
+	}, {
+		zone:        "b",
+		machineType: "custom-2-1024",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/cpus", Count: 0},
+	}, {
+		zone:        "a",
+		machineType: "n2-standard-4",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+	}, {
+		zone:        "a",
+		machineType: "n2-custom-4-1024",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+	}, {
+		zone:        "b",
+		machineType: "n2-standard-4",
+		expected:    quota.Constraint{Name: "compute.googleapis.com/n2_cpus", Count: 4},
+	}}
+
+	for idx, test := range tests {
+		t.Run(fmt.Sprintf("test %d", idx), func(t *testing.T) {
+			got := machineTypeToQuota(fake, test.zone, test.machineType)
 			assert.EqualValues(t, test.expected, got)
 		})
 	}
 }
+
+type fakeMachineTypeGetter struct {
+	knownTypes map[string]*computev1.MachineType
+}
+
+func newFakeMachineTypeGetter(mtypes []*computev1.MachineType) *fakeMachineTypeGetter {
+	fake := &fakeMachineTypeGetter{
+		knownTypes: map[string]*computev1.MachineType{},
+	}
+
+	for idx, mtype := range mtypes {
+		fake.knownTypes[fmt.Sprintf("%s__%s", mtype.Zone, mtype.Name)] = mtypes[idx]
+	}
+
+	return fake
+}
+
+func (fake *fakeMachineTypeGetter) GetMachineType(zone string, machineType string) (*computev1.MachineType, error) {
+	mtype, ok := fake.knownTypes[fmt.Sprintf("%s__%s", zone, machineType)]
+	if !ok {
+		return nil, errors.New("unknwown")
+	}
+	return mtype, nil
+}