Merge pull request #33981 from freehan/e2e-deletens-on-failure

Automatic merge from submit-queue

add delete-namespace-on-failure flag

I have been doing this for a while. 

Setting `--delete-namespace=false --clean-start=true` only works if you have only one e2e test running in a loop. 

This PR lets someone to set `delete-namespace-on-failure=false` and run multiple tests in parallel and preserve the crime scene. It makes it easier to reproduce failures. 

Let me know if this is worth it or there are some other tricks I am not aware.

docs/devel/e2e-tests.md

@@ -137,6 +137,9 @@ go run hack/e2e.go -v --test --test_args="--ginkgo.skip=Pods.*env"
 # Run tests in parallel, skip any that must be run serially
 GINKGO_PARALLEL=y go run hack/e2e.go --v --test --test_args="--ginkgo.skip=\[Serial\]"
 
+# Run tests in parallel, skip any that must be run serially and keep the test namespace if test failed
+GINKGO_PARALLEL=y go run hack/e2e.go --v --test --test_args="--ginkgo.skip=\[Serial\] --delete-namespace-on-falure=false"
+
 # Flags can be combined, and their actions will take place in this order:
 # --build, --up, --test, --down
 #

hack/verify-flags/known-flags.txt

@@ -118,6 +118,7 @@ delete-collection-workers
 delete-instances
 delete-local-data
 delete-namespace
+delete-namespace-on-failure
 deleting-pods-burst
 deleting-pods-qps
 deployment-controller-sync-period

test/e2e/framework/framework.go

@@ -315,7 +315,10 @@ func (f *Framework) AfterEach() {
 	// expectation failures preventing deleting the namespace.
 	defer func() {
 		nsDeletionErrors := map[string]error{}
-		if TestContext.DeleteNamespace {
+		// Whether to delete namespace is determined by 3 factors: delete-namespace flag, delete-namespace-on-failure flag and the test result
+		// if delete-namespace set to false, namespace will always be preserved.
+		// if delete-namespace is true and delete-namespace-on-failure is false, namespace will be preserved if test failed.
+		if TestContext.DeleteNamespace && (TestContext.DeleteNamespaceOnFailure || !CurrentGinkgoTestDescription().Failed) {
 			for _, ns := range f.namespacesToDelete {
 				By(fmt.Sprintf("Destroying namespace %q for this suite.", ns.Name))
 				timeout := 5 * time.Minute
@@ -333,7 +336,12 @@ func (f *Framework) AfterEach() {
 			// Delete the federation namespace.
 			f.deleteFederationNs()
 		} else {
-			Logf("Found DeleteNamespace=false, skipping namespace deletion!")
+			if TestContext.DeleteNamespace {
+				Logf("Found DeleteNamespace=false, skipping namespace deletion!")
+			} else if TestContext.DeleteNamespaceOnFailure {
+				Logf("Found DeleteNamespaceOnFailure=false, skipping namespace deletion!")
+			}
+
 		}
 
 		// Paranoia-- prevent reuse!

test/e2e/framework/test_context.go

@@ -55,6 +55,7 @@ type TestContextType struct {
 	NodeOSDistro             string
 	VerifyServiceAccount     bool
 	DeleteNamespace          bool
+	DeleteNamespaceOnFailure bool
 	AllowedNotReadyNodes     int
 	CleanStart               bool
 	// If set to 'true' or 'all' framework will start a goroutine monitoring resource usage of system add-ons.
@@ -148,6 +149,7 @@ func RegisterCommonFlags() {
 	flag.StringVar(&TestContext.OutputPrintType, "output-print-type", "hr", "Comma separated list: 'hr' for human readable summaries 'json' for JSON ones.")
 	flag.BoolVar(&TestContext.DumpLogsOnFailure, "dump-logs-on-failure", true, "If set to true test will dump data about the namespace in which test was running.")
 	flag.BoolVar(&TestContext.DeleteNamespace, "delete-namespace", true, "If true tests will delete namespace after completion. It is only designed to make debugging easier, DO NOT turn it off by default.")
+	flag.BoolVar(&TestContext.DeleteNamespaceOnFailure, "delete-namespace-on-failure", true, "If true, framework will delete test namespace on failure. Used only during test debugging.")
 	flag.IntVar(&TestContext.AllowedNotReadyNodes, "allowed-not-ready-nodes", 0, "If non-zero, framework will allow for that many non-ready nodes when checking for all ready nodes.")
 	flag.StringVar(&TestContext.Host, "host", "http://127.0.0.1:8080", "The host, or apiserver, to connect to")
 	flag.StringVar(&TestContext.ReportPrefix, "report-prefix", "", "Optional prefix for JUnit XML reports. Default is empty, which doesn't prepend anything to the default name.")