Merge pull request #83653 from cwdsuzhou/Octo/e2efix

Fix storage e2e clean up

test/e2e/storage/testsuites/BUILD

@@ -56,6 +56,7 @@ go_library(
         "//test/utils/image:go_default_library",
         "//vendor/github.com/onsi/ginkgo:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/github.com/pkg/errors:go_default_library",
     ],
 )
 

test/e2e/storage/testsuites/base.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo"
+	"github.com/pkg/errors"
 
 	v1 "k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
@@ -33,6 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	apierrors "k8s.io/apimachinery/pkg/util/errors"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	clientset "k8s.io/client-go/kubernetes"
@@ -81,7 +83,7 @@ type TestSuiteInfo struct {
 // TestResource represents an interface for resources that is used by TestSuite
 type TestResource interface {
 	// cleanupResource cleans up the test resources created when setting up the resource
-	cleanupResource()
+	cleanupResource() error
 }
 
 func getTestNameStr(suite TestSuite, pattern testpatterns.TestPattern) string {
@@ -268,9 +270,9 @@ func createVolumeSource(pvcName string, readOnly bool) *v1.VolumeSource {
 }
 
 // cleanupResource cleans up genericVolumeTestResource
-func (r *genericVolumeTestResource) cleanupResource() {
+func (r *genericVolumeTestResource) cleanupResource() error {
 	f := r.config.Framework
-
+	var cleanUpErrs []error
 	if r.pvc != nil || r.pv != nil {
 		switch r.pattern.VolType {
 		case testpatterns.PreprovisionedPV:
@@ -287,10 +289,15 @@ func (r *genericVolumeTestResource) cleanupResource() {
 			}
 			if r.pvc != nil {
 				err := e2epv.DeletePersistentVolumeClaim(f.ClientSet, r.pvc.Name, f.Namespace.Name)
-				framework.ExpectNoError(err, "Failed to delete PVC %v", r.pvc.Name)
+				if err != nil {
+					cleanUpErrs = append(cleanUpErrs, errors.Wrapf(err, "Failed to delete PVC %v", r.pvc.Name))
+				}
 				if r.pv != nil {
 					err = framework.WaitForPersistentVolumeDeleted(f.ClientSet, r.pv.Name, 5*time.Second, 5*time.Minute)
-					framework.ExpectNoError(err, "Persistent Volume %v not deleted by dynamic provisioner", r.pv.Name)
+					if err != nil {
+						cleanUpErrs = append(cleanUpErrs, errors.Wrapf(err,
+							"Persistent Volume %v not deleted by dynamic provisioner", r.pv.Name))
+					}
 				}
 			}
 		default:
@@ -300,13 +307,18 @@ func (r *genericVolumeTestResource) cleanupResource() {
 
 	if r.sc != nil {
 		ginkgo.By("Deleting sc")
-		deleteStorageClass(f.ClientSet, r.sc.Name)
+		if err := deleteStorageClass(f.ClientSet, r.sc.Name); err != nil {
+			cleanUpErrs = append(cleanUpErrs, errors.Wrapf(err, "Failed to delete StorageClass %v", r.sc.Name))
+		}
 	}
 
 	// Cleanup volume for pre-provisioned volume tests
 	if r.volume != nil {
-		r.volume.DeleteVolume()
+		if err := tryFunc(r.volume.DeleteVolume); err != nil {
+			cleanUpErrs = append(cleanUpErrs, errors.Wrap(err, "Failed to delete Volume"))
+		}
 	}
+	return apierrors.NewAggregate(cleanUpErrs)
 }
 
 func createPVCPV(
@@ -396,11 +408,12 @@ func isDelayedBinding(sc *storagev1.StorageClass) bool {
 }
 
 // deleteStorageClass deletes the passed in StorageClass and catches errors other than "Not Found"
-func deleteStorageClass(cs clientset.Interface, className string) {
+func deleteStorageClass(cs clientset.Interface, className string) error {
 	err := cs.StorageV1().StorageClasses().Delete(className, nil)
 	if err != nil && !apierrs.IsNotFound(err) {
-		framework.ExpectNoError(err)
+		return err
 	}
+	return nil
 }
 
 // convertTestConfig returns a framework test config with the
@@ -684,3 +697,17 @@ func skipVolTypePatterns(pattern testpatterns.TestPattern, driver TestDriver, sk
 		framework.Skipf("Driver supports dynamic provisioning, skipping %s pattern", pattern.VolType)
 	}
 }
+
+func tryFunc(f func()) error {
+	var err error
+	if f == nil {
+		return nil
+	}
+	defer func() {
+		if recoverError := recover(); recoverError != nil {
+			err = fmt.Errorf("%v", recoverError)
+		}
+	}()
+	f()
+	return err
+}

test/e2e/storage/testsuites/disruptive.go

@@ -18,7 +18,9 @@ package testsuites
 
 import (
 	"github.com/onsi/ginkgo"
+
 	v1 "k8s.io/api/core/v1"
+	errors "k8s.io/apimachinery/pkg/util/errors"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
@@ -97,22 +99,23 @@ func (s *disruptiveTestSuite) defineTests(driver TestDriver, pattern testpattern
 	}
 
 	cleanup := func() {
+		var errs []error
 		if l.pod != nil {
 			ginkgo.By("Deleting pod")
 			err := e2epod.DeletePodWithWait(f.ClientSet, l.pod)
-			framework.ExpectNoError(err, "while deleting pod")
+			errs = append(errs, err)
 			l.pod = nil
 		}
 
 		if l.resource != nil {
-			l.resource.cleanupResource()
+			err := l.resource.cleanupResource()
+			errs = append(errs, err)
 			l.resource = nil
 		}
 
-		if l.driverCleanup != nil {
+		errs = append(errs, tryFunc(l.driverCleanup))
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
-		}
 	}
 
 	type testBody func(c clientset.Interface, f *framework.Framework, clientPod *v1.Pod)

test/e2e/storage/testsuites/ephemeral.go

@@ -107,10 +107,9 @@ func (p *ephemeralTestSuite) defineTests(driver TestDriver, pattern testpatterns
 	}
 
 	cleanup := func() {
-		if l.driverCleanup != nil {
+		err := tryFunc(l.driverCleanup)
-			l.driverCleanup()
+		framework.ExpectNoError(err, "while cleaning up driver")
-			l.driverCleanup = nil
+		l.driverCleanup = nil
-		}
 	}
 
 	ginkgo.It("should create read-only inline ephemeral volume", func() {

test/e2e/storage/testsuites/multivolume.go

@@ -24,6 +24,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/errors"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2enode "k8s.io/kubernetes/test/e2e/framework/node"
@@ -109,15 +110,14 @@ func (t *multiVolumeTestSuite) defineTests(driver TestDriver, pattern testpatter
 	}
 
 	cleanup := func() {
+		var errs []error
 		for _, resource := range l.resources {
-			resource.cleanupResource()
+			errs = append(errs, resource.cleanupResource())
-		}
-
-		if l.driverCleanup != nil {
-			l.driverCleanup()
-			l.driverCleanup = nil
 		}
 
+		errs = append(errs, tryFunc(l.driverCleanup))
+		l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleanup resource")
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}
 

test/e2e/storage/testsuites/provisioning.go

@@ -162,10 +162,9 @@ func (p *provisioningTestSuite) defineTests(driver TestDriver, pattern testpatte
 	}
 
 	cleanup := func() {
-		if l.driverCleanup != nil {
+		err := tryFunc(l.driverCleanup)
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(err, "while cleaning up driver")
-		}
 
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}
@@ -454,7 +453,10 @@ func (t StorageClassTest) TestBindingWaitForFirstConsumerMultiPVC(claims []*v1.P
 	ginkgo.By("creating a storage class " + t.Class.Name)
 	class, err := t.Client.StorageV1().StorageClasses().Create(t.Class)
 	framework.ExpectNoError(err)
-	defer deleteStorageClass(t.Client, class.Name)
+	defer func() {
+		err = deleteStorageClass(t.Client, class.Name)
+		framework.ExpectNoError(err, "While deleting storage class")
+	}()
 
 	ginkgo.By("creating claims")
 	var claimNames []string

test/e2e/storage/testsuites/snapshottable.go

@@ -106,7 +106,10 @@ func (s *snapshottableTestSuite) defineTests(driver TestDriver, pattern testpatt
 
 		// Now do the more expensive test initialization.
 		config, driverCleanup := driver.PrepareTest(f)
-		defer driverCleanup()
+		defer func() {
+			err := tryFunc(driverCleanup)
+			framework.ExpectNoError(err, "while cleaning up driver")
+		}()
 
 		vsc := sDriver.GetSnapshotClass(config)
 		class := dDriver.GetDynamicProvisionStorageClass(config, "")

test/e2e/storage/testsuites/subpath.go

@@ -21,9 +21,14 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
+
+	"github.com/onsi/ginkgo"
+	"github.com/onsi/gomega"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -34,11 +39,6 @@ import (
 	"k8s.io/kubernetes/test/e2e/storage/testpatterns"
 	"k8s.io/kubernetes/test/e2e/storage/utils"
 	imageutils "k8s.io/kubernetes/test/utils/image"
-
-	"time"
-
-	"github.com/onsi/ginkgo"
-	"github.com/onsi/gomega"
 )
 
 var (
@@ -162,22 +162,22 @@ func (s *subPathTestSuite) defineTests(driver TestDriver, pattern testpatterns.T
 	}
 
 	cleanup := func() {
+		var errs []error
 		if l.pod != nil {
 			ginkgo.By("Deleting pod")
 			err := e2epod.DeletePodWithWait(f.ClientSet, l.pod)
-			framework.ExpectNoError(err, "while deleting pod")
+			errs = append(errs, err)
 			l.pod = nil
 		}
 
 		if l.resource != nil {
-			l.resource.cleanupResource()
+			errs = append(errs, l.resource.cleanupResource())
 			l.resource = nil
 		}
 
-		if l.driverCleanup != nil {
+		errs = append(errs, tryFunc(l.driverCleanup))
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
-		}
 
 		if l.hostExec != nil {
 			l.hostExec.Cleanup()

test/e2e/storage/testsuites/topology.go

@@ -154,9 +154,9 @@ func (t *topologyTestSuite) defineTests(driver TestDriver, pattern testpatterns.
 
 	cleanup := func(l topologyTest) {
 		t.cleanupResources(cs, &l)
-		if l.driverCleanup != nil {
+		err := tryFunc(l.driverCleanup)
-			l.driverCleanup()
+		l.driverCleanup = nil
-		}
+		framework.ExpectNoError(err, "while cleaning up driver")
 
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}
@@ -353,5 +353,7 @@ func (t *topologyTestSuite) cleanupResources(cs clientset.Interface, l *topology
 		err := e2epod.DeletePodWithWait(cs, l.pod)
 		framework.ExpectNoError(err, "while deleting pod")
 	}
-	l.resource.cleanupResource()
+
+	err := l.resource.cleanupResource()
+	framework.ExpectNoError(err, "while clean up resource")
 }

test/e2e/storage/testsuites/volume_expand.go

@@ -26,6 +26,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
@@ -113,30 +114,29 @@ func (v *volumeExpandTestSuite) defineTests(driver TestDriver, pattern testpatte
 	}
 
 	cleanup := func() {
+		var errs []error
 		if l.pod != nil {
 			ginkgo.By("Deleting pod")
 			err := e2epod.DeletePodWithWait(f.ClientSet, l.pod)
-			framework.ExpectNoError(err, "while deleting pod")
+			errs = append(errs, err)
 			l.pod = nil
 		}
 
 		if l.pod2 != nil {
 			ginkgo.By("Deleting pod2")
 			err := e2epod.DeletePodWithWait(f.ClientSet, l.pod2)
-			framework.ExpectNoError(err, "while deleting pod2")
+			errs = append(errs, err)
 			l.pod2 = nil
 		}
 
 		if l.resource != nil {
-			l.resource.cleanupResource()
+			errs = append(errs, l.resource.cleanupResource())
 			l.resource = nil
 		}
 
-		if l.driverCleanup != nil {
+		errs = append(errs, tryFunc(l.driverCleanup))
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
-		}
-
 		validateMigrationVolumeOpCounts(f.ClientSet, driver.GetDriverInfo().InTreePluginName, l.intreeOps, l.migratedOps)
 	}
 

test/e2e/storage/testsuites/volume_io.go

@@ -33,6 +33,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/errors"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
@@ -124,16 +125,18 @@ func (t *volumeIOTestSuite) defineTests(driver TestDriver, pattern testpatterns.
 	}
 
 	cleanup := func() {
+		var errs []error
 		if l.resource != nil {
-			l.resource.cleanupResource()
+			errs = append(errs, l.resource.cleanupResource())
 			l.resource = nil
 		}
 
 		if l.driverCleanup != nil {
-			l.driverCleanup()
+			errs = append(errs, tryFunc(l.driverCleanup))
 			l.driverCleanup = nil
 		}
 
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}
 

test/e2e/storage/testsuites/volumelimits.go

@@ -143,8 +143,10 @@ func (t *volumeLimitsTestSuite) defineTests(driver TestDriver, pattern testpatte
 		framework.ExpectNoError(err, "determine intersection of test size range %+v and driver size range %+v", testVolumeSizeRange, dDriver)
 
 		l.resource = createGenericVolumeTestResource(driver, l.config, pattern, testVolumeSizeRange)
-		defer l.resource.cleanupResource()
+		defer func() {
-
+			err := l.resource.cleanupResource()
+			framework.ExpectNoError(err, "while cleaning up resource")
+		}()
 		defer func() {
 			cleanupTest(l.cs, l.ns.Name, l.runningPod.Name, l.unschedulablePod.Name, l.pvcs, l.pvNames)
 		}()

test/e2e/storage/testsuites/volumemode.go

@@ -29,6 +29,7 @@ import (
 	storagev1 "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	volevents "k8s.io/kubernetes/pkg/controller/volume/events"
@@ -177,13 +178,11 @@ func (t *volumeModeTestSuite) defineTests(driver TestDriver, pattern testpattern
 	}
 
 	cleanup := func() {
-		l.cleanupResource()
+		var errs []error
-
+		errs = append(errs, l.cleanupResource())
-		if l.driverCleanup != nil {
+		errs = append(errs, tryFunc(l.driverCleanup))
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
-		}
-
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}
 

test/e2e/storage/testsuites/volumes.go

@@ -28,6 +28,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
 	"k8s.io/kubernetes/test/e2e/framework/volume"
@@ -134,16 +135,15 @@ func (t *volumesTestSuite) defineTests(driver TestDriver, pattern testpatterns.T
 	}
 
 	cleanup := func() {
+		var errs []error
 		if l.resource != nil {
-			l.resource.cleanupResource()
+			errs = append(errs, l.resource.cleanupResource())
 			l.resource = nil
 		}
 
-		if l.driverCleanup != nil {
+		errs = append(errs, tryFunc(l.driverCleanup))
-			l.driverCleanup()
+		l.driverCleanup = nil
-			l.driverCleanup = nil
+		framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource")
-		}
-
 		validateMigrationVolumeOpCounts(f.ClientSet, dInfo.InTreePluginName, l.intreeOps, l.migratedOps)
 	}