Fix the random failure of E2E test cases (#4005)

Fix the random failure by increasing the timeout and introducing few minor refactor/bug fixes

Fixes #3970

Signed-off-by: Wenkai Yin(尹文开) <yinw@vmware.com>

test/e2e/common.go

@@ -25,7 +25,6 @@ import (
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 	corev1api "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 
@@ -37,36 +36,6 @@ func ensureClusterExists(ctx context.Context) error {
 	return exec.CommandContext(ctx, "kubectl", "cluster-info").Run()
 }
 
-// createNamespace creates a kubernetes namespace
-func createNamespace(ctx context.Context, client testClient, namespace string) error {
-	ns := builder.ForNamespace(namespace).Result()
-	_, err := client.clientGo.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{})
-	if apierrors.IsAlreadyExists(err) {
-		return nil
-	}
-	return err
-}
-
-func getNamespace(ctx context.Context, client testClient, namespace string) (*corev1api.Namespace, error) {
-	return client.clientGo.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
-}
-
-// waitForNamespaceDeletion waits for namespace to be deleted.
-func waitForNamespaceDeletion(interval, timeout time.Duration, client testClient, ns string) error {
-	err := wait.PollImmediate(interval, timeout, func() (bool, error) {
-		_, err := client.clientGo.CoreV1().Namespaces().Get(context.TODO(), ns, metav1.GetOptions{})
-		if err != nil {
-			if apierrors.IsNotFound(err) {
-				return true, nil
-			}
-			return false, err
-		}
-		fmt.Printf("Namespace %s is still being deleted...\n", ns)
-		return false, nil
-	})
-	return err
-}
-
 func createSecretFromFiles(ctx context.Context, client testClient, namespace string, name string, files map[string]string) error {
 	data := make(map[string][]byte)
 

test/e2e/enable_api_group_versions_test.go

@@ -224,9 +224,13 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 			if err := createNamespace(ctx, client, ns); err != nil {
 				return errors.Wrapf(err, "create %s namespace", ns)
 			}
+			defer func(namespace string) {
+				if err = deleteNamespace(ctx, client, namespace, true); err != nil {
+					fmt.Println(errors.Wrapf(err, "failed to delete the namespace %q", ns))
+				}
+			}(ns)
 
 			if err := installCR(ctx, cr, ns); err != nil {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.Wrapf(err, "install %s custom resource on source cluster in namespace %s", cr, ns)
 			}
 
@@ -237,7 +241,6 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 		// instead of waiting for discovery helper to refresh. See
 		// https://github.com/vmware-tanzu/velero/issues/3471.
 		if err := restartPods(ctx, veleroNamespace); err != nil {
-			deleteNamespacesOnErr(ctx, tc.namespaces)
 			return errors.Wrapf(err, "restart Velero pods")
 		}
 
@@ -247,18 +250,15 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 		err = veleroBackupNamespace(ctx, veleroCLI, veleroNamespace, backup, namespacesStr, "", false)
 		if err != nil {
 			veleroBackupLogs(ctx, veleroCLI, veleroNamespace, backup)
-			deleteNamespacesOnErr(ctx, tc.namespaces)
 			return errors.Wrapf(err, "back up %s namespaces on source cluster", namespacesStr)
 		}
 
 		if err := deleteCRD(ctx, tc.srcCrdYaml); err != nil {
-			deleteNamespacesOnErr(ctx, tc.namespaces)
 			return errors.Wrapf(err, "delete music-system CRD from source cluster")
 		}
 
 		for _, ns := range tc.namespaces {
-			if err := deleteNamespace(ctx, ns); err != nil {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
+			if err := deleteNamespace(ctx, client, ns, true); err != nil {
 				return errors.Wrapf(err, "delete %s namespace from source cluster", ns)
 			}
 		}
@@ -266,7 +266,6 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 		// Install music-system CRD for target cluster.
 		if tc.tgtCrdYaml != "" {
 			if err := installCRD(ctx, tc.tgtCrdYaml); err != nil {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.Wrapf(err, "install music-system CRD on target cluster")
 			}
 		}
@@ -275,14 +274,12 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 		if tc.cm != nil {
 			_, err := client.clientGo.CoreV1().ConfigMaps(veleroNamespace).Create(ctx, tc.cm, metav1.CreateOptions{})
 			if err != nil {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.Wrap(err, "create config map with user version priorities")
 			}
 		}
 
 		// Reset Velero to recognize music-system CRD.
 		if err := restartPods(ctx, veleroNamespace); err != nil {
-			deleteNamespacesOnErr(ctx, tc.namespaces)
 			return errors.Wrapf(err, "restart Velero pods")
 		}
 
@@ -292,13 +289,11 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 		if tc.want != nil {
 			if err := veleroRestore(ctx, veleroCLI, veleroNamespace, restore, backup); err != nil {
 				veleroRestoreLogs(ctx, veleroCLI, veleroNamespace, restore)
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.Wrapf(err, "restore %s namespaces on target cluster", namespacesStr)
 			}
 
 			annoSpec, err := resourceInfo(ctx, group, tc.tgtVer, resource)
 			if err != nil {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.Wrapf(
 					err,
 					"get annotation and spec from %s.%s/%s object",
@@ -315,7 +310,6 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 					annoSpec["annotations"],
 					tc.want["annotations"],
 				)
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.New(msg)
 			}
 
@@ -326,7 +320,6 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 					annoSpec["specs"],
 					tc.want["specs"],
 				)
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.New(msg)
 			}
 
@@ -336,16 +329,10 @@ func runEnableAPIGroupVersionsTests(ctx context.Context, client testClient, reso
 			err := veleroRestore(ctx, veleroCLI, veleroNamespace, restore, backup)
 
 			if err.Error() != "Unexpected restore phase got PartiallyFailed, expecting Completed" {
-				deleteNamespacesOnErr(ctx, tc.namespaces)
 				return errors.New("expected error but not none")
 			}
 		}
 
-		// Clean up.
-		for _, ns := range tc.namespaces {
-			fmt.Println("Delete namespace", ns)
-			deleteNamespace(ctx, ns)
-		}
 		_ = deleteCRD(ctx, tc.srcCrdYaml)
 		if tc.tgtCrdYaml != "" {
 			_ = deleteCRD(ctx, tc.tgtCrdYaml)
@@ -396,33 +383,6 @@ func restartPods(ctx context.Context, ns string) error {
 	return nil
 }
 
-func deleteNamespace(ctx context.Context, ns string) error {
-	fmt.Println("Delete namespace", ns)
-	cmd := exec.CommandContext(ctx, "kubectl", "delete", "ns", ns, "--wait")
-
-	_, stderr, err := veleroexec.RunCommand(cmd)
-	if strings.Contains(stderr, "not found") {
-		return nil
-	}
-	if err != nil {
-		return errors.Wrap(err, stderr)
-	}
-
-	return nil
-}
-
-// DeleteNamespacesOnErr cleans up the namespaces created for a test cast after an
-// error interrupts a test case.
-func deleteNamespacesOnErr(ctx context.Context, namespaces []string) {
-	if len(namespaces) > 0 {
-		fmt.Println("An error has occurred. Cleaning up test case namespaces.")
-	}
-
-	for _, ns := range namespaces {
-		deleteNamespace(ctx, ns)
-	}
-}
-
 func installCR(ctx context.Context, crFile, ns string) error {
 	retries := 5
 	var stderr string

test/e2e/kibishii_tests.go

@@ -25,8 +25,6 @@ import (
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec"
 )
 
@@ -93,18 +91,19 @@ func verifyData(ctx context.Context, namespace string, levels int, filesPerLevel
 // runKibishiiTests runs kibishii tests on the provider.
 func runKibishiiTests(client testClient, providerName, veleroCLI, veleroNamespace, backupName, restoreName, backupLocation string,
 	useVolumeSnapshots bool, registryCredentialFile string) error {
-	fiveMinTimeout, _ := context.WithTimeout(context.Background(), 5*time.Minute)
 	oneHourTimeout, _ := context.WithTimeout(context.Background(), time.Minute*60)
-	timeout := 10 * time.Minute
-	interval := 5 * time.Second
 	serviceAccountName := "default"
-
-	if err := createNamespace(fiveMinTimeout, client, kibishiiNamespace); err != nil {
+	if err := createNamespace(oneHourTimeout, client, kibishiiNamespace); err != nil {
 		return errors.Wrapf(err, "Failed to create namespace %s to install Kibishii workload", kibishiiNamespace)
 	}
+	defer func() {
+		if err := deleteNamespace(oneHourTimeout, client, kibishiiNamespace, true); err != nil {
+			fmt.Println(errors.Wrapf(err, "failed to delete the namespace %q", kibishiiNamespace))
+		}
+	}()
 
 	// wait until the service account is created before patch the image pull secret
-	if err := waitUntilServiceAccountCreated(oneHourTimeout, client, kibishiiNamespace, serviceAccountName, timeout); err != nil {
+	if err := waitUntilServiceAccountCreated(oneHourTimeout, client, kibishiiNamespace, serviceAccountName, 10*time.Minute); err != nil {
 		return errors.Wrapf(err, "failed to wait the service account %q created under the namespace %q", serviceAccountName, kibishiiNamespace)
 	}
 	// add the image pull secret to avoid the image pull limit issue of Docker Hub
@@ -112,7 +111,7 @@ func runKibishiiTests(client testClient, providerName, veleroCLI, veleroNamespac
 		return errors.Wrapf(err, "failed to patch the service account %q under the namespace %q", serviceAccountName, kibishiiNamespace)
 	}
 
-	if err := installKibishii(fiveMinTimeout, kibishiiNamespace, providerName); err != nil {
+	if err := installKibishii(oneHourTimeout, kibishiiNamespace, providerName); err != nil {
 		return errors.Wrap(err, "Failed to install Kibishii workload")
 	}
 
@@ -128,7 +127,7 @@ func runKibishiiTests(client testClient, providerName, veleroCLI, veleroNamespac
 	}
 
 	if err := veleroBackupNamespace(oneHourTimeout, veleroCLI, veleroNamespace, backupName, kibishiiNamespace, backupLocation, useVolumeSnapshots); err != nil {
-		veleroBackupLogs(fiveMinTimeout, veleroCLI, veleroNamespace, backupName)
+		veleroBackupLogs(oneHourTimeout, veleroCLI, veleroNamespace, backupName)
 		return errors.Wrapf(err, "Failed to backup kibishii namespace %s", kibishiiNamespace)
 	}
 
@@ -141,17 +140,12 @@ func runKibishiiTests(client testClient, providerName, veleroCLI, veleroNamespac
 		}
 	}
 	fmt.Printf("Simulating a disaster by removing namespace %s\n", kibishiiNamespace)
-	if err := client.clientGo.CoreV1().Namespaces().Delete(oneHourTimeout, kibishiiNamespace, metav1.DeleteOptions{}); err != nil {
-		return errors.Wrap(err, "Failed to simulate a disaster")
-	}
-	// wait for ns delete
-	err := waitForNamespaceDeletion(interval, timeout, client, kibishiiNamespace)
-	if err != nil {
-		return errors.Wrapf(err, fmt.Sprintf("Failed to wait for deletion of namespace %s", kibishiiNamespace))
+	if err := deleteNamespace(oneHourTimeout, client, kibishiiNamespace, true); err != nil {
+		return errors.Wrapf(err, "failed to delete namespace %s", kibishiiNamespace)
 	}
 
 	if err := veleroRestore(oneHourTimeout, veleroCLI, veleroNamespace, restoreName, backupName); err != nil {
-		veleroRestoreLogs(fiveMinTimeout, veleroCLI, veleroNamespace, restoreName)
+		veleroRestoreLogs(oneHourTimeout, veleroCLI, veleroNamespace, restoreName)
 		return errors.Wrapf(err, "Restore %s failed from backup %s", restoreName, backupName)
 	}
 
@@ -168,13 +162,6 @@ func runKibishiiTests(client testClient, providerName, veleroCLI, veleroNamespac
 		return errors.Wrap(err, "Failed to verify data generated by kibishii")
 	}
 
-	if err := client.clientGo.CoreV1().Namespaces().Delete(oneHourTimeout, kibishiiNamespace, metav1.DeleteOptions{}); err != nil {
-		return errors.Wrapf(err, "Failed to cleanup %s workload namespace", kibishiiNamespace)
-	}
-	// wait for ns delete
-	if err = waitForNamespaceDeletion(interval, timeout, client, kibishiiNamespace); err != nil {
-		return errors.Wrapf(err, fmt.Sprintf("Failed to wait for deletion of namespace %s", kibishiiNamespace))
-	}
 	fmt.Printf("kibishii test completed successfully\n")
 	return nil
 }

test/e2e/multiple_namespaces_test.go

@@ -34,8 +34,7 @@ var _ = Describe("[Basic] Backup/restore of 2 namespaces", func() {
 
 	AfterEach(func() {
 		if installVelero {
-			timeoutCTX, _ := context.WithTimeout(context.Background(), time.Minute)
-			err := veleroUninstall(timeoutCTX, client.kubebuilder, installVelero, veleroNamespace)
+			err := veleroUninstall(context.Background(), client.kubebuilder, installVelero, veleroNamespace)
 			Expect(err).To(Succeed())
 		}
 
@@ -70,8 +69,7 @@ var _ = Describe("[Scale] Backup/restore of 2500 namespaces", func() {
 
 	AfterEach(func() {
 		if installVelero {
-			timeoutCTX, _ := context.WithTimeout(context.Background(), time.Minute)
-			err := veleroUninstall(timeoutCTX, client.kubebuilder, installVelero, veleroNamespace)
+			err := veleroUninstall(context.Background(), client.kubebuilder, installVelero, veleroNamespace)
 			Expect(err).To(Succeed())
 		}
 
@@ -81,21 +79,20 @@ var _ = Describe("[Scale] Backup/restore of 2500 namespaces", func() {
 		It("should be successfully backed up and restored", func() {
 			backupName := "backup-" + uuidgen.String()
 			restoreName := "restore-" + uuidgen.String()
-			oneHourTimeout, _ := context.WithTimeout(context.Background(), 1*time.Hour)
-			Expect(RunMultipleNamespaceTest(oneHourTimeout, client, "nstest-"+uuidgen.String(), 2500,
+			twoHourTimeout, _ := context.WithTimeout(context.Background(), 2*time.Hour)
+			Expect(RunMultipleNamespaceTest(twoHourTimeout, client, "nstest-"+uuidgen.String(), 2500,
 				backupName, restoreName)).To(Succeed(), "Failed to successfully backup and restore multiple namespaces")
 		})
 	})
 })
 
 func RunMultipleNamespaceTest(ctx context.Context, client testClient, nsBaseName string, numberOfNamespaces int, backupName string, restoreName string) error {
-	shortTimeout, _ := context.WithTimeout(ctx, 5*time.Minute)
 	defer cleanupNamespaces(ctx, client, nsBaseName) // Run at exit for final cleanup
 	var excludeNamespaces []string
 
 	// Currently it's hard to build a large list of namespaces to include and wildcards do not work so instead
 	// we will exclude all of the namespaces that existed prior to the test from the backup
-	namespaces, err := client.clientGo.CoreV1().Namespaces().List(shortTimeout, v1.ListOptions{})
+	namespaces, err := client.clientGo.CoreV1().Namespaces().List(ctx, v1.ListOptions{})
 	if err != nil {
 		return errors.Wrap(err, "Could not retrieve namespaces")
 	}
@@ -103,6 +100,8 @@ func RunMultipleNamespaceTest(ctx context.Context, client testClient, nsBaseName
 	for _, excludeNamespace := range namespaces.Items {
 		excludeNamespaces = append(excludeNamespaces, excludeNamespace.Name)
 	}
+
+	fmt.Printf("Creating namespaces ...\n")
 	for nsNum := 0; nsNum < numberOfNamespaces; nsNum++ {
 		createNSName := fmt.Sprintf("%s-%00000d", nsBaseName, nsNum)
 		if err := createNamespace(ctx, client, createNSName); err != nil {
@@ -121,13 +120,14 @@ func RunMultipleNamespaceTest(ctx context.Context, client testClient, nsBaseName
 
 	err = veleroRestore(ctx, veleroCLI, veleroNamespace, restoreName, backupName)
 	if err != nil {
+		veleroRestoreLogs(ctx, veleroCLI, veleroNamespace, restoreName)
 		return errors.Wrap(err, "Restore failed")
 	}
 
 	// Verify that we got back all of the namespaces we created
 	for nsNum := 0; nsNum < numberOfNamespaces; nsNum++ {
 		checkNSName := fmt.Sprintf("%s-%00000d", nsBaseName, nsNum)
-		checkNS, err := getNamespace(shortTimeout, client, checkNSName)
+		checkNS, err := getNamespace(ctx, client, checkNSName)
 		if err != nil {
 			return errors.Wrapf(err, "Could not retrieve test namespace %s", checkNSName)
 		}
@@ -145,9 +145,9 @@ func cleanupNamespaces(ctx context.Context, client testClient, nsBaseName string
 		return errors.Wrap(err, "Could not retrieve namespaces")
 	}
 
+	fmt.Printf("Cleaning up namespaces ...\n")
 	for _, checkNamespace := range namespaces.Items {
 		if strings.HasPrefix(checkNamespace.Name, nsBaseName) {
-			fmt.Printf("Cleaning up namespace %s\n", checkNamespace.Name)
 			err = client.clientGo.CoreV1().Namespaces().Delete(ctx, checkNamespace.Name, v1.DeleteOptions{})
 			if err != nil {
 				return errors.Wrapf(err, "Could not delete namespace %s", checkNamespace.Name)

test/e2e/namespace.go

@@ -0,0 +1,67 @@
+/*
+Copyright the Velero contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+
+	corev1api "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	waitutil "k8s.io/apimachinery/pkg/util/wait"
+
+	"github.com/vmware-tanzu/velero/pkg/builder"
+)
+
+func createNamespace(ctx context.Context, client testClient, namespace string) error {
+	ns := builder.ForNamespace(namespace).Result()
+	_, err := client.clientGo.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{})
+	if apierrors.IsAlreadyExists(err) {
+		return nil
+	}
+	return err
+}
+
+func getNamespace(ctx context.Context, client testClient, namespace string) (*corev1api.Namespace, error) {
+	return client.clientGo.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
+}
+
+func deleteNamespace(ctx context.Context, client testClient, namespace string, wait bool) error {
+	if err := client.clientGo.CoreV1().Namespaces().Delete(ctx, namespace, metav1.DeleteOptions{}); err != nil {
+		return errors.Wrap(err, fmt.Sprintf("failed to delete the namespace %q", namespace))
+	}
+	if !wait {
+		return nil
+	}
+
+	return waitutil.PollImmediateInfinite(5*time.Second,
+		func() (bool, error) {
+			if _, err := client.clientGo.CoreV1().Namespaces().Get(context.TODO(), namespace, metav1.GetOptions{}); err != nil {
+				if apierrors.IsNotFound(err) {
+					return true, nil
+				}
+				return false, err
+			}
+			logrus.Debugf("namespace %q is still being deleted...", namespace)
+			return false, nil
+		})
+}