Use test namespaces with numeric suffix (knative#5337)

* Use test namespaces from pre-defined range * ReuseNamespace flag * ReuseNamespace variable in lib package * prevents import cycle when flags are accessed directly from the lib package * Fix gofmt * Fix goimports * Remove the last occurrence of import cycle * Remove unused variable * Fix detection of NotFound api error * also make the retry count and interval shorter to speed up things * Delete NS only if we created it * leave previously existing namespaces there * Call CreateNamespaceWithRetry directly
mgencur · Aug 3, 2021 · 86556d5 · 86556d5
1 parent 1be9f5f
commit 86556d5
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 43 deletions.
diff --git a/test/conformance/main_test.go b/test/conformance/main_test.go
@@ -47,6 +47,7 @@ var brokerClass string
 func TestMain(m *testing.M) {
 	os.Exit(func() int {
 		test.InitializeEventingFlags()
+		testlib.ReuseNamespace = test.EventingFlags.ReuseNamespace
 		channelTestRunner = testlib.ComponentsTestRunner{
 			ComponentFeatureMap: testlib.ChannelFeatureMap,
 			ComponentsToTest:    test.EventingFlags.Channels,

diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go
@@ -33,6 +33,7 @@ var brokerClass string
 
 func TestMain(m *testing.M) {
 	test.InitializeEventingFlags()
+	testlib.ReuseNamespace = test.EventingFlags.ReuseNamespace
 	channelTestRunner = testlib.ComponentsTestRunner{
 		ComponentFeatureMap: testlib.ChannelFeatureMap,
 		ComponentsToTest:    test.EventingFlags.Channels,

diff --git a/test/e2e_flags.go b/test/e2e_flags.go
@@ -60,6 +60,9 @@ func InitializeEventingFlags() {
 	flag.Var(&EventingFlags.Brokers, "brokers", BrokerUsage)
 	flag.StringVar(&EventingFlags.BrokerName, "brokername", "", BrokerNameUsage)
 	flag.StringVar(&EventingFlags.BrokerNamespace, "brokernamespace", "", BrokerNamespaceUsage)
+	// Might be useful in restricted environments where namespaces need to be
+	// created by a user with increased privileges (admin).
+	flag.BoolVar(&EventingFlags.ReuseNamespace, "reusenamespace", false, "Whether to re-use namespace for a test if it already exists.")
 	flag.Parse()
 
 	// If no channel is passed through the flag, initialize it as the DefaultChannel.

diff --git a/test/flags/eventing_environment.go b/test/flags/eventing_environment.go
@@ -26,4 +26,5 @@ type EventingEnvironmentFlags struct {
 	ReadyFile       string
 	BrokerName      string
 	BrokerNamespace string
+	ReuseNamespace  bool
 }
diff --git a/test/lib/test_runner.go b/test/lib/test_runner.go
@@ -18,21 +18,21 @@ package lib
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
 	"path/filepath"
 	"sort"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
-	"k8s.io/apiserver/pkg/storage/names"
-
 	pkgTest "knative.dev/pkg/test"
-	"knative.dev/pkg/test/helpers"
 	"knative.dev/pkg/test/prow"
 
 	"knative.dev/eventing/pkg/utils"
@@ -47,6 +47,15 @@ import (
 const (
 	podLogsDir         = "pod-logs"
 	testPullSecretName = "kn-eventing-test-pull-secret"
+	MaxNamespaceSkip   = 20
+	MaxRetries         = 5
+	RetrySleepDuration = 2 * time.Second
+)
+
+var (
+	nsMutex        sync.Mutex
+	namespaceCount int
+	ReuseNamespace bool
 )
 
 // ComponentsTestRunner is used to run tests against different eventing components.
@@ -148,19 +157,16 @@ var SetupClientOptionNoop SetupClientOption = func(*Client) {
 // Setup creates the client objects needed in the e2e tests,
 // and does other setups, like creating namespaces, set the test case to run in parallel, etc.
 func Setup(t *testing.T, runInParallel bool, options ...SetupClientOption) *Client {
-	// Create a new namespace to run this test case.
-	namespace := makeK8sNamespace(t.Name())
-	t.Logf("namespace is : %q", namespace)
-	client, err := NewClient(
-		pkgTest.Flags.Kubeconfig,
-		pkgTest.Flags.Cluster,
-		namespace,
-		t)
+	client, err := CreateNamespacedClient(t)
 	if err != nil {
 		t.Fatal("Couldn't initialize clients:", err)
 	}
 
-	CreateNamespaceIfNeeded(t, client, namespace)
+	// If namespaces are re-used the pull-secret is supposed to be created in advance.
+	if !ReuseNamespace {
+		SetupServiceAccount(t, client)
+		SetupPullSecret(t, client)
+	}
 
 	// Run the test case in parallel if needed.
 	if runInParallel {
@@ -178,9 +184,73 @@ func Setup(t *testing.T, runInParallel bool, options ...SetupClientOption) *Clie
 	return client
 }
 
-func makeK8sNamespace(baseFuncName string) string {
-	base := helpers.MakeK8sNamePrefix(baseFuncName)
-	return names.SimpleNameGenerator.GenerateName(base + "-")
+func CreateNamespacedClient(t *testing.T) (*Client, error) {
+	ns := ""
+	// Try next MaxNamespaceSkip namespaces before giving up. This should address the issue with
+	// development cycles when namespaces from previous runs were not cleaned properly.
+	for i := 0; i < MaxNamespaceSkip; i++ {
+		ns = NextNamespace()
+		client, err := NewClient(
+			pkgTest.Flags.Kubeconfig,
+			pkgTest.Flags.Cluster,
+			ns,
+			t)
+		if err != nil {
+			return nil, err
+		}
+		if ReuseNamespace {
+			// Re-using existing namespace, no need to create it.
+			// The namespace is supposed to be created in advance.
+			return client, nil
+		} else {
+			// The test is supposed to create a new test namespace for itself.
+			// Keep trying until we find a namespace that doesn't exist yet.
+			if err := CreateNamespaceWithRetry(client, ns); err != nil {
+				if apierrs.IsAlreadyExists(err) {
+					continue
+				}
+				return nil, err
+			}
+		}
+		return client, nil
+	}
+	return nil, errors.New("unable to find available namespace")
+}
+
+// NextNamespace returns the next unique namespace.
+func NextNamespace() string {
+	ns := os.Getenv("EVENTING_E2E_NAMESPACE")
+	if ns == "" {
+		ns = "eventing-e2e"
+	}
+	return fmt.Sprintf("%s%d", ns, GetNextNamespaceId())
+}
+
+// GetNextNamespaceId return the next unique ID for the next namespace.
+func GetNextNamespaceId() int {
+	nsMutex.Lock()
+	defer nsMutex.Unlock()
+	current := namespaceCount
+	namespaceCount++
+	return current
+}
+
+// CreateNamespaceWithRetry creates the given namespace with retries.
+func CreateNamespaceWithRetry(client *Client, namespace string) error {
+	var (
+		retries int
+		err     error
+	)
+	for retries < MaxRetries {
+		nsSpec := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
+		if _, err = client.Kube.CoreV1().Namespaces().
+			Create(context.Background(), nsSpec, metav1.CreateOptions{}); err == nil {
+			return nil
+		}
+		retries++
+		time.Sleep(RetrySleepDuration)
+	}
+	return err
 }
 
 // TearDown will delete created names using clients.
@@ -227,8 +297,11 @@ func TearDown(client *Client) {
 	}
 
 	client.Tracker.Clean(true)
-	if err := DeleteNameSpace(client); err != nil {
-		client.T.Logf("Could not delete the namespace %q: %v", client.Namespace, err)
+	// If we're reusing existing namespaces leave the deletion to the creator.
+	if !ReuseNamespace {
+		if err := DeleteNameSpace(client); err != nil {
+			client.T.Logf("Could not delete the namespace %q: %v", client.Namespace, err)
+		}
 	}
 }
 
@@ -253,33 +326,25 @@ func formatEvent(e *corev1.Event) string {
 	}, "\n")
 }
 
-// CreateNamespaceIfNeeded creates a new namespace if it does not exist.
-func CreateNamespaceIfNeeded(t *testing.T, client *Client, namespace string) {
-	_, err := client.Kube.CoreV1().Namespaces().Get(context.Background(), namespace, metav1.GetOptions{})
-
-	if err != nil && apierrs.IsNotFound(err) {
-		nsSpec := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}
-		_, err = client.Kube.CoreV1().Namespaces().Create(context.Background(), nsSpec, metav1.CreateOptions{})
-
-		if err != nil {
-			t.Fatalf("Failed to create Namespace: %s; %v", namespace, err)
-		}
-
-		// https://github.com/kubernetes/kubernetes/issues/66689
-		// We can only start creating pods after the default ServiceAccount is created by the kube-controller-manager.
-		err = waitForServiceAccountExists(client, "default", namespace)
-		if err != nil {
-			t.Fatal("The default ServiceAccount was not created for the Namespace:", namespace)
-		}
+// SetupServiceAccount creates a new namespace if it does not exist.
+func SetupServiceAccount(t *testing.T, client *Client) {
+	// https://github.com/kubernetes/kubernetes/issues/66689
+	// We can only start creating pods after the default ServiceAccount is created by the kube-controller-manager.
+	err := waitForServiceAccountExists(client, "default", client.Namespace)
+	if err != nil {
+		t.Fatal("The default ServiceAccount was not created for the Namespace:", client.Namespace)
+	}
+}
 
-		// If the "default" Namespace has a secret called
-		// "kn-eventing-test-pull-secret" then use that as the ImagePullSecret
-		// on the "default" ServiceAccount in this new Namespace.
-		// This is needed for cases where the images are in a private registry.
-		_, err := utils.CopySecret(client.Kube.CoreV1(), "default", testPullSecretName, namespace, "default")
-		if err != nil && !apierrs.IsNotFound(err) {
-			t.Fatalf("error copying the secret into ns %q: %s", namespace, err)
-		}
+// SetupPullSecret sets up kn-eventing-test-pull-secret on the client namespace.
+func SetupPullSecret(t *testing.T, client *Client) {
+	// If the "default" Namespace has a secret called
+	// "kn-eventing-test-pull-secret" then use that as the ImagePullSecret
+	// on the "default" ServiceAccount in this new Namespace.
+	// This is needed for cases where the images are in a private registry.
+	_, err := utils.CopySecret(client.Kube.CoreV1(), "default", testPullSecretName, client.Namespace, "default")
+	if err != nil && !apierrs.IsNotFound(err) {
+		t.Fatalf("error copying the secret into ns %q: %s", client.Namespace, err)
 	}
 }