e2e framework: remove AddAfterEach

For cleanup purposes the ginkgo.DeferCleanup is a better replacement for
f.AddAfterEach:
- the cleanup only gets executed when the corresponding setup code ran
  and can use the same local variables
- the callback runs after the test and before the framework
  deletes namespaces (as before)
- if one callback fails, the others still get executed

For the original purpose (https://github.com/kubernetes/kubernetes/pull/86177 "This is
very useful for custom gathering scripts.") it is now possible to use
ginkgo.AfterEach because it will always get executed. Just beware that its
callbacks run in first-in-first-out order.
This commit is contained in:
Patrick Ohly 2022-08-26 12:31:19 +02:00
parent cbf94307ef
commit 53e1c7b4c3
4 changed files with 23 additions and 52 deletions

View File

@ -103,11 +103,6 @@ type Framework struct {
// should abort, the AfterSuite hook should run all Cleanup actions.
cleanupHandle CleanupActionHandle
// afterEaches is a map of name to function to be called after each test. These are not
// cleared. The call order is randomized so that no dependencies can grow between
// the various afterEaches
afterEaches map[string]AfterEachActionFunc
// configuration for framework's client
Options Options
@ -122,9 +117,6 @@ type Framework struct {
Timeouts *TimeoutContext
}
// AfterEachActionFunc is a function that can be called after each test
type AfterEachActionFunc func(f *Framework, failed bool)
// TestDataSummary is an interface for managing test data.
type TestDataSummary interface {
SummaryKind() string
@ -168,20 +160,6 @@ func NewFramework(baseName string, options Options, client clientset.Interface)
Timeouts: NewTimeoutContextWithDefaults(),
}
f.AddAfterEach("dumpNamespaceInfo", func(f *Framework, failed bool) {
if !failed {
return
}
if !TestContext.DumpLogsOnFailure {
return
}
if !f.SkipNamespaceCreation {
for _, ns := range f.namespacesToDelete {
DumpAllNamespaceInfo(f.ClientSet, ns.Name)
}
}
})
ginkgo.BeforeEach(f.BeforeEach)
return f
@ -196,6 +174,9 @@ func (f *Framework) BeforeEach() {
// In addition, AfterEach will not be called if a test never gets here.
ginkgo.DeferCleanup(f.AfterEach)
// Registered later and thus runs before deleting namespaces.
ginkgo.DeferCleanup(f.dumpNamespaceInfo)
// The fact that we need this feels like a bug in ginkgo.
// https://github.com/onsi/ginkgo/v2/issues/222
f.cleanupHandle = AddCleanupAction(f.AfterEach)
@ -320,6 +301,22 @@ func (f *Framework) BeforeEach() {
f.flakeReport = NewFlakeReport()
}
func (f *Framework) dumpNamespaceInfo() {
if !ginkgo.CurrentSpecReport().Failed() {
return
}
if !TestContext.DumpLogsOnFailure {
return
}
ginkgo.By("dump namespace information after failure", func() {
if !f.SkipNamespaceCreation {
for _, ns := range f.namespacesToDelete {
DumpAllNamespaceInfo(f.ClientSet, ns.Name)
}
}
})
}
// printSummaries prints summaries of tests.
func printSummaries(summaries []TestDataSummary, testBaseName string) {
now := time.Now()
@ -357,19 +354,6 @@ func printSummaries(summaries []TestDataSummary, testBaseName string) {
}
}
// AddAfterEach is a way to add a function to be called after every test. The execution order is intentionally random
// to avoid growing dependencies. If you register the same name twice, it is a coding error and will panic.
func (f *Framework) AddAfterEach(name string, fn AfterEachActionFunc) {
if _, ok := f.afterEaches[name]; ok {
panic(fmt.Sprintf("%q is already registered", name))
}
if f.afterEaches == nil {
f.afterEaches = map[string]AfterEachActionFunc{}
}
f.afterEaches[name] = fn
}
// AfterEach deletes the namespace, after reading its events.
func (f *Framework) AfterEach() {
RemoveCleanupAction(f.cleanupHandle)
@ -427,11 +411,6 @@ func (f *Framework) AfterEach() {
}
}()
// run all aftereach functions in random order to ensure no dependencies grow
for _, afterEachFn := range f.afterEaches {
afterEachFn(f, ginkgo.CurrentSpecReport().Failed())
}
if TestContext.GatherKubeSystemResourceUsageData != "false" && TestContext.GatherKubeSystemResourceUsageData != "none" && f.gatherer != nil {
ginkgo.By("Collecting resource usage data")
summary, resourceViolationError := f.gatherer.StopAndSummarize([]int{90, 99, 100}, f.AddonResourceConstraints)

View File

@ -248,14 +248,10 @@ func (t *snapshottableStressTestSuite) DefineTests(driver storageframework.TestD
ginkgo.BeforeEach(func() {
init()
ginkgo.DeferCleanup(cleanup)
createPodsAndVolumes()
})
// See #96177, this is necessary for cleaning up resources when tests are interrupted.
f.AddAfterEach("cleanup", func(f *framework.Framework, failed bool) {
cleanup()
})
ginkgo.It("should support snapshotting of many volumes repeatedly [Slow] [Serial]", func() {
// Repeatedly create and delete snapshots of each volume.
for i := 0; i < stressTest.testOptions.NumPods; i++ {

View File

@ -194,14 +194,10 @@ func (t *volumeStressTestSuite) DefineTests(driver storageframework.TestDriver,
ginkgo.BeforeEach(func() {
init()
ginkgo.DeferCleanup(cleanup)
createPodsAndVolumes()
})
// See #96177, this is necessary for cleaning up resources when tests are interrupted.
f.AddAfterEach("cleanup", func(f *framework.Framework, failed bool) {
cleanup()
})
ginkgo.It("multiple pods should access different volumes repeatedly [Slow] [Serial]", func() {
// Restart pod repeatedly
for i := 0; i < l.testOptions.NumPods; i++ {

View File

@ -23,7 +23,6 @@ import (
"time"
"github.com/davecgh/go-spew/spew"
"github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
@ -129,7 +128,8 @@ func (t *volumePerformanceTestSuite) DefineTests(driver storageframework.TestDri
}
f := framework.NewFramework("volume-lifecycle-performance", frameworkOptions, nil)
f.NamespacePodSecurityEnforceLevel = admissionapi.LevelPrivileged
f.AddAfterEach("cleanup", func(f *framework.Framework, failed bool) {
ginkgo.AfterEach(func() {
ginkgo.By("Closing informer channel")
close(l.stopCh)
ginkgo.By("Deleting all PVCs")