Make AfterSuite hooks ordered

ginkgo has a weird bug that - AfterEach does not get called when
testsuite exits with certain kind of interrupt (Ctrl-C for example).
More info - https://github.com/onsi/ginkgo/issues/222

We workaround this issue in Kubernetes by adding a special hook into
AfterSuite call, but AfterSuite can not be used to peforms certain
kind of cleanup because it can race with AfterEach hook and
framework.AfterEach hook will set framework.ClientSet to nil.

This presents a problem in cleaning up CSI driver and testpods. This
PR removes cleanup of driver manifest via CleanupAction because that
is not safe and racy (such as f.ClientSet may disappear!) and makes
AfterSuite hooks run in a ordered fashion
This commit is contained in:
Hemant Kumar 2020-04-07 19:19:34 -04:00
parent 32113f6451
commit 708261e06c
2 changed files with 19 additions and 14 deletions

View File

@ -16,22 +16,30 @@ limitations under the License.
package framework
import "sync"
import (
"sync"
)
// CleanupActionHandle is an integer pointer type for handling cleanup action
type CleanupActionHandle *int
type cleanupFuncHandle struct {
actionHandle CleanupActionHandle
actionHook func()
}
var cleanupActionsLock sync.Mutex
var cleanupActions = map[CleanupActionHandle]func(){}
var cleanupHookList = []cleanupFuncHandle{}
// AddCleanupAction installs a function that will be called in the event of the
// whole test being terminated. This allows arbitrary pieces of the overall
// test to hook into SynchronizedAfterSuite().
// The hooks are called in last-in-first-out order.
func AddCleanupAction(fn func()) CleanupActionHandle {
p := CleanupActionHandle(new(int))
cleanupActionsLock.Lock()
defer cleanupActionsLock.Unlock()
cleanupActions[p] = fn
c := cleanupFuncHandle{actionHandle: p, actionHook: fn}
cleanupHookList = append([]cleanupFuncHandle{c}, cleanupHookList...)
return p
}
@ -40,7 +48,12 @@ func AddCleanupAction(fn func()) CleanupActionHandle {
func RemoveCleanupAction(p CleanupActionHandle) {
cleanupActionsLock.Lock()
defer cleanupActionsLock.Unlock()
delete(cleanupActions, p)
for i, item := range cleanupHookList {
if item.actionHandle == p {
cleanupHookList = append(cleanupHookList[:i], cleanupHookList[i+1:]...)
break
}
}
}
// RunCleanupActions runs all functions installed by AddCleanupAction. It does
@ -51,8 +64,8 @@ func RunCleanupActions() {
func() {
cleanupActionsLock.Lock()
defer cleanupActionsLock.Unlock()
for _, fn := range cleanupActions {
list = append(list, fn)
for _, p := range cleanupHookList {
list = append(list, p.actionHook)
}
}()
// Run unlocked.

View File

@ -141,14 +141,7 @@ func PatchItems(f *framework.Framework, items ...interface{}) error {
// - only the latest stable API version for each item is supported
func CreateItems(f *framework.Framework, items ...interface{}) (func(), error) {
var destructors []func() error
var cleanupHandle framework.CleanupActionHandle
cleanup := func() {
if cleanupHandle == nil {
// Already done.
return
}
framework.RemoveCleanupAction(cleanupHandle)
// TODO (?): use same logic as framework.go for determining
// whether we are expected to clean up? This would change the
// meaning of the -delete-namespace and -delete-namespace-on-failure
@ -160,7 +153,6 @@ func CreateItems(f *framework.Framework, items ...interface{}) (func(), error) {
}
}
}
cleanupHandle = framework.AddCleanupAction(cleanup)
var result error
for _, item := range items {