test: add cluster-names e2e arg (#713)

This argument accepts a list of cluster names. This can either be used
with the create-clusters arg to create clusters with the specific name,
or without create-clusters to assume pre-provisioned clusters. If
create-clusters is provided for a cluster that already exists, it will
reuse the existing cluster.

This is written in a way that works for both KinD and GKE clusters, which
reconciles one of the final differences between the interfaces for
interacting with the two cluster types.

Also adds a delete-clusters option, which can be used to indicate
whether to delete clusters after the tests. This can be used in
conjunction with create-clusters to create clusters in the test
execution, and leave them for a subsequent execution.

Makefile.e2e

@@ -7,6 +7,9 @@ E2E_PARAMS := \
 
 GOTOPT2_BINARY := docker run -i -u $(UID):$(GID) $(BUILDENV_IMAGE) /bin/gotopt2
 
+E2E_CREATE_CLUSTERS ?= "true"
+E2E_DESTROY_CLUSTERS ?= "true"
+
 # Runs nomos vet locally for example repos.
 # Requires kubeconfig set up properly with Nomos cluster.
 # This is now a duplicate of cli.bats from the e2e tests framework,
@@ -41,10 +44,12 @@ test-e2e-nobuild: install-kustomize install-helm
 .PHONY: test-e2e-gke-nobuild
 test-e2e-gke-nobuild:
 	$(MAKE) test-e2e-nobuild \
-		E2E_ARGS="$(E2E_ARGS) --timeout=$(GKE_E2E_TIMEOUT) --share-test-env --create-clusters --test-cluster=gke" \
+		E2E_ARGS="$(E2E_ARGS) --timeout=$(GKE_E2E_TIMEOUT) --share-test-env --test-cluster=gke" \
 		GCP_PROJECT=$(GCP_PROJECT) \
 		GCP_ZONE=$(GCP_ZONE) \
-		GCP_REGION=$(GCP_REGION)
+		GCP_REGION=$(GCP_REGION) \
+		E2E_CREATE_CLUSTERS=$(E2E_CREATE_CLUSTERS) \
+		E2E_DESTROY_CLUSTERS=$(E2E_DESTROY_CLUSTERS)
 
 # Build Config Sync and run e2e tests on GKE
 .PHONY: test-e2e-gke

docs/development.md

@@ -122,7 +122,7 @@ deleted after the test execution.
 ```shell
 # GKE cluster options can be provided as command line flags
 go test ./e2e/... --e2e --test.v --share-test-env --gcp-project=<PROJECT_ID> --test-cluster=gke \
-  --create-clusters --num-clusters=5 --gcp-zone=us-central1-a \
+  --create-clusters=true --num-clusters=5 --gcp-zone=us-central1-a \
   --test.run (test name regexp)
 ```
 

e2e/flags.go

@@ -19,13 +19,36 @@ package e2e
 import (
 	"flag"
 	"fmt"
+	"strings"
 	"testing"
 
 	// kubectl auth provider plugins
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"kpt.dev/configsync/pkg/util"
 )
 
+// stringListFlag parses a comma delimited string field into a string slice
+type stringListFlag struct {
+	arr []string
+}
+
+func (i *stringListFlag) String() string {
+	return strings.Join(i.arr, ",")
+}
+
+func (i *stringListFlag) Set(value string) error {
+	i.arr = strings.Split(value, ",")
+	return nil
+}
+
+func newStringListFlag(name string, def []string, usage string) *[]string {
+	lf := &stringListFlag{
+		arr: def,
+	}
+	flag.Var(lf, name, usage)
+	return &lf.arr
+}
+
 // E2E enables running end-to-end tests.
 var E2E = flag.Bool("e2e", false,
 	"If true, run end-to-end tests.")
@@ -93,6 +116,11 @@ var TestFeatures = flag.String("test-features", "",
 var NumClusters = flag.Int("num-clusters", util.EnvInt("E2E_NUM_CLUSTERS", 1),
 	"Number of parallel test threads to run. Also dictates the number of clusters which will be created in parallel. Overrides the -test.parallel flag.")
 
+// ClusterNames is a list of cluster names to use for the tests. If specified without
+// create-clusters, assumes the clusters were pre-provisioned.
+var ClusterNames = newStringListFlag("cluster-names", util.EnvList("E2E_CLUSTER_NAMES", nil),
+	"List of cluster names to use for the tests. If specified without create-clusters, assumes the clusters were pre-provisioned.")
+
 // Usage indicates to print usage and exit. This is a workaround for the builtin
 // help command of `go test`
 var Usage = flag.Bool("usage", false, "Print usage and exit.")
@@ -141,9 +169,17 @@ var GKENumNodes = flag.Int("gke-num-nodes", util.EnvInt("GKE_NUM_NODES", Default
 var GKEAutopilot = flag.Bool("gke-autopilot", util.EnvBool("GKE_AUTOPILOT", false),
 	"Whether to create GKE clusters with autopilot enabled.")
 
-// CreateClusters indicates the test framework should create GKE clusters.
-var CreateClusters = flag.Bool("create-clusters", util.EnvBool("CREATE_CLUSTERS", false),
-	"Whether to create GKE clusters, otherwise will assume pre-provisioned GKE clusters.")
+// CreateClusters indicates the test framework should create clusters.
+var CreateClusters = flag.String("create-clusters", util.EnvString("E2E_CREATE_CLUSTERS", CreateClustersDisabled),
+	fmt.Sprintf("Whether to create clusters, otherwise will assume pre-provisioned clusters. Allowed values: [%s]",
+		strings.Join(CreateClustersAllowedValues, ", ")))
+
+// CreateClustersAllowedValues is a list of allowed values for the create-clusters parameter
+var CreateClustersAllowedValues = []string{CreateClustersEnabled, CreateClustersLazy, CreateClustersDisabled}
+
+// DestroyClusters indicates whether to destroy clusters that were created by the test suite after the tests.
+var DestroyClusters = flag.Bool("destroy-clusters", util.EnvBool("E2E_DESTROY_CLUSTERS", true),
+	"Whether to destroy clusters that were created by the test suite after the tests.")
 
 const (
 	// Kind indicates creating a Kind cluster for testing.
@@ -158,6 +194,14 @@ const (
 	DefaultGKEMachineType = "e2-standard-4"
 	// DefaultGKENumNodes is the default number of nodes to use when creating a GKE cluster
 	DefaultGKENumNodes = 3
+	// CreateClustersEnabled indicates that clusters should be created and error
+	// if the cluster already exists.
+	CreateClustersEnabled = "true"
+	// CreateClustersLazy indicates to use clusters that exist and create them
+	// if they don't
+	CreateClustersLazy = "lazy"
+	// CreateClustersDisabled indicates to not create clusters
+	CreateClustersDisabled = "false"
 )
 
 const (

e2e/nomostest/client.go

@@ -102,6 +102,8 @@ func newScheme(t testing.NTB) *runtime.Scheme {
 
 // Cluster is an interface for a target k8s cluster used by the e2e tests
 type Cluster interface {
+	// Exists whether the cluster exists
+	Exists() (bool, error)
 	// Create the k8s cluster
 	Create() error
 	// Delete the k8s cluster
@@ -110,16 +112,54 @@ type Cluster interface {
 	Connect() error
 }
 
+func upsertCluster(t testing.NTB, cluster Cluster, opts *ntopts.New) {
+	exists, err := cluster.Exists()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if exists && *e2e.CreateClusters != e2e.CreateClustersLazy {
+		t.Fatalf("cluster %s already exists and create-clusters=%s", opts.ClusterName, *e2e.CreateClusters)
+	}
+	t.Cleanup(func() {
+		if !*e2e.DestroyClusters {
+			t.Logf("[WARNING] Skipping deletion of %s cluster %s (--destroy-clusters=false)", *e2e.TestCluster, opts.ClusterName)
+			return
+		} else if t.Failed() && *e2e.Debug {
+			t.Logf("[WARNING] Skipping deletion of %s cluster %s (tests failed with --debug)", *e2e.TestCluster, opts.ClusterName)
+			return
+		}
+		deleteStart := time.Now()
+		t.Logf("Deleting %s cluster %s at %s",
+			*e2e.TestCluster, opts.ClusterName, deleteStart.Format(time.RFC3339))
+		defer func() {
+			t.Logf("took %s to delete %s cluster %s",
+				time.Since(deleteStart), *e2e.TestCluster, opts.ClusterName)
+		}()
+		if err := cluster.Delete(); err != nil {
+			t.Error(err)
+		}
+	})
+	if exists && *e2e.CreateClusters == e2e.CreateClustersLazy {
+		return
+	}
+	createStart := time.Now()
+	t.Logf("Creating %s cluster %s at %s",
+		*e2e.TestCluster, opts.ClusterName, createStart.Format(time.RFC3339))
+	if err := cluster.Create(); err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("took %s to create %s cluster %s",
+		time.Since(createStart), *e2e.TestCluster, opts.ClusterName)
+}
+
 // RestConfig sets up the config for creating a Client connection to a K8s cluster.
 // If --test-cluster=kind, it creates a Kind cluster.
 // If --test-cluster=kubeconfig, it uses the context specified in kubeconfig.
 func RestConfig(t testing.NTB, opts *ntopts.New) {
 	opts.KubeconfigPath = filepath.Join(opts.TmpDir, clusters.Kubeconfig)
 	t.Logf("kubeconfig will be created at %s", opts.KubeconfigPath)
 	t.Logf("Connect to %s cluster using:\nexport KUBECONFIG=%s", *e2e.TestCluster, opts.KubeconfigPath)
-	createCluster := true
 	opts.ClusterName = opts.Name
-	opts.IsEphemeralCluster = true
 	var cluster Cluster
 	switch strings.ToLower(*e2e.TestCluster) {
 	case e2e.Kind:
@@ -131,11 +171,6 @@ func RestConfig(t testing.NTB, opts *ntopts.New) {
 			KubernetesVersion: *e2e.KubernetesVersion,
 		}
 	case e2e.GKE:
-		if !*e2e.CreateClusters {
-			createCluster = false
-			opts.ClusterName = *e2e.GCPCluster
-			opts.IsEphemeralCluster = false
-		}
 		cluster = &clusters.GKECluster{
 			T:              t,
 			Name:           opts.ClusterName,
@@ -144,31 +179,8 @@ func RestConfig(t testing.NTB, opts *ntopts.New) {
 	default:
 		t.Fatalf("unsupported test cluster config %s. Allowed values are %s and %s.", *e2e.TestCluster, e2e.GKE, e2e.Kind)
 	}
-	if createCluster {
-		t.Cleanup(func() {
-			if t.Failed() && *e2e.Debug {
-				t.Logf("[WARNING] Skipping deletion of %s cluster %s", *e2e.TestCluster, opts.ClusterName)
-				return
-			}
-			deleteStart := time.Now()
-			t.Logf("Deleting %s cluster %s at %s",
-				*e2e.TestCluster, opts.ClusterName, deleteStart.Format(time.RFC3339))
-			defer func() {
-				t.Logf("took %s to delete %s cluster %s",
-					time.Since(deleteStart), *e2e.TestCluster, opts.ClusterName)
-			}()
-			if err := cluster.Delete(); err != nil {
-				t.Error(err)
-			}
-		})
-		createStart := time.Now()
-		t.Logf("Creating %s cluster %s at %s",
-			*e2e.TestCluster, opts.ClusterName, createStart.Format(time.RFC3339))
-		if err := cluster.Create(); err != nil {
-			t.Fatal(err)
-		}
-		t.Logf("took %s to create %s cluster %s",
-			time.Since(createStart), *e2e.TestCluster, opts.ClusterName)
+	if *e2e.CreateClusters != e2e.CreateClustersDisabled {
+		upsertCluster(t, cluster, opts)
 	}
 	t.Logf("Connecting to %s cluster %s",
 		*e2e.TestCluster, opts.ClusterName)

e2e/nomostest/clusters/gke.go

@@ -46,6 +46,11 @@ type GKECluster struct {
 	KubeConfigPath string
 }
 
+// Exists returns whether the GKE cluster exists
+func (c *GKECluster) Exists() (bool, error) {
+	return clusterExistsGKE(c.T, c.Name)
+}
+
 // Create the GKE cluster
 func (c *GKECluster) Create() error {
 	return createGKECluster(c.T, c.Name)
@@ -261,3 +266,30 @@ func getGKECredentials(t testing.NTB, clusterName, kubeconfig string) error {
 	// before handing over this cluster to the test environment
 	return listAndWaitForOperations(context.Background(), t, clusterName)
 }
+
+func clusterExistsGKE(t testing.NTB, clusterName string) (bool, error) {
+	args := []string{
+		"container", "clusters", "list",
+		"--project", *e2e.GCPProject,
+		"--filter", fmt.Sprintf("name ~ ^%s$", clusterName),
+		"--format", "value(name)",
+	}
+	if *e2e.GCPZone != "" {
+		args = append(args, "--zone", *e2e.GCPZone)
+	}
+	if *e2e.GCPRegion != "" {
+		args = append(args, "--region", *e2e.GCPRegion)
+	}
+	t.Logf("gcloud %s", strings.Join(args, " "))
+	cmd := exec.Command("gcloud", args...)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return false, errors.Errorf("failed to list cluster (%s): %v\nstdout/stderr:\n%s",
+			clusterName, err, string(out))
+	}
+	clusters := strings.Fields(string(out))
+	if len(clusters) == 1 && clusters[0] == clusterName {
+		return true, nil
+	}
+	return false, nil
+}

e2e/nomostest/clusters/kind.go

@@ -70,10 +70,30 @@ type KindCluster struct {
 	provider           *cluster.Provider
 }
 
+// Exists returns whether the KinD cluster exists
+func (c *KindCluster) Exists() (bool, error) {
+	c.initProvider()
+	kindClusters, err := c.provider.List()
+	if err != nil {
+		return false, err
+	}
+	for _, kindCluster := range kindClusters {
+		if kindCluster == c.Name {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+func (c *KindCluster) initProvider() {
+	if c.provider == nil {
+		c.provider = cluster.NewProvider()
+	}
+}
+
 // Create the kind cluster
 func (c *KindCluster) Create() error {
-	c.provider = cluster.NewProvider()
-
+	c.initProvider()
 	version, err := asKindVersion(c.KubernetesVersion)
 	if err != nil {
 		return err
@@ -89,6 +109,7 @@ func (c *KindCluster) Create() error {
 
 // Delete the kind cluster
 func (c *KindCluster) Delete() error {
+	c.initProvider()
 	if !c.creationSuccessful {
 		// Since we have set retain=true, the cluster is still available even
 		// though creation did not execute successfully.
@@ -114,6 +135,7 @@ func (c *KindCluster) Delete() error {
 
 // Connect to the kind cluster
 func (c *KindCluster) Connect() error {
+	c.initProvider()
 	return c.provider.ExportKubeConfig(c.Name, c.KubeConfigPath, false)
 }
 

e2e/nomostest/new.go

@@ -286,20 +286,28 @@ func FreshTestEnv(t nomostesting.NTB, opts *ntopts.New) *NT {
 		// a single test.
 		connectToLocalRegistry(nt)
 	}
-	if !opts.IsEphemeralCluster {
-		// We aren't using an ephemeral cluster, so make sure the cluster is
-		// cleaned before and after running the test.
+	t.Cleanup(func() {
+		if *e2e.DestroyClusters {
+			nt.T.Logf("Skipping cleanup because cluster will be destroyed (--destroy-clusters=true)")
+			return
+		} else if t.Failed() && *e2e.Debug {
+			nt.T.Logf("Skipping cleanup because test failed with --debug")
+			return
+		}
+		// We aren't deleting the cluster after the test, so clean up
+		nt.T.Log("[CLEANUP] FreshTestEnv after test")
+		if err := Clean(nt); err != nil {
+			nt.T.Errorf("[CLEANUP] Failed to clean test environment: %v", err)
+		}
+	})
+	if *e2e.CreateClusters != e2e.CreateClustersEnabled {
+		// We may not be using a fresh cluster, so make sure the cluster is
+		// cleaned before running the test.
 		nt.T.Log("[CLEANUP] FreshTestEnv before test")
 		if err := Clean(nt); err != nil {
 			nt.T.Fatalf("[CLEANUP] Failed to clean test environment: %v", err)
 		}
-		t.Cleanup(func() {
-			// Clean the cluster now that the test is over.
-			nt.T.Log("[CLEANUP] FreshTestEnv after test")
-			if err := Clean(nt); err != nil {
-				nt.T.Errorf("[CLEANUP] Failed to clean test environment: %v", err)
-			}
-		})
+
 	}
 
 	t.Cleanup(func() {

e2e/nomostest/nt.go

@@ -313,15 +313,21 @@ func InitSharedEnvironments() error {
 	}
 	timeStamp := time.Now().Unix()
 	tg := taskgroup.New()
-	for x := 0; x < e2e.NumParallel(); x++ {
-		name := fmt.Sprintf("cs-e2e-%v-%v", timeStamp, x)
+	clusters := *e2e.ClusterNames
+	if len(clusters) == 0 { // generate names for ephemeral clusters
+		for x := 0; x < e2e.NumParallel(); x++ {
+			clusters = append(clusters, fmt.Sprintf("cs-e2e-%v-%v", timeStamp, x))
+		}
+	}
+	for _, name := range clusters {
+		clusterName := name
 		tg.Go(func() (err error) {
 			defer func() {
 				if recoverErr := recover(); recoverErr != nil {
-					err = errors.Errorf("recovered from panic in InitSharedEnvironments (%s): %v", name, recoverErr)
+					err = errors.Errorf("recovered from panic in InitSharedEnvironments (%s): %v", clusterName, recoverErr)
 				}
 			}()
-			return newSharedNT(name)
+			return newSharedNT(clusterName)
 		})
 	}
 	return tg.Wait()

e2e/nomostest/ntopts/new.go

@@ -63,10 +63,6 @@ type New struct {
 	// TestFeature is the feature that the test verifies
 	TestFeature testing.Feature
 
-	// IsEphemeralCluster indicates whether the cluster is ephemeral, i.e. created
-	// and destroyed in the scope of test execution.
-	IsEphemeralCluster bool
-
 	Nomos
 	MultiRepo
 	TestType