From 339da85539667072c5829e08e8d36b40a20ebac7 Mon Sep 17 00:00:00 2001 From: chaodaiG <45011425+chaodaiG@users.noreply.github.com> Date: Fri, 27 Sep 2019 10:42:42 -0700 Subject: [PATCH] Cluster management lib: allow custom cluster name (#723) * Cluster management lib: allow custom cluster name * Apply suggestions from code review Co-Authored-By: Chi Zhang * Apply suggestions from code review Co-Authored-By: Chi Zhang * revert unit64 change * Apply suggestions from code review Co-Authored-By: Adriano Cunha <35786489+adrcunha@users.noreply.github.com> --- testutils/clustermanager/example_test.go | 19 ++- testutils/clustermanager/gke.go | 120 ++++++++------- testutils/clustermanager/gke_test.go | 178 +++++++++++++++++------ 3 files changed, 220 insertions(+), 97 deletions(-) diff --git a/testutils/clustermanager/example_test.go b/testutils/clustermanager/example_test.go index ccaa9a73a..e5e3691b3 100644 --- a/testutils/clustermanager/example_test.go +++ b/testutils/clustermanager/example_test.go @@ -26,14 +26,21 @@ func Example() { var ( minNodes int64 = 1 maxNodes int64 = 3 - nodeType = "n1-standard-8" - region = "us-east1" - zone = "a" - project = "myGKEproject" - addons = []string{"istio"} + nodeType = "n1-standard-8" + region = "us-east1" + zone = "a" + project = "myGKEproject" + addons = []string{"istio"} ) gkeClient := GKEClient{} - clusterOps := gkeClient.Setup(&minNodes, &maxNodes, &nodeType, ®ion, &zone, &project, addons) + clusterOps := gkeClient.Setup(GKERequest{ + MinNodes: minNodes, + MaxNodes: maxNodes, + NodeType: nodeType, + Region: region, + Zone: zone, + Project: project, + Addons: addons}) // Cast to GKEOperation gkeOps := clusterOps.(*GKECluster) if err := gkeOps.Initialize(); err != nil { diff --git a/testutils/clustermanager/gke.go b/testutils/clustermanager/gke.go index 8fe4f43af..df9003234 100644 --- a/testutils/clustermanager/gke.go +++ b/testutils/clustermanager/gke.go @@ -57,13 +57,35 @@ type GKEClient struct { // GKERequest contains all requests collected for cluster creation type GKERequest struct { - MinNodes int64 - MaxNodes int64 - NodeType string - Region string - Zone string + // Project: GKE project, no default. Fall back to get project from kubeconfig + // then gcloud config + Project string + + // ClusterName: custom cluster name to use. Fall back to cluster set by + // kubeconfig, else composed as k[REPO]-cls-e2e-[BUILD_ID] + ClusterName string + + // MinNodes: default to 1 if not provided + MinNodes int64 + + // MaxNodes: default to max(3, MinNodes) if not provided + MaxNodes int64 + + // NodeType: default to n1-standard-4 if not provided + NodeType string + + // Region: default to regional cluster if not provided, and use default backup regions + Region string + + // Zone: default is none, must be provided together with region + Zone string + + // BackupRegions: fall back regions to try out in case of cluster creation + // failure due to regional issue(s) BackupRegions []string - Addons []string + + // Addons: cluster addons to be added to cluster, such as istio + Addons []string } // GKECluster implements ClusterOperations @@ -123,55 +145,50 @@ func (gsc *GKESDKClient) setAutoscaling(project, clusterName, location, nodepool return gsc.Service.Projects.Locations.Clusters.NodePools.SetAutoscaling(parent, rb).Do() } -// Setup sets up a GKECluster client. -// minNodes: default to 1 if not provided -// maxNodes: default to 3 if not provided -// nodeType: default to n1-standard-4 if not provided -// region: default to regional cluster if not provided, and use default backup regions -// zone: default is none, must be provided together with region -// project: no default -// addons: cluster addons to be added to cluster -func (gs *GKEClient) Setup(minNodes *int64, maxNodes *int64, nodeType *string, region *string, zone *string, project *string, addons []string) ClusterOperations { - gc := &GKECluster{ - Request: &GKERequest{ - MinNodes: DefaultGKEMinNodes, - MaxNodes: DefaultGKEMaxNodes, - NodeType: DefaultGKENodeType, - Region: DefaultGKERegion, - Zone: DefaultGKEZone, - BackupRegions: DefaultGKEBackupRegions, - Addons: addons, - }, - } +// Setup sets up a GKECluster client, takes GEKRequest as parameter and applies +// all defaults if not defined. +func (gs *GKEClient) Setup(r GKERequest) ClusterOperations { + gc := &GKECluster{} - if project != nil { // use provided project and create cluster - gc.Project = project + if r.Project != "" { // use provided project and create cluster + gc.Project = &r.Project gc.NeedCleanup = true } - if minNodes != nil { - gc.Request.MinNodes = *minNodes + if r.MinNodes == 0 { + r.MinNodes = DefaultGKEMinNodes } - if maxNodes != nil { - gc.Request.MaxNodes = *maxNodes + if r.MaxNodes == 0 { + r.MaxNodes = DefaultGKEMaxNodes + // We don't want MaxNodes < MinNodes + if r.MinNodes > r.MaxNodes { + r.MaxNodes = r.MinNodes + } } - if nodeType != nil { - gc.Request.NodeType = *nodeType + if r.NodeType == "" { + r.NodeType = DefaultGKENodeType } - if region != nil { - gc.Request.Region = *region + // Only use default backup regions if region is not provided + if len(r.BackupRegions) == 0 && r.Region == "" { + r.BackupRegions = DefaultGKEBackupRegions + if common.GetOSEnv(backupRegionEnv) != "" { + r.BackupRegions = strings.Split(common.GetOSEnv(backupRegionEnv), " ") + } } - if common.GetOSEnv(regionEnv) != "" { - gc.Request.Region = common.GetOSEnv(regionEnv) + if r.Region == "" { + r.Region = DefaultGKERegion + if common.GetOSEnv(regionEnv) != "" { + r.Region = common.GetOSEnv(regionEnv) + } } - if common.GetOSEnv(backupRegionEnv) != "" { - gc.Request.BackupRegions = strings.Split(common.GetOSEnv(backupRegionEnv), " ") - } - if zone != nil { - gc.Request.Zone = *zone - gc.Request.BackupRegions = make([]string, 0) + if r.Zone == "" { + r.Zone = DefaultGKEZone + } else { // No backupregions if zone is provided + r.BackupRegions = make([]string, 0) } + gc.Request = &r + ctx := context.Background() c, err := google.DefaultClient(ctx, container.CloudPlatformScope) if err != nil { @@ -189,8 +206,8 @@ func (gs *GKEClient) Setup(minNodes *int64, maxNodes *int64, nodeType *string, r return gc } -// Initialize sets up GKE SDK client, checks environment for cluster and -// projects to decide whether use existing cluster/project or creating new ones. +// Initialize checks environment for cluster and projects to decide whether using +// existing cluster/project or creating new ones. func (gc *GKECluster) Initialize() error { // Try obtain project name via `kubectl`, `gcloud` if gc.Project == nil { @@ -229,15 +246,20 @@ func (gc *GKECluster) Provider() string { // Region or Zone is provided then there is no retries func (gc *GKECluster) Acquire() error { gc.ensureProtected() + var clusterName string var err error // Check if using existing cluster if gc.Cluster != nil { return nil } // Perform GKE specific cluster creation logics - clusterName, err := getResourceName(ClusterResource) - if err != nil { - return fmt.Errorf("failed getting cluster name: '%v'", err) + if gc.Request.ClusterName == "" { + clusterName, err = getResourceName(ClusterResource) + if err != nil { + return fmt.Errorf("failed getting cluster name: '%v'", err) + } + } else { + clusterName = gc.Request.ClusterName } regions := []string{gc.Request.Region} diff --git a/testutils/clustermanager/gke_test.go b/testutils/clustermanager/gke_test.go index ea16f2565..2cda1620a 100644 --- a/testutils/clustermanager/gke_test.go +++ b/testutils/clustermanager/gke_test.go @@ -178,16 +178,14 @@ func TestSetup(t *testing.T) { zoneOverride := "foozone" fakeAddons := "fake-addon" datas := []struct { - minNodes *int64 - maxNodes *int64 - nodeType, region, zone, project *string - addons []string - regionEnv, backupRegionEnv string - expClusterOperations *GKECluster + r GKERequest + regionEnv, backupRegionEnv string + expClusterOperations *GKECluster }{ { // Defaults - nil, nil, nil, nil, nil, nil, []string{}, "", "", + GKERequest{}, + "", "", &GKECluster{ Request: &GKERequest{ MinNodes: 1, @@ -196,28 +194,57 @@ func TestSetup(t *testing.T) { Region: "us-central1", Zone: "", BackupRegions: []string{"us-west1", "us-east1"}, - Addons: []string{}, + Addons: nil, }, }, }, { // Project provided - nil, nil, nil, nil, nil, &fakeProj, []string{}, "", "", + GKERequest{ + Project: fakeProj, + }, + "", "", &GKECluster{ Request: &GKERequest{ + Project: "b", MinNodes: 1, MaxNodes: 3, NodeType: "n1-standard-4", Region: "us-central1", Zone: "", BackupRegions: []string{"us-west1", "us-east1"}, - Addons: []string{}, + Addons: nil, }, Project: &fakeProj, NeedCleanup: true, }, + }, { + // Cluster name provided + GKERequest{ + ClusterName: "predefined-cluster-name", + }, + "", "", + &GKECluster{ + Request: &GKERequest{ + ClusterName: "predefined-cluster-name", + MinNodes: 1, + MaxNodes: 3, + NodeType: "n1-standard-4", + Region: "us-central1", + Zone: "", + BackupRegions: []string{"us-west1", "us-east1"}, + Addons: nil, + }, + }, }, { // Override other parts - &minNodesOverride, &maxNodesOverride, &nodeTypeOverride, ®ionOverride, &zoneOverride, nil, []string{}, "", "", + GKERequest{ + MinNodes: minNodesOverride, + MaxNodes: maxNodesOverride, + NodeType: nodeTypeOverride, + Region: regionOverride, + Zone: zoneOverride, + }, + "", "", &GKECluster{ Request: &GKERequest{ MinNodes: 2, @@ -226,12 +253,18 @@ func TestSetup(t *testing.T) { Region: "fooregion", Zone: "foozone", BackupRegions: []string{}, - Addons: []string{}, + Addons: nil, }, }, }, { // Override other parts but not zone - &minNodesOverride, &maxNodesOverride, &nodeTypeOverride, ®ionOverride, nil, nil, []string{}, "", "", + GKERequest{ + MinNodes: minNodesOverride, + MaxNodes: maxNodesOverride, + NodeType: nodeTypeOverride, + Region: regionOverride, + }, + "", "", &GKECluster{ Request: &GKERequest{ MinNodes: 2, @@ -239,13 +272,14 @@ func TestSetup(t *testing.T) { NodeType: "foonode", Region: "fooregion", Zone: "", - BackupRegions: []string{"us-west1", "us-east1"}, - Addons: []string{}, + BackupRegions: nil, + Addons: nil, }, }, }, { // Set env Region - nil, nil, nil, nil, nil, nil, []string{}, "customregion", "", + GKERequest{}, + "customregion", "", &GKECluster{ Request: &GKERequest{ MinNodes: 1, @@ -254,12 +288,13 @@ func TestSetup(t *testing.T) { Region: "customregion", Zone: "", BackupRegions: []string{"us-west1", "us-east1"}, - Addons: []string{}, + Addons: nil, }, }, }, { // Set env backupzone - nil, nil, nil, nil, nil, nil, []string{}, "", "backupregion1 backupregion2", + GKERequest{}, + "", "backupregion1 backupregion2", &GKECluster{ Request: &GKERequest{ MinNodes: 1, @@ -268,12 +303,15 @@ func TestSetup(t *testing.T) { Region: "us-central1", Zone: "", BackupRegions: []string{"backupregion1", "backupregion2"}, - Addons: []string{}, + Addons: nil, }, }, }, { // Set addons - nil, nil, nil, nil, nil, nil, []string{fakeAddons}, "", "", + GKERequest{ + Addons: []string{fakeAddons}, + }, + "", "", &GKECluster{ Request: &GKERequest{ MinNodes: 1, @@ -331,15 +369,15 @@ func TestSetup(t *testing.T) { return oldEnvFunc(s) } c := GKEClient{} - co := c.Setup(data.minNodes, data.maxNodes, data.nodeType, data.region, data.zone, data.project, data.addons) - errMsg := fmt.Sprintf("testing setup with:\n\tminNodes: %v\n\tnodeType: %v\n\tregion: %v\n\tzone: %v\n\tproject: %v\n\taddons: %v\n\tregionEnv: %v\n\tbackupRegionEnv: %v", - data.minNodes, data.nodeType, data.region, data.zone, data.project, data.addons, data.regionEnv, data.backupRegionEnv) + co := c.Setup(data.r) + errMsg := fmt.Sprintf("testing setup with:\n\t%+v\n\tregionEnv: %v\n\tbackupRegionEnv: %v", + data.r, data.regionEnv, data.backupRegionEnv) gotCo := co.(*GKECluster) // mock for easier comparison gotCo.operations = nil gotCo.boskosOps = nil - if !reflect.DeepEqual(co, data.expClusterOperations) { - t.Fatalf("%s\nwant GKECluster:\n'%v'\ngot GKECluster:\n'%v'", errMsg, data.expClusterOperations, co) + if dif := cmp.Diff(gotCo.Request, data.expClusterOperations.Request); dif != "" { + t.Errorf("%s\nRequest got(+) is different from wanted(-)\n%v", errMsg, dif) } } } @@ -575,23 +613,41 @@ func TestGKECheckEnvironment(t *testing.T) { } func TestAcquire(t *testing.T) { + predefinedClusterName := "predefined-cluster-name" fakeClusterName := "kpkg-e2e-cls-1234" fakeBuildID := "1234" datas := []struct { - existCluster *container.Cluster - kubeconfigSet bool - addons []string - nextOpStatus []string - expCluster *container.Cluster - expErr error - expPanic bool + existCluster *container.Cluster + predefinedClusterName string + kubeconfigSet bool + addons []string + nextOpStatus []string + expCluster *container.Cluster + expErr error + expPanic bool }{ { // cluster already found &container.Cluster{ Name: "customcluster", Location: "us-central1", - }, true, []string{}, []string{}, &container.Cluster{ + }, "", true, []string{}, []string{}, &container.Cluster{ + Name: "customcluster", + Location: "us-central1", + Status: "RUNNING", + AddonsConfig: &container.AddonsConfig{}, + NodePools: []*container.NodePool{ + { + Name: "default-pool", + }, + }, + }, nil, false, + }, { + // cluster already found and clustername predefined + &container.Cluster{ + Name: "customcluster", + Location: "us-central1", + }, predefinedClusterName, true, []string{}, []string{}, &container.Cluster{ Name: "customcluster", Location: "us-central1", Status: "RUNNING", @@ -608,7 +664,7 @@ func TestAcquire(t *testing.T) { &container.Cluster{ Name: fakeClusterName, Location: "us-central1", - }, false, []string{}, []string{}, &container.Cluster{ + }, "", false, []string{}, []string{}, &container.Cluster{ Name: fakeClusterName, Location: "us-central1", Status: "RUNNING", @@ -629,7 +685,7 @@ func TestAcquire(t *testing.T) { &container.Cluster{ Name: fakeClusterName, Location: "us-central1", - }, false, []string{}, []string{"BAD"}, &container.Cluster{ + }, "", false, []string{}, []string{"BAD"}, &container.Cluster{ Name: fakeClusterName, Location: "us-west1", Status: "RUNNING", @@ -644,9 +700,46 @@ func TestAcquire(t *testing.T) { Username: "admin", }, }, nil, false, + }, { + // cluster exists but not set in kubeconfig, clusterName defined + &container.Cluster{ + Name: fakeClusterName, + Location: "us-central1", + }, predefinedClusterName, false, []string{}, []string{}, &container.Cluster{ + Name: predefinedClusterName, + Location: "us-central1", + Status: "RUNNING", + AddonsConfig: &container.AddonsConfig{}, + NodePools: []*container.NodePool{ + { + Name: "default-pool", + Autoscaling: &container.NodePoolAutoscaling{Enabled: true, MaxNodeCount: 3, MinNodeCount: 1}, + }, + }, + MasterAuth: &container.MasterAuth{ + Username: "admin", + }, + }, nil, false, + }, { + // cluster not exist, but clustername defined + nil, predefinedClusterName, false, []string{}, []string{}, &container.Cluster{ + Name: predefinedClusterName, + Location: "us-central1", + Status: "RUNNING", + AddonsConfig: &container.AddonsConfig{}, + NodePools: []*container.NodePool{ + { + Name: "default-pool", + Autoscaling: &container.NodePoolAutoscaling{Enabled: true, MaxNodeCount: 3, MinNodeCount: 1}, + }, + }, + MasterAuth: &container.MasterAuth{ + Username: "admin", + }, + }, nil, false, }, { // cluster creation succeeded - nil, false, []string{}, []string{}, &container.Cluster{ + nil, "", false, []string{}, []string{}, &container.Cluster{ Name: fakeClusterName, Location: "us-central1", Status: "RUNNING", @@ -663,7 +756,7 @@ func TestAcquire(t *testing.T) { }, nil, false, }, { // cluster creation succeeded with addon - nil, false, []string{"istio"}, []string{}, &container.Cluster{ + nil, "", false, []string{"istio"}, []string{}, &container.Cluster{ Name: fakeClusterName, Location: "us-central1", Status: "RUNNING", @@ -682,7 +775,7 @@ func TestAcquire(t *testing.T) { }, nil, false, }, { // cluster creation succeeded retry - nil, false, []string{}, []string{"PENDING"}, &container.Cluster{ + nil, "", false, []string{}, []string{"PENDING"}, &container.Cluster{ Name: fakeClusterName, Location: "us-west1", Status: "RUNNING", @@ -699,7 +792,7 @@ func TestAcquire(t *testing.T) { }, nil, false, }, { // cluster creation failed set addon, but succeeded retry - nil, false, []string{}, []string{"DONE", "PENDING"}, &container.Cluster{ + nil, "", false, []string{}, []string{"DONE", "PENDING"}, &container.Cluster{ Name: fakeClusterName, Location: "us-west1", Status: "RUNNING", @@ -716,13 +809,13 @@ func TestAcquire(t *testing.T) { }, nil, false, }, { // cluster creation failed all retry - nil, false, []string{}, []string{"PENDING", "PENDING", "PENDING"}, nil, fmt.Errorf("timed out waiting"), false, + nil, "", false, []string{}, []string{"PENDING", "PENDING", "PENDING"}, nil, fmt.Errorf("timed out waiting"), false, }, { // cluster creation went bad state - nil, false, []string{}, []string{"BAD", "BAD", "BAD"}, nil, fmt.Errorf("unexpected operation status: %q", "BAD"), false, + nil, "", false, []string{}, []string{"BAD", "BAD", "BAD"}, nil, fmt.Errorf("unexpected operation status: %q", "BAD"), false, }, { // bad addon, should get a panic - nil, false, []string{"bad_addon"}, []string{}, nil, nil, true, + nil, "", false, []string{"bad_addon"}, []string{}, nil, nil, true, }, } @@ -783,6 +876,7 @@ func TestAcquire(t *testing.T) { } fgc.Request = &GKERequest{ + ClusterName: data.predefinedClusterName, MinNodes: DefaultGKEMinNodes, MaxNodes: DefaultGKEMaxNodes, NodeType: DefaultGKENodeType,