test: Deflake deprovisioning testing (#203)

* Deflake deprovisioning testing

* Wait for verify action before completing test

Makefile

@@ -9,6 +9,7 @@ test: ## Run tests
 	go test ./... \
 		-race \
 		--ginkgo.focus="${FOCUS}" \
+		--ginkgo.v \
 		-cover -coverprofile=coverage.out -outputdir=. -coverpkg=./...
 
 deflake: ## Run randomized, racing tests until the test fails to catch flakes

hack/toolchain.sh

@@ -20,6 +20,7 @@ tools() {
     go install github.com/sigstore/cosign/cmd/cosign@v1.10.0
     go install github.com/gohugoio/hugo@v0.97.3+extended
     go install golang.org/x/vuln/cmd/govulncheck@v0.0.0-20220902211423-27dd78d2ca39
+    go install github.com/onsi/ginkgo/v2/ginkgo@latest
 
     if ! echo "$PATH" | grep -q "${GOPATH:-undefined}/bin\|$HOME/go/bin"; then
         echo "Go workspace's \"bin\" directory is not in PATH. Run 'export PATH=\"\$PATH:\${GOPATH:-\$HOME/go}/bin\"'."

pkg/controllers/deprovisioning/drift_test.go

@@ -15,6 +15,7 @@ limitations under the License.
 package deprovisioning_test
 
 import (
+	"sync"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -61,9 +62,11 @@ var _ = Describe("Drift", func() {
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node))
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))
 		ExpectExists(ctx, env.Client, node)
@@ -93,9 +96,12 @@ var _ = Describe("Drift", func() {
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node))
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))
 		ExpectExists(ctx, env.Client, node)
@@ -154,9 +160,11 @@ var _ = Describe("Drift", func() {
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node))
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		// we don't need a new node, but we should evict everything off one of node2 which only has a single pod
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))
@@ -207,9 +215,11 @@ var _ = Describe("Drift", func() {
 		Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(node), node)).To(Succeed())
 
 		// deprovisioning won't delete the old node until the new node is ready
-		wg := ExpectMakeNewNodesReady(ctx, env.Client, 1, node)
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
+		ExpectMakeNewNodesReady(ctx, env.Client, &wg, 1, node)
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
 		wg.Wait()
@@ -299,9 +309,11 @@ var _ = Describe("Drift", func() {
 		Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(node), node)).To(Succeed())
 
 		// deprovisioning won't delete the old node until the new node is ready
-		wg := ExpectMakeNewNodesReady(ctx, env.Client, 3, node)
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
+		ExpectMakeNewNodesReady(ctx, env.Client, &wg, 3, node)
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
 		wg.Wait()
@@ -344,10 +356,13 @@ var _ = Describe("Drift", func() {
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node1))
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node2))
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		// we don't need a new node, but we should evict everything off one of node2 which only has a single pod
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))

pkg/controllers/deprovisioning/expiration_test.go

@@ -15,6 +15,7 @@ limitations under the License.
 package deprovisioning_test
 
 import (
+	"sync"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -86,10 +87,14 @@ var _ = Describe("Expiration", func() {
 
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(node))
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		// we don't need a new node, but we should evict everything off one of node2 which only has a single pod
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))
@@ -130,10 +135,13 @@ var _ = Describe("Expiration", func() {
 		// inform cluster state about the nodes
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(nodeToExpire))
 		ExpectReconcileSucceeded(ctx, nodeStateController, client.ObjectKeyFromObject(nodeNotExpire))
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
+		wg.Wait()
 
 		// we don't need a new node, but we should evict everything off one of node2 which only has a single pod
 		Expect(cloudProvider.CreateCalls).To(HaveLen(0))
@@ -183,9 +191,11 @@ var _ = Describe("Expiration", func() {
 		Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(node), node)).To(Succeed())
 
 		// deprovisioning won't delete the old node until the new node is ready
-		wg := ExpectMakeNewNodesReady(ctx, env.Client, 1, node)
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
+		ExpectMakeNewNodesReady(ctx, env.Client, &wg, 1, node)
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
 		wg.Wait()
@@ -275,9 +285,11 @@ var _ = Describe("Expiration", func() {
 		Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(node), node)).To(Succeed())
 
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).To(HaveOccurred())
+		wg.Wait()
 
 		// Expiration should try to make 3 calls but fail for the third.
 		Expect(cloudProvider.CreateCalls).To(HaveLen(3))
@@ -365,9 +377,11 @@ var _ = Describe("Expiration", func() {
 		Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(node), node)).To(Succeed())
 
 		// deprovisioning won't delete the old node until the new node is ready
-		wg := ExpectMakeNewNodesReady(ctx, env.Client, 3, node)
+		var wg sync.WaitGroup
+		ExpectTriggerVerifyAction(&wg)
+		ExpectMakeNewNodesReady(ctx, env.Client, &wg, 3, node)
+
 		fakeClock.Step(10 * time.Minute)
-		go triggerVerifyAction()
 		_, err := deprovisioningController.Reconcile(ctx, reconcile.Request{})
 		Expect(err).ToNot(HaveOccurred())
 		wg.Wait()