From 074aba8dcea0318c23f6c99ff7b412563a41f365 Mon Sep 17 00:00:00 2001 From: caiweidong Date: Wed, 9 Oct 2019 11:15:49 +0800 Subject: [PATCH] Fix storage e2e clean up --- test/e2e/storage/testsuites/BUILD | 1 + test/e2e/storage/testsuites/base.go | 45 ++++++++++++++++---- test/e2e/storage/testsuites/disruptive.go | 15 ++++--- test/e2e/storage/testsuites/ephemeral.go | 7 ++- test/e2e/storage/testsuites/multivolume.go | 12 +++--- test/e2e/storage/testsuites/provisioning.go | 12 +++--- test/e2e/storage/testsuites/snapshottable.go | 5 ++- test/e2e/storage/testsuites/subpath.go | 22 +++++----- test/e2e/storage/testsuites/topology.go | 10 +++-- test/e2e/storage/testsuites/volume_expand.go | 16 +++---- test/e2e/storage/testsuites/volume_io.go | 7 ++- test/e2e/storage/testsuites/volumelimits.go | 6 ++- test/e2e/storage/testsuites/volumemode.go | 13 +++--- test/e2e/storage/testsuites/volumes.go | 12 +++--- 14 files changed, 112 insertions(+), 71 deletions(-) diff --git a/test/e2e/storage/testsuites/BUILD b/test/e2e/storage/testsuites/BUILD index 8f2d2074bbb..a6b6c7e418c 100644 --- a/test/e2e/storage/testsuites/BUILD +++ b/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", ], ) diff --git a/test/e2e/storage/testsuites/base.go b/test/e2e/storage/testsuites/base.go index 82dfb5b673b..7cf4d489a20 100644 --- a/test/e2e/storage/testsuites/base.go +++ b/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 @@ -685,3 +698,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 +} diff --git a/test/e2e/storage/testsuites/disruptive.go b/test/e2e/storage/testsuites/disruptive.go index 34f3723aa9c..6ca3502a34e 100644 --- a/test/e2e/storage/testsuites/disruptive.go +++ b/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 { - l.driverCleanup() - l.driverCleanup = nil - } + errs = append(errs, tryFunc(l.driverCleanup)) + l.driverCleanup = nil + framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource") } type testBody func(c clientset.Interface, f *framework.Framework, clientPod *v1.Pod) diff --git a/test/e2e/storage/testsuites/ephemeral.go b/test/e2e/storage/testsuites/ephemeral.go index c1a823d486a..8587257bb59 100644 --- a/test/e2e/storage/testsuites/ephemeral.go +++ b/test/e2e/storage/testsuites/ephemeral.go @@ -107,10 +107,9 @@ func (p *ephemeralTestSuite) defineTests(driver TestDriver, pattern testpatterns } cleanup := func() { - if l.driverCleanup != nil { - l.driverCleanup() - l.driverCleanup = nil - } + err := tryFunc(l.driverCleanup) + framework.ExpectNoError(err, "while cleaning up driver") + l.driverCleanup = nil } ginkgo.It("should create read-only inline ephemeral volume", func() { diff --git a/test/e2e/storage/testsuites/multivolume.go b/test/e2e/storage/testsuites/multivolume.go index 6244f263f8b..b2662d9f3be 100644 --- a/test/e2e/storage/testsuites/multivolume.go +++ b/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() - } - - if l.driverCleanup != nil { - l.driverCleanup() - l.driverCleanup = nil + errs = append(errs, resource.cleanupResource()) } + 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) } diff --git a/test/e2e/storage/testsuites/provisioning.go b/test/e2e/storage/testsuites/provisioning.go index ce87f9652b8..b41fbd4e77e 100644 --- a/test/e2e/storage/testsuites/provisioning.go +++ b/test/e2e/storage/testsuites/provisioning.go @@ -162,10 +162,9 @@ func (p *provisioningTestSuite) defineTests(driver TestDriver, pattern testpatte } cleanup := func() { - if l.driverCleanup != nil { - l.driverCleanup() - l.driverCleanup = nil - } + err := tryFunc(l.driverCleanup) + 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 diff --git a/test/e2e/storage/testsuites/snapshottable.go b/test/e2e/storage/testsuites/snapshottable.go index 8fce14c57d3..8d744607000 100644 --- a/test/e2e/storage/testsuites/snapshottable.go +++ b/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, "") diff --git a/test/e2e/storage/testsuites/subpath.go b/test/e2e/storage/testsuites/subpath.go index 342374d1159..6c17ec48fa4 100644 --- a/test/e2e/storage/testsuites/subpath.go +++ b/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 { - l.driverCleanup() - l.driverCleanup = nil - } + errs = append(errs, tryFunc(l.driverCleanup)) + l.driverCleanup = nil + framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource") if l.hostExec != nil { l.hostExec.Cleanup() diff --git a/test/e2e/storage/testsuites/topology.go b/test/e2e/storage/testsuites/topology.go index 423148f594e..410419cf38e 100644 --- a/test/e2e/storage/testsuites/topology.go +++ b/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 { - l.driverCleanup() - } + err := tryFunc(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") } diff --git a/test/e2e/storage/testsuites/volume_expand.go b/test/e2e/storage/testsuites/volume_expand.go index 0d1a1b60216..b07d3abd16e 100644 --- a/test/e2e/storage/testsuites/volume_expand.go +++ b/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 { - l.driverCleanup() - l.driverCleanup = nil - } - + errs = append(errs, tryFunc(l.driverCleanup)) + l.driverCleanup = nil + framework.ExpectNoError(errors.NewAggregate(errs), "while cleaning up resource") validateMigrationVolumeOpCounts(f.ClientSet, driver.GetDriverInfo().InTreePluginName, l.intreeOps, l.migratedOps) } diff --git a/test/e2e/storage/testsuites/volume_io.go b/test/e2e/storage/testsuites/volume_io.go index 3022b5dadee..fd0e53cba30 100644 --- a/test/e2e/storage/testsuites/volume_io.go +++ b/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) } diff --git a/test/e2e/storage/testsuites/volumelimits.go b/test/e2e/storage/testsuites/volumelimits.go index 6aaf3484d1b..3f7e3b9ec5c 100644 --- a/test/e2e/storage/testsuites/volumelimits.go +++ b/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) }() diff --git a/test/e2e/storage/testsuites/volumemode.go b/test/e2e/storage/testsuites/volumemode.go index f97940d4550..4cdab57e715 100644 --- a/test/e2e/storage/testsuites/volumemode.go +++ b/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() - - if l.driverCleanup != nil { - l.driverCleanup() - l.driverCleanup = nil - } - + var errs []error + errs = append(errs, l.cleanupResource()) + 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) } diff --git a/test/e2e/storage/testsuites/volumes.go b/test/e2e/storage/testsuites/volumes.go index bfcdfef7e2c..81244749852 100644 --- a/test/e2e/storage/testsuites/volumes.go +++ b/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 { - l.driverCleanup() - l.driverCleanup = nil - } - + 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) }