Refactor Kubernetes E2E test definition for ease of use (#9581) (#9640)

* make tests runnable with injection

* yoink var into better package

* refactor for un/ordered, convert the rest

* fix a missed merge conflict

* add changelog

* oops wrong skip

* rename and refactor

* clean up and compiler-assert new suite signatures

* documentation

* documentation

* fix my runs

* wrap un/ordered in struct for ease of use

* Adding changelog file to new location

* Deleting changelog file from old location

* move changelog

* Update test/kubernetes/e2e/tests/glooctl_istio_inject_tests.go

* refactor to export less

* move and rename suite runner

* update readme

* rename some more stuff

---------

Co-authored-by: Jacob Bohanon <jacob.bohanon@solo.io>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>

changelog/v1.17.0-rc5/e2e-tests-cleanup.yaml

@@ -0,0 +1,8 @@
+changelog:
+  - type: NON_USER_FACING
+    issueLink: https://github.com/solo-io/gloo/issues/9353
+    resolvesIssue: false
+    description: >-
+      Make Kubernetes E2E tests easily importable and runnable from enterprise suites
+
+      skipCI-docs-build:true

test/kubernetes/e2e/README.md

@@ -18,11 +18,21 @@ We define all tests in the [features](./features) package. This is done for a va
 1. We group the tests by feature, so it's easy to identify which behaviors we assert for a given feature.
 2. We can invoke that same test against different `TestInstallation`s. This means we can test a feature against a variety of installation values, or even against OSS and Enterprise installations.
 
+## Test Suites
+A Test Suite is a subset of the Feature concept. A single Feature has at minimum one Test Suite, and can have many. Each Test Suite within the feature must have a function which satisfies the signature `NewSuiteFunc` found in [suite.go](./suite.go).
+
+These test suites are registered by a name and this func in [Tests](#tests) to be run against various `TestInstallation`s.
+
 ## Tests
 This package holds the entry point for each of our `TestInstallation`. 
 
 See [Load balancing tests](./load_balancing_tests.md) for more information about how these tests are run in CI.
 
+Each `*_test.go` file contains a specific test installation and exists within the `tests_test` package. In order for tests to be imported and run from other repos, each `*_test.go` file has a corresponding `*_test.go` file which exists in the `tests` package. This is done because `_test` packages cannot be imported.
+
+In order to add a feature suite to be run in a given test installation, it must be added to the exported function in the corresponding `*_tests.go` file.
+e.g. In order to add a feature suite to be run with the test installation defined in `istio_test.go`, we have to register it by adding it to `IstioTests()` in `istio_tests.go` following the existing paradigm.
+
 ## Environment Variables
 
 Some tests may require environment variables to be set. Some required env vars are:
@@ -50,4 +60,4 @@ Below are a set of known areas of improvement. The goal is to provide a starting
 - **Improved install action(s)**: We rely on the [SoloTestHelper](/test/kube2e/helper/install.go) currently, and it would be nice if we relied directly on Helm or Glooctl.
 - **Cluster provisioning**: We rely on the [setup-kind](/ci/kind/setup-kind.sh) script to provision a cluster. We should make this more flexible by providing a configurable, declarative way to do this.
 - **Istio action**: We need a way to perform Istio actions against a cluster.
-- **Argo action**: We need an easy utility to perform ArgoCD commands against a cluster.
+- **Argo action**: We need an easy utility to perform ArgoCD commands against a cluster.

test/kubernetes/e2e/features/deployer/istio_integration_deployer_suite.go

@@ -13,6 +13,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
 )
 
+var _ e2e.NewSuiteFunc = NewIstioIntegrationTestingSuite
+
 // istioIntegrationDeployerSuite is the entire Suite of tests for the "deployer" feature that relies on an Istio installation
 // The "deployer" code can be found here: /projects/gateway2/deployer
 type istioIntegrationDeployerSuite struct {

test/kubernetes/e2e/features/deployer/suite.go

@@ -21,6 +21,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/testutils/runtime"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "deployer" feature
 // The "deployer" code can be found here: /projects/gateway2/deployer
 type testingSuite struct {

test/kubernetes/e2e/features/example/suite.go

@@ -9,6 +9,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "example" feature
 // Typically, we would include a link to the feature code here
 type testingSuite struct {

test/kubernetes/e2e/features/glooctl/check_suite.go

@@ -10,6 +10,8 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
+var _ e2e.NewSuiteFunc = NewCheckSuite
+
 // checkSuite contains the set of tests to validate the behavior of `glooctl check`
 // These tests attempt to mirror: https://github.com/solo-io/gloo/blob/v1.16.x/test/kube2e/glooctl/check_test.go
 type checkSuite struct {

test/kubernetes/e2e/features/glooctl/debug_suite.go

@@ -9,6 +9,8 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
+var _ e2e.NewSuiteFunc = NewDebugSuite
+
 // debugSuite contains the set of tests to validate the behavior of `glooctl debug`
 // These tests attempt to mirror: https://github.com/solo-io/gloo/blob/v1.16.x/test/kube2e/glooctl/debug_test.go
 type debugSuite struct {

test/kubernetes/e2e/features/glooctl/get_proxy_suite.go

@@ -19,6 +19,8 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
+var _ e2e.NewSuiteFunc = NewGetProxySuite
+
 var (
 	yamlSeparator = regexp.MustCompile("\n---\n")
 )

test/kubernetes/e2e/features/glooctl/istio_inject_suite.go

@@ -13,6 +13,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewIstioInjectTestingSuite
+
 // istioInjectTestingSuite is the entire Suite of tests for the "glooctl istio inject" integration cases
 // NOTE: This suite is not intended to be run as a standalone test suite. It applies the "glooctl istio inject" command
 // to an existing installation of Gloo Gateway and verifies that the necessary resources are created, but does not clean

test/kubernetes/e2e/features/glooctl/istio_uninject_suite.go

@@ -13,6 +13,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewIstioUninjectTestingSuite
+
 // istioUninjectTestingSuite is the entire Suite of tests for the "glooctl istio uninject" integration cases
 // NOTE: This suite is not intended to be run as a standalone test suite. It applies the "glooctl unistio inject" command
 // to an existing installation of Gloo Gateway where the istio-proxy and sds containers have already been injected

test/kubernetes/e2e/features/headless_svc/gloo_gateway_suite.go

@@ -13,6 +13,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewEdgeGatewayHeadlessSvcSuite
+
 type edgeGatewaySuite struct {
 	suite.Suite
 

test/kubernetes/e2e/features/headless_svc/k8s_gw_suite.go

@@ -13,6 +13,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/testutils/resources"
 )
 
+var _ e2e.NewSuiteFunc = NewK8sGatewayHeadlessSvcSuite
+
 type k8sGatewaySuite struct {
 	suite.Suite
 

test/kubernetes/e2e/features/http_listener_options/http_lis_opt_suite.go

@@ -15,6 +15,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "HttpListenerOptions" feature
 type testingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/istio/gloo_gateway_auto_mtls_suite.go

@@ -16,6 +16,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewGlooIstioAutoMtlsSuite
+
 // glooIstioAutoMtlsTestingSuite is the entire Suite of tests for the "Istio" integration cases where auto mTLS is enabled
 type glooIstioAutoMtlsTestingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/istio/gloo_gateway_no_auto_mtls_suite.go

@@ -16,6 +16,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewGlooTestingSuite
+
 // glooIstioTestingSuite is the entire Suite of tests for the "Istio" integration cases where auto mtls is disabled
 // and Upstreams do not have sslConfig values set
 type glooIstioTestingSuite struct {

test/kubernetes/e2e/features/istio/k8s_gateway_auto_mtls_suite.go

@@ -12,6 +12,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
 )
 
+var _ e2e.NewSuiteFunc = NewIstioAutoMtlsSuite
+
 // istioMtlsTestingSuite is the entire Suite of tests for the "Istio" integration cases where auto mTLS is enabled
 type istioAutoMtlsTestingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/istio/k8s_gateway_no_auto_mtls_suite.go

@@ -11,6 +11,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // istioTestingSuite is the entire Suite of tests for the "Istio" integration cases where auto mtls is disabled
 // and Upstreams do not have sslConfig values set
 type istioTestingSuite struct {

test/kubernetes/e2e/features/listener_options/lis_opt_suite.go

@@ -15,6 +15,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "ListenerOptions" feature
 type testingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/port_routing/suite.go

@@ -11,6 +11,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // portRoutingTestingSuite is the entire Suite of tests for the "PortRouting" cases
 type portRoutingTestingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/route_delegation/suite.go

@@ -19,6 +19,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/testutils/gloogateway"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 type tsuite struct {
 	suite.Suite
 
@@ -33,7 +35,7 @@ type tsuite struct {
 	manifestObjects map[string][]client.Object
 }
 
-func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) *tsuite {
+func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.TestingSuite {
 	return &tsuite{
 		ctx: ctx,
 		ti:  testInst,

test/kubernetes/e2e/features/route_options/suite.go

@@ -19,6 +19,8 @@ import (
 	testdefaults "github.com/solo-io/gloo/test/kubernetes/e2e/defaults"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "Route Options" feature
 type testingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/upstreams/suite.go

@@ -14,6 +14,8 @@ import (
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "Upstream" feature
 type testingSuite struct {
 	suite.Suite

test/kubernetes/e2e/features/virtualhost_options/vhost_opt_suite.go

@@ -18,6 +18,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+var _ e2e.NewSuiteFunc = NewTestingSuite
+
 // testingSuite is the entire Suite of tests for the "VirtualHostOptions" feature
 type testingSuite struct {
 	suite.Suite

test/kubernetes/e2e/suite.go

@@ -0,0 +1,87 @@
+package e2e
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type (
+	NewSuiteFunc func(ctx context.Context, testInstallation *TestInstallation) suite.TestingSuite
+
+	namedSuite struct {
+		name     string
+		newSuite NewSuiteFunc
+	}
+
+	orderedSuites struct {
+		suites []namedSuite
+	}
+
+	suites struct {
+		suites map[string]NewSuiteFunc
+	}
+
+	// A SuiteRunner is an interface that allows E2E tests to simply Register tests in one location and execute them
+	// with Run.
+	SuiteRunner interface {
+		Run(ctx context.Context, t *testing.T, testInstallation *TestInstallation)
+		Register(name string, newSuite NewSuiteFunc)
+	}
+)
+
+var (
+	_ SuiteRunner = new(orderedSuites)
+	_ SuiteRunner = new(suites)
+)
+
+// NewSuiteRunner returns an implementation of TestRunner that will execute tests as specified
+// in the ordered parameter.
+//
+// NOTE: it should be strongly preferred to use unordered tests. Only pass true to this function
+// if there is a clear need for the tests to be ordered, and specify in a comment near the call
+// to NewSuiteRunner why the tests need to be ordered.
+func NewSuiteRunner(ordered bool) SuiteRunner {
+	if ordered {
+		return new(orderedSuites)
+	}
+
+	return new(suites)
+}
+
+func (o orderedSuites) Run(ctx context.Context, t *testing.T, testInstallation *TestInstallation) {
+	for _, namedTest := range o.suites {
+		t.Run(namedTest.name, func(t *testing.T) {
+			suite.Run(t, namedTest.newSuite(ctx, testInstallation))
+		})
+	}
+}
+
+func (o *orderedSuites) Register(name string, newSuite NewSuiteFunc) {
+	if o.suites == nil {
+		o.suites = make([]namedSuite, 0)
+	}
+	o.suites = append(o.suites, namedSuite{
+		name:     name,
+		newSuite: newSuite,
+	})
+
+}
+
+func (u suites) Run(ctx context.Context, t *testing.T, testInstallation *TestInstallation) {
+	// TODO(jbohanon) does some randomness need to be injected here to ensure they aren't run in the same order every time?
+	// from https://goplay.tools/snippet/A-qqQCWkFaZ it looks like maps are not stable, but tend toward stability.
+	for testName, newSuite := range u.suites {
+		t.Run(testName, func(t *testing.T) {
+			suite.Run(t, newSuite(ctx, testInstallation))
+		})
+	}
+}
+
+func (u *suites) Register(name string, newSuite NewSuiteFunc) {
+	if u.suites == nil {
+		u.suites = make(map[string]NewSuiteFunc)
+	}
+	u.suites[name] = newSuite
+}

test/kubernetes/e2e/tests/automtls_istio_edge_api_test.go

@@ -7,13 +7,11 @@ import (
 	"time"
 
 	"github.com/solo-io/gloo/test/kube2e/helper"
-	"github.com/solo-io/gloo/test/kubernetes/e2e/features/headless_svc"
-	"github.com/solo-io/gloo/test/kubernetes/e2e/features/istio"
 
 	"github.com/solo-io/skv2/codegen/util"
-	"github.com/stretchr/testify/suite"
 
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
+	. "github.com/solo-io/gloo/test/kubernetes/e2e/tests"
 	"github.com/solo-io/gloo/test/kubernetes/testutils/gloogateway"
 )
 
@@ -67,11 +65,5 @@ func TestAutomtlsIstioEdgeApisGateway(t *testing.T) {
 		return testHelper.InstallGloo(ctx, helper.GATEWAY, 5*time.Minute, helper.ExtraArgs("--values", testInstallation.Metadata.ValuesManifestFile))
 	})
 
-	t.Run("HeadlessSvc", func(t *testing.T) {
-		suite.Run(t, headless_svc.NewEdgeGatewayHeadlessSvcSuite(ctx, testInstallation))
-	})
-
-	t.Run("IstioIntegration", func(t *testing.T) {
-		suite.Run(t, istio.NewGlooIstioAutoMtlsSuite(ctx, testInstallation))
-	})
+	AutomtlsIstioEdgeApiSuiteRunner().Run(ctx, t, testInstallation)
 }

test/kubernetes/e2e/tests/automtls_istio_edge_api_tests.go

@@ -0,0 +1,16 @@
+package tests
+
+import (
+	"github.com/solo-io/gloo/test/kubernetes/e2e"
+	"github.com/solo-io/gloo/test/kubernetes/e2e/features/headless_svc"
+	"github.com/solo-io/gloo/test/kubernetes/e2e/features/istio"
+)
+
+func AutomtlsIstioEdgeApiSuiteRunner() e2e.SuiteRunner {
+	automtlsIstioEdgeApiSuiteRunner := e2e.NewSuiteRunner(false)
+
+	automtlsIstioEdgeApiSuiteRunner.Register("HeadlessSvc", headless_svc.NewEdgeGatewayHeadlessSvcSuite)
+	automtlsIstioEdgeApiSuiteRunner.Register("IstioIntegrationAutoMtls", istio.NewGlooIstioAutoMtlsSuite)
+
+	return automtlsIstioEdgeApiSuiteRunner
+}

test/kubernetes/e2e/tests/automtls_istio_test.go

@@ -9,12 +9,9 @@ import (
 
 	"github.com/solo-io/gloo/test/kube2e/helper"
 	"github.com/solo-io/gloo/test/kubernetes/e2e"
-	"github.com/solo-io/gloo/test/kubernetes/e2e/features/headless_svc"
-	"github.com/solo-io/gloo/test/kubernetes/e2e/features/istio"
-	"github.com/solo-io/gloo/test/kubernetes/e2e/features/port_routing"
+	. "github.com/solo-io/gloo/test/kubernetes/e2e/tests"
 	"github.com/solo-io/gloo/test/kubernetes/testutils/gloogateway"
 	"github.com/solo-io/skv2/codegen/util"
-	"github.com/stretchr/testify/suite"
 )
 
 // TestK8sGatewayIstioAutoMtls is the function which executes a series of tests against a given installation
@@ -70,15 +67,5 @@ func TestK8sGatewayIstioAutoMtls(t *testing.T) {
 		return testHelper.InstallGloo(ctx, helper.GATEWAY, 10*time.Minute, helper.ExtraArgs("--values", testInstallation.Metadata.ValuesManifestFile))
 	})
 
-	t.Run("PortRouting", func(t *testing.T) {
-		suite.Run(t, port_routing.NewTestingSuite(ctx, testInstallation))
-	})
-
-	t.Run("HeadlessSvc", func(t *testing.T) {
-		suite.Run(t, headless_svc.NewK8sGatewayHeadlessSvcSuite(ctx, testInstallation))
-	})
-
-	t.Run("IstioIntegrationAutoMtls", func(t *testing.T) {
-		suite.Run(t, istio.NewIstioAutoMtlsSuite(ctx, testInstallation))
-	})
+	AutomtlsIstioSuiteRunner().Run(ctx, t, testInstallation)
 }