Ensure CleanupActionHandle always completes

The way gingko handles interrupts is:
 - It starts running AfterSuite hooks in a separate goroutine (this includes cleanupAction hooks)
 - Once AfterSuite hook is done executing it calls
   os.Exit(1) on test suite.

So how cleanupFunc() that runs via defer in test can be interrupted
is:
 - cleanupFunc starts running via defer (or AfterEach hook) but first
   thing that function does is to remove cleanupHandle from
   framework.RemoveCleanupAction.
 - Test suite receives interrupt from user and AfterSuite block
   starts executing
 - remember that while cleanupFunc is running in goroutine#1,
   AfterSuite is running concurrently in goroutine#2.
 - AfterSuite hook has bunch of CleanupActions it needs to run which
   were registered via framework.AddCleanupAction(cleanupFunc) but
   once cleanupFunc starts executing via defer in the test, it will
   remove the cleanupHandle from framework's aftersuite hooks.
 - So if AfterSuite did not had anything to run (because
   those actions were removed via framework.RemoveCleanupAction
   then it will simply go to the last framework.AfterEach action and call os.Exit(1)
 - So if os.Exit(1) is called before cleanupFunc has a chance to finish in defer, it will not complete.
This commit is contained in:
Hemant Kumar 2020-06-02 12:40:32 -04:00
parent 3fc7831cd8
commit 74be9f04fa

View File

@ -210,7 +210,6 @@ func (h *hostpathCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.Per
// Cleanup CSI driver and namespaces. This function needs to be idempotent and can be // Cleanup CSI driver and namespaces. This function needs to be idempotent and can be
// concurrently called from defer (or AfterEach) and AfterSuite action hooks. // concurrently called from defer (or AfterEach) and AfterSuite action hooks.
cleanupFunc := func() { cleanupFunc := func() {
framework.RemoveCleanupAction(h.cleanupHandle)
ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1)) ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1))
// Delete the primary namespace but its okay to fail here because this namespace will // Delete the primary namespace but its okay to fail here because this namespace will
// also be deleted by framework.Aftereach hook // also be deleted by framework.Aftereach hook
@ -222,6 +221,12 @@ func (h *hostpathCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.Per
ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2)) ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2))
tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout)) tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout))
// cleanup function has already ran and hence we don't need to run it again.
// We do this as very last action because in-case defer(or AfterEach) races
// with AfterSuite and test routine gets killed then this block still
// runs in AfterSuite
framework.RemoveCleanupAction(h.cleanupHandle)
} }
h.cleanupHandle = framework.AddCleanupAction(cleanupFunc) h.cleanupHandle = framework.AddCleanupAction(cleanupFunc)
@ -404,7 +409,6 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTest
// Cleanup CSI driver and namespaces. This function needs to be idempotent and can be // Cleanup CSI driver and namespaces. This function needs to be idempotent and can be
// concurrently called from defer (or AfterEach) and AfterSuite action hooks. // concurrently called from defer (or AfterEach) and AfterSuite action hooks.
cleanupFunc := func() { cleanupFunc := func() {
framework.RemoveCleanupAction(m.cleanupHandle)
ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1)) ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1))
// Delete the primary namespace but its okay to fail here because this namespace will // Delete the primary namespace but its okay to fail here because this namespace will
// also be deleted by framework.Aftereach hook // also be deleted by framework.Aftereach hook
@ -422,6 +426,12 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTest
tryFunc(cancelLogging) tryFunc(cancelLogging)
ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2)) ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2))
tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout)) tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout))
// cleanup function has already ran and hence we don't need to run it again.
// We do this as very last action because in-case defer(or AfterEach) races
// with AfterSuite and test routine gets killed then this block still
// runs in AfterSuite
framework.RemoveCleanupAction(m.cleanupHandle)
} }
m.cleanupHandle = framework.AddCleanupAction(cleanupFunc) m.cleanupHandle = framework.AddCleanupAction(cleanupFunc)
@ -560,7 +570,6 @@ func (g *gcePDCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTes
// Cleanup CSI driver and namespaces. This function needs to be idempotent and can be // Cleanup CSI driver and namespaces. This function needs to be idempotent and can be
// concurrently called from defer (or AfterEach) and AfterSuite action hooks. // concurrently called from defer (or AfterEach) and AfterSuite action hooks.
cleanupFunc := func() { cleanupFunc := func() {
framework.RemoveCleanupAction(g.cleanupHandle)
ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1)) ginkgo.By(fmt.Sprintf("deleting the test namespace: %s", ns1))
// Delete the primary namespace but its okay to fail here because this namespace will // Delete the primary namespace but its okay to fail here because this namespace will
// also be deleted by framework.Aftereach hook // also be deleted by framework.Aftereach hook
@ -572,6 +581,12 @@ func (g *gcePDCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTes
ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2)) ginkgo.By(fmt.Sprintf("deleting the driver namespace: %s", ns2))
tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout)) tryFunc(deleteNamespaceFunc(f.ClientSet, ns2, framework.DefaultNamespaceDeletionTimeout))
// cleanup function has already ran and hence we don't need to run it again.
// We do this as very last action because in-case defer(or AfterEach) races
// with AfterSuite and test routine gets killed then this block still
// runs in AfterSuite
framework.RemoveCleanupAction(g.cleanupHandle)
} }
g.cleanupHandle = framework.AddCleanupAction(cleanupFunc) g.cleanupHandle = framework.AddCleanupAction(cleanupFunc)