Bug 2080379: Group all e2e tests as parallel or serial

Makefile

@@ -12,7 +12,7 @@ endif
 GO=GO111MODULE=on GOFLAGS=-mod=vendor go
 GO_BUILD_RECIPE=CGO_ENABLED=0 $(GO) build -o $(BIN) $(GO_GCFLAGS) $(MAIN_PACKAGE)
 
-TEST ?= .*
+TEST ?= TestAll
 
 .PHONY: build
 build:
@@ -55,6 +55,10 @@ release-local:
 test-e2e:
 	$(GO) test -timeout 1h -count 1 -v -tags e2e -run "$(TEST)" ./test/e2e
 
+.PHONY: test-e2e-list
+test-e2e-list:
+	@(cd ./test/e2e; E2E_TEST_MAIN_SKIP_SETUP=1 $(GO) test -list . -tags e2e | grep ^Test | sort)
+
 .PHONY: clean
 clean:
 	$(GO) clean
@@ -67,6 +71,7 @@ verify:
 	hack/verify-profile-manifests.sh
 	hack/verify-generated-bindata.sh
 	hack/verify-deps.sh
+	hack/verify-e2e-test-all-presence.sh
 
 .PHONY: uninstall
 uninstall:

hack/verify-e2e-test-all-presence.sh

@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+
+# This script verifies that all the e2e tests defined in package
+# test/e2e have a corresponding invocation in the TestAll function
+# defined in test/e2e/all_test.go.
+#
+# The TestAll function provides ordering of all e2e tests. Tests that
+# can be run in parallel are invoked first and will run to completion
+# before starting those that must run serially.
+#
+# The CI job runs `make verify` before starting the e2e tests and the
+# verify target will run this script. If this script detects any Go
+# Test function by name that is not invoked by the TestAll function
+# then it will list the omission and exit with an error, preventing
+# the e2e tests from starting.
+
+# This script has been tested on Linux and macOS.
+
+set -u
+set -o pipefail
+
+thisdir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)"
+e2e_test_dir="$thisdir/../test/e2e"
+e2e_test_file="$e2e_test_dir/all_test.go"
+
+if ! [[ -f ${e2e_test_file} ]]; then
+    echo "error: $e2e_test_file is missing." >&2
+    exit 1
+fi
+
+# "go test -list" must run in the directory where the test files are.
+pushd "$e2e_test_dir" >/dev/null || {
+    echo "error: pushd $e2e_test_dir failed" >&2;
+    exit 2                      # ENOENT
+}
+
+go_test_list_output=$(E2E_TEST_MAIN_SKIP_SETUP=1 go test -list . -tags e2e)
+go_test_list_status=$?
+
+if [[ $go_test_list_status -ne 0 ]]; then
+    echo "error: go test -list failed" >&2
+    echo "$go_test_list_output" >&2
+    exit $go_test_list_status
+fi
+
+popd >/dev/null || {
+    echo "error: popd failed" >&2;
+    exit 1
+}
+
+errors=0
+
+for i in $go_test_list_output; do
+    if ! [[ $i =~ ^Test ]] || [[ $i == "TestAll" ]]; then
+        continue
+    fi
+    pattern="^	\+t\.Run(\"$i\", $i)$" # ^multiple<TAB>s.
+    if ! grep -q -e "$pattern" -- "$e2e_test_file"; then
+        echo "error: test function '$i' not called by 'TestAll' in $e2e_test_file" >&2
+        errors=1
+    fi
+done
+
+exit $errors

test/e2e/all_test.go

@@ -0,0 +1,95 @@
+//go:build e2e
+// +build e2e
+
+package e2e
+
+import "testing"
+
+// TestAll is the entrypoint for `make test-e2e` unless you override
+// with: make TEST=Test<foo> test-e2e.
+//
+// The overriding goal of this test is to run as many tests in
+// parallel as possible before running those tests that must run serially. There
+// are two goals: 1) cut down on test execution time and 2) provide
+// explicit ordering for tests that do not expect a rolling update of
+// ingresscontroller pods because a previous test modified the
+// ingressconfig object and the defer logic for cleanup is still
+// runnng when the new test starts.
+func TestAll(t *testing.T) {
+	// This call to Run() will not return until all of its
+	// parallel subtests complete. Each "parallel" test must
+	// invoke t.Parallel().
+	t.Run("parallel", func(t *testing.T) {
+		t.Run("TestAWSELBConnectionIdleTimeout", TestAWSELBConnectionIdleTimeout)
+		t.Run("TestClientTLS", TestClientTLS)
+		t.Run("TestContainerLogging", TestContainerLogging)
+		t.Run("TestCreateIngressControllerThenSecret", TestCreateIngressControllerThenSecret)
+		t.Run("TestCreateSecretThenIngressController", TestCreateSecretThenIngressController)
+		t.Run("TestCustomErrorpages", TestCustomErrorpages)
+		t.Run("TestCustomIngressClass", TestCustomIngressClass)
+		t.Run("TestDynamicConfigManagerUnsupportedConfigOverride", TestDynamicConfigManagerUnsupportedConfigOverride)
+		t.Run("TestForwardedHeaderPolicyAppend", TestForwardedHeaderPolicyAppend)
+		t.Run("TestForwardedHeaderPolicyIfNone", TestForwardedHeaderPolicyIfNone)
+		t.Run("TestForwardedHeaderPolicyNever", TestForwardedHeaderPolicyNever)
+		t.Run("TestForwardedHeaderPolicyReplace", TestForwardedHeaderPolicyReplace)
+		t.Run("TestHAProxyTimeouts", TestHAProxyTimeouts)
+		t.Run("TestHAProxyTimeoutsRejection", TestHAProxyTimeoutsRejection)
+		t.Run("TestHTTPCookieCapture", TestHTTPCookieCapture)
+		t.Run("TestHTTPHeaderBufferSize", TestHTTPHeaderBufferSize)
+		t.Run("TestHTTPHeaderCapture", TestHTTPHeaderCapture)
+		t.Run("TestHeaderNameCaseAdjustment", TestHeaderNameCaseAdjustment)
+		t.Run("TestHealthCheckIntervalIngressController", TestHealthCheckIntervalIngressController)
+		t.Run("TestHostNetworkEndpointPublishingStrategy", TestHostNetworkEndpointPublishingStrategy)
+		t.Run("TestIngressControllerScale", TestIngressControllerScale)
+		t.Run("TestIngressControllerServiceNameCollision", TestIngressControllerServiceNameCollision)
+		t.Run("TestInternalLoadBalancer", TestInternalLoadBalancer)
+		t.Run("TestInternalLoadBalancerGlobalAccessGCP", TestInternalLoadBalancerGlobalAccessGCP)
+		t.Run("TestLoadBalancingAlgorithmUnsupportedConfigOverride", TestLoadBalancingAlgorithmUnsupportedConfigOverride)
+		t.Run("TestLocalWithFallbackOverrideForNodePortService", TestLocalWithFallbackOverrideForNodePortService)
+		t.Run("TestNetworkLoadBalancer", TestNetworkLoadBalancer)
+		t.Run("TestNodePortServiceEndpointPublishingStrategy", TestNodePortServiceEndpointPublishingStrategy)
+		t.Run("TestProxyProtocolAPI", TestProxyProtocolAPI)
+		t.Run("TestReloadIntervalUnsupportedConfigOverride", TestReloadIntervalUnsupportedConfigOverride)
+		t.Run("TestRouteAdmissionPolicy", TestRouteAdmissionPolicy)
+		t.Run("TestRouterCompressionParsing", TestRouterCompressionParsing)
+		t.Run("TestScopeChange", TestScopeChange)
+		t.Run("TestSyslogLogging", TestSyslogLogging)
+		t.Run("TestTLSSecurityProfile", TestTLSSecurityProfile)
+		t.Run("TestTunableMaxConnectionsInvalidValues", TestTunableMaxConnectionsInvalidValues)
+		t.Run("TestTunableMaxConnectionsValidValues", TestTunableMaxConnectionsValidValues)
+		t.Run("TestTunableRouterKubeletProbesForCustomIngressController", TestTunableRouterKubeletProbesForCustomIngressController)
+		t.Run("TestUniqueDomainRejection", TestUniqueDomainRejection)
+		t.Run("TestUniqueIdHeader", TestUniqueIdHeader)
+		t.Run("TestUserDefinedIngressController", TestUserDefinedIngressController)
+	})
+
+	t.Run("serial", func(t *testing.T) {
+		t.Run("TestDefaultIngressControllerSteadyConditions", TestDefaultIngressControllerSteadyConditions)
+		t.Run("TestClusterOperatorStatusRelatedObjects", TestClusterOperatorStatusRelatedObjects)
+		t.Run("TestConfigurableRouteNoConsumingUserNoRBAC", TestConfigurableRouteNoConsumingUserNoRBAC)
+		t.Run("TestConfigurableRouteNoSecretNoRBAC", TestConfigurableRouteNoSecretNoRBAC)
+		t.Run("TestConfigurableRouteRBAC", TestConfigurableRouteRBAC)
+		t.Run("TestDefaultIngressCertificate", TestDefaultIngressCertificate)
+		t.Run("TestDefaultIngressClass", TestDefaultIngressClass)
+		t.Run("TestHstsPolicyWorks", TestHstsPolicyWorks)
+		t.Run("TestIngressControllerCustomEndpoints", TestIngressControllerCustomEndpoints)
+		t.Run("TestIngressStatus", TestIngressStatus)
+		t.Run("TestLocalWithFallbackOverrideForLoadBalancerService", TestLocalWithFallbackOverrideForLoadBalancerService)
+		t.Run("TestOperatorSteadyConditions", TestOperatorSteadyConditions)
+		t.Run("TestPodDisruptionBudgetExists", TestPodDisruptionBudgetExists)
+		t.Run("TestProxyProtocolOnAWS", TestProxyProtocolOnAWS)
+		t.Run("TestRouteHTTP2EnableAndDisableIngressController", TestRouteHTTP2EnableAndDisableIngressController)
+		t.Run("TestRouteHardStopAfterEnableOnIngressController", TestRouteHardStopAfterEnableOnIngressController)
+		t.Run("TestRouteHardStopAfterTestInvalidDuration", TestRouteHardStopAfterTestInvalidDuration)
+		t.Run("TestRouteHardStopAfterTestOneDayDuration", TestRouteHardStopAfterTestOneDayDuration)
+		t.Run("TestRouteHardStopAfterTestZeroLengthDuration", TestRouteHardStopAfterTestZeroLengthDuration)
+		t.Run("TestRouteNbthreadIngressController", TestRouteNbthreadIngressController)
+		t.Run("TestRouterCompressionOperation", TestRouterCompressionOperation)
+		t.Run("TestUpdateDefaultIngressController", TestUpdateDefaultIngressController)
+		t.Run("TestCanaryRoute", TestCanaryRoute)
+		t.Run("TestRouteHTTP2EnableAndDisableIngressConfig", TestRouteHTTP2EnableAndDisableIngressConfig)
+		t.Run("TestRouteHardStopAfterEnableOnIngressConfig", TestRouteHardStopAfterEnableOnIngressConfig)
+		t.Run("TestRouteHardStopAfterEnableOnIngressControllerHasPriorityOverIngressConfig", TestRouteHardStopAfterEnableOnIngressControllerHasPriorityOverIngressConfig)
+		t.Run("TestHostNetworkPortBinding", TestHostNetworkPortBinding)
+	})
+}

test/e2e/canary_test.go

@@ -6,14 +6,10 @@ package e2e
 import (
 	"bufio"
 	"context"
-	"fmt"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
-
 	operatorv1 "github.com/openshift/api/operator/v1"
 	routev1 "github.com/openshift/api/route/v1"
 
@@ -152,84 +148,3 @@ func buildCanaryCurlPod(name, namespace, image, host string) *corev1.Pod {
 		},
 	}
 }
-
-// TestCanaryRouteRotationAnnotation verifies that the
-// canary controller respects the canary route rotation
-// annotation for the default ingress controller.
-//
-// Note that this test will mutate the default ingress controller
-func TestCanaryRouteRotationAnnotation(t *testing.T) {
-	// Set the CanaryRouteRotation annotation to true
-	// on the default ingress controller.
-	if err := setDefaultIngressControllerRotationAnnotation(t, true); err != nil {
-		t.Fatalf("failed to set canary route rotation annotation: %v", err)
-	}
-
-	// Cleanup default ingress controller by setting the canary rotation
-	// annotation to false (and verifying that we were successful in doing so).
-	defer func() {
-		if err := setDefaultIngressControllerRotationAnnotation(t, false); err != nil {
-			t.Fatalf("failed to set canary route rotation annotation: %v", err)
-		}
-	}()
-
-	// Get canary route.
-	canaryRoute := &routev1.Route{}
-	name := controller.CanaryRouteName()
-	err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
-		if err := kclient.Get(context.TODO(), name, canaryRoute); err != nil {
-			t.Logf("failed to get canary route %s: %v", name, err)
-			return false, nil
-		}
-		return true, nil
-	})
-
-	if err != nil {
-		t.Fatalf("failed to observe canary route: %v", err)
-	}
-
-	// The canary controller should update the canary route after 5 successful
-	// canary checks. Canary checks happen once a minute.
-	updatedCanaryRoute := &routev1.Route{}
-	err = wait.PollImmediate(5*time.Second, 10*time.Minute, func() (bool, error) {
-		if err := kclient.Get(context.TODO(), name, updatedCanaryRoute); err != nil {
-			t.Logf("failed to get canary route %s: %v", name, err)
-			return false, nil
-		}
-		// If the canaryRoute and updatedCanaryRoute do not have the same targetPort,
-		// then the canary route rotation annotation is working.
-		if cmp.Equal(canaryRoute.Spec.Port.TargetPort, updatedCanaryRoute.Spec.Port.TargetPort) {
-			return false, nil
-		}
-
-		return true, nil
-	})
-
-	if err != nil {
-		t.Fatalf("failed to observe canary route rotation: %v", err)
-	}
-}
-
-func setDefaultIngressControllerRotationAnnotation(t *testing.T, val bool) error {
-	t.Helper()
-	ic := &operatorv1.IngressController{}
-	if err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
-		if err := kclient.Get(context.TODO(), defaultName, ic); err != nil {
-			t.Logf("Get failed: %v, retrying...", err)
-			return false, nil
-		}
-		if ic.Annotations == nil {
-			ic.Annotations = map[string]string{}
-		}
-		ic.Annotations[canarycontroller.CanaryRouteRotationAnnotation] = strconv.FormatBool(val)
-		if err := kclient.Update(context.TODO(), ic); err != nil {
-			t.Logf("failed to update ingress controller: %v", err)
-			return false, nil
-		}
-		return true, nil
-	}); err != nil {
-		return fmt.Errorf("failed to update ingress controller: %v", err)
-	}
-
-	return nil
-}

test/e2e/certificate_publisher_test.go

@@ -24,6 +24,7 @@ import (
 // references a secret that does not exist, then creates the secret and verifies
 // that the operator updates the "router-certs" global secret.
 func TestCreateIngressControllerThenSecret(t *testing.T) {
+	t.Parallel()
 	name := types.NamespacedName{Namespace: operatorNamespace, Name: names.SimpleNameGenerator.GenerateName("test-")}
 	ic := newPrivateController(name, name.Name+"."+dnsConfig.Spec.BaseDomain)
 	ic.Spec.DefaultCertificate = &corev1.LocalObjectReference{
@@ -73,6 +74,7 @@ func TestCreateIngressControllerThenSecret(t *testing.T) {
 // ingresscontroller that references the secret and verifies that the operator
 // updates the "router-certs" global secret.
 func TestCreateSecretThenIngressController(t *testing.T) {
+	t.Parallel()
 	name := types.NamespacedName{Namespace: operatorNamespace, Name: names.SimpleNameGenerator.GenerateName("test-")}
 
 	// Create the secret.

test/e2e/checkinterval_test.go

@@ -24,6 +24,7 @@ const (
 )
 
 func TestHealthCheckIntervalIngressController(t *testing.T) {
+	t.Parallel()
 	name := types.NamespacedName{Namespace: operatorNamespace, Name: "healthcheckinterval"}
 	domain := name.Name + "." + dnsConfig.Spec.BaseDomain
 	ic := newPrivateController(name, domain)

test/e2e/client_tls_test.go

@@ -55,6 +55,7 @@ import (
 // accepts connections with a valid, matching certificate and rejects other
 // connections.
 func TestClientTLS(t *testing.T) {
+	t.Parallel()
 	// We will configure the ingresscontroller to recognize certificates
 	// signed by this CA.
 	ca, caKey, err := generateClientCA()

test/e2e/forwarded_header_policy_test.go

@@ -52,7 +52,7 @@ func testRouteHeaders(t *testing.T, image string, route *routev1.Route, address
 		extraCurlArgs = append(extraCurlArgs, "-H", header)
 	}
 	testPodCount++
-	name := fmt.Sprintf("forwardedheader%d", testPodCount)
+	name := fmt.Sprintf("%s%d", route.Name, testPodCount)
 	clientPod := buildCurlPod(name, route.Namespace, image, route.Spec.Host, address, extraCurlArgs...)
 	if err := kclient.Create(context.TODO(), clientPod); err != nil {
 		t.Fatalf("failed to create pod %s/%s: %v", clientPod.Namespace, clientPod.Name, err)
@@ -121,7 +121,8 @@ func testRouteHeaders(t *testing.T, image string, route *routev1.Route, address
 // client specifies 2 X-Forwarded-For headers, then the router should append a
 // 3rd.
 func TestForwardedHeaderPolicyAppend(t *testing.T) {
-	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader"}
+	t.Parallel()
+	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader-append"}
 	domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
 	ic := newPrivateController(icName, domain)
 	if err := kclient.Create(context.TODO(), ic); err != nil {
@@ -215,7 +216,8 @@ func TestForwardedHeaderPolicyAppend(t *testing.T) {
 // expected behavior if its policy is "Replace".  A forwarded client request
 // should always have exactly 1 X-Forwarded-For header.
 func TestForwardedHeaderPolicyReplace(t *testing.T) {
-	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader"}
+	t.Parallel()
+	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader-replace"}
 	domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
 	ic := newPrivateController(icName, domain)
 	ic.Spec.HTTPHeaders = &operatorv1.IngressControllerHTTPHeaders{
@@ -284,7 +286,8 @@ func TestForwardedHeaderPolicyReplace(t *testing.T) {
 // should always have exactly as many X-Forwarded-For headers as the client
 // specified.
 func TestForwardedHeaderPolicyNever(t *testing.T) {
-	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader"}
+	t.Parallel()
+	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader-never"}
 	domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
 	ic := newPrivateController(icName, domain)
 	ic.Spec.HTTPHeaders = &operatorv1.IngressControllerHTTPHeaders{
@@ -354,7 +357,8 @@ func TestForwardedHeaderPolicyNever(t *testing.T) {
 // specifies more than 1 X-Forwarded-For header, the forwarded request should
 // include exactly as many X-Forwarded-For headers as the client specified.
 func TestForwardedHeaderPolicyIfNone(t *testing.T) {
-	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader"}
+	t.Parallel()
+	icName := types.NamespacedName{Namespace: operatorNamespace, Name: "forwardedheader-none"}
 	domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
 	ic := newPrivateController(icName, domain)
 	ic.Spec.HTTPHeaders = &operatorv1.IngressControllerHTTPHeaders{

test/e2e/haproxy_timeouts_test.go

@@ -24,6 +24,7 @@ import (
 )
 
 func TestHAProxyTimeouts(t *testing.T) {
+	t.Parallel()
 	const (
 		clientTimeoutInput     = 45 * time.Second
 		clientTimeoutOutput    = "45s"
@@ -178,6 +179,7 @@ func TestHAProxyTimeouts(t *testing.T) {
 }
 
 func TestHAProxyTimeoutsRejection(t *testing.T) {
+	t.Parallel()
 	const (
 		clientTimeoutInput      = -45 * time.Second
 		clientFinTimeoutInput   = 0 * time.Millisecond

test/e2e/hsts_policy_test.go

@@ -61,8 +61,9 @@ func TestHstsPolicyWorks(t *testing.T) {
 	}
 	defer func() {
 		// Remove the HSTS policies from the ingress config
-		ing.Spec.RequiredHSTSPolicies = nil
-		if err := kclient.Update(context.TODO(), ing); err != nil {
+		if err := updateIngressConfigSpecWithRetryOnConflict(t, clusterConfigName, timeout, func(spec *configv1.IngressSpec) {
+			spec.RequiredHSTSPolicies = nil
+		}); err != nil {
 			t.Fatalf("failed to restore ingress config: %v", err)
 		}
 	}()