Merge pull request #4086 from vincepri/wait-for-manager

🐛 Test environments should wait for manager before running tests

bootstrap/kubeadm/controllers/suite_test.go

@@ -44,7 +44,7 @@ func TestAPIs(t *testing.T) {
 		[]Reporter{printer.NewlineReporter{}})
 }
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	By("bootstrapping test environment")
 	testEnv = helpers.NewTestEnvironment()
 
@@ -54,7 +54,7 @@ var _ = BeforeSuite(func(done Done) {
 		Expect(testEnv.StartManager(ctx)).To(Succeed())
 	}()
 
-	close(done)
+	<-testEnv.Manager.Elected()
 }, 60)
 
 var _ = AfterSuite(func() {

controllers/remote/cluster_cache_healthcheck_test.go

@@ -65,6 +65,7 @@ var _ = Describe("ClusterCache HealthCheck suite", func() {
 			go func() {
 				Expect(mgr.Start(mgrContext)).To(Succeed())
 			}()
+			<-testEnv.Manager.Elected()
 
 			k8sClient = mgr.GetClient()
 

controllers/remote/cluster_cache_reconciler_test.go

@@ -101,6 +101,7 @@ var _ = Describe("ClusterCache Reconciler suite", func() {
 			go func() {
 				Expect(mgr.Start(mgrContext)).To(Succeed())
 			}()
+			<-testEnv.Manager.Elected()
 
 			k8sClient = mgr.GetClient()
 

controllers/remote/cluster_cache_tracker_test.go

@@ -80,6 +80,7 @@ var _ = Describe("ClusterCache Tracker suite", func() {
 			go func() {
 				Expect(mgr.Start(mgrContext)).To(Succeed())
 			}()
+			<-testEnv.Manager.Elected()
 
 			k8sClient = mgr.GetClient()
 

controllers/remote/suite_test.go

@@ -49,7 +49,7 @@ func TestGinkgoSuite(t *testing.T) {
 		[]Reporter{printer.NewlineReporter{}})
 }
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	By("bootstrapping test environment")
 	testEnv = helpers.NewTestEnvironment()
 
@@ -59,7 +59,7 @@ var _ = BeforeSuite(func(done Done) {
 		Expect(testEnv.StartManager(ctx)).To(Succeed())
 	}()
 
-	close(done)
+	<-testEnv.Manager.Elected()
 }, 60)
 
 var _ = AfterSuite(func() {

controllers/suite_test.go

@@ -107,6 +107,8 @@ func TestMain(m *testing.M) {
 			panic(fmt.Sprintf("Failed to start the envtest manager: %v", err))
 		}
 	}()
+	<-testEnv.Manager.Elected()
+
 	// wait for webhook port to be open prior to running tests
 	testEnv.WaitForWebhooks()
 

controlplane/kubeadm/controllers/suite_test.go

@@ -54,6 +54,8 @@ func TestMain(m *testing.M) {
 			panic(fmt.Sprintf("Failed to start the envtest manager: %v", err))
 		}
 	}()
+	<-testEnv.Manager.Elected()
+
 	// Run tests
 	code := m.Run()
 	// Tearing down the test environment

controlplane/kubeadm/internal/suite_test.go

@@ -37,6 +37,7 @@ func TestMain(m *testing.M) {
 			panic(fmt.Sprintf("Failed to start the envtest manager: %v", err))
 		}
 	}()
+	<-testEnv.Manager.Elected()
 
 	code := m.Run()
 

docs/book/src/developer/testing.md

@@ -7,33 +7,33 @@ feedback and suggestions.
 
 ## Unit tests
 
-Unit tests focus on individual pieces of logic - a single func - and don't require any additional services to execute. They should 
+Unit tests focus on individual pieces of logic - a single func - and don't require any additional services to execute. They should
 be fast and great for getting the first signal on the current implementation, but unit test have the risk that
 to allow integration bugs to slip through.
 
 Historically, in Cluster API unit test were developed using [go test], [gomega] and the [fakeclient]; see the quick reference below.
- 
+
 However, considered some changes introduced in the v0.3.x releases (e.g. ObservedGeneration, Conditions), there is a common
 agreement among Cluster API maintainers that usage [fakeclient] should be progressively deprecated in favor of usage
 of [envtest]; see the quick reference below.
 
 ## Integration tests
 
 Integration tests are focuses on testing the behavior of an entire controller or the interactions between two or
-more Cluster API controllers. 
+more Cluster API controllers.
 
 In older versions of Cluster API, integration test were based on a real cluster and meant to be run in CI only; however,
 now we are considering a different approach base on [envtest] and with one or more controllers configured to run against
 the test cluster.
 
-With this approach it is possible to interact with Cluster API like in a real environment, by creating/updating 
+With this approach it is possible to interact with Cluster API like in a real environment, by creating/updating
 Kubernetes objects and waiting for the controllers to take action.
 
-Please note that while using this mode, as of today, when testing the interactions with an infrastructure provider 
+Please note that while using this mode, as of today, when testing the interactions with an infrastructure provider
 some infrastructure components will be generated, and this could have relevant impacts on test durations (and requirements).
 
-While, as of today this is a strong limitation, in the future we might consider to have a "dry-run" option in CAPD or 
-a fake infrastructure provider to allow test coverage for testing the interactions with an infrastructure provider as well. 
+While, as of today this is a strong limitation, in the future we might consider to have a "dry-run" option in CAPD or
+a fake infrastructure provider to allow test coverage for testing the interactions with an infrastructure provider as well.
 
 ## Running unit and integration tests
 
@@ -76,18 +76,18 @@ Additionally, `test-e2e` target supports the following env variables:
 
 [envtest] is a testing environment that is provided by the [controller-runtime] project. This environment spins up a
 local instance of etcd and the kube-apiserver. This allows test to be executed in an environment very similar to a
-real environment. 
+real environment.
 
 Additionally, in Cluster API there is a set of utilities under [test/helpers] that helps developers in setting up
 a [envtest] ready for Cluster API testing, and most specifically:
 
 - With the required CRDs already pre-configured.
 - With all the Cluster API webhook pre-configured, so there are enforced guarantees about the semantic accuracy
   of the test objects you are going to create.
-  
+
 This is an example of how to create an instance of [envtest] that can be shared across all the tests in a package;
 by convention, this code should be in a file named `suite_test.go`:
- 
+
 ```golang
 var (
 	testEnv *helpers.TestEnvironment
@@ -102,6 +102,8 @@ func TestMain(m *testing.M) {
 			panic(fmt.Sprintf("Failed to start the envtest manager: %v", err))
 		}
 	}()
+	<-testEnv.Manager.Elected()
+
 	// Run tests
 	code := m.Run()
 	// Tearing down the test environment
@@ -113,16 +115,16 @@ func TestMain(m *testing.M) {
 	os.Exit(code)
 }
 ```
-  
+
 Most notably, [envtest] provides not only a real API server to user during test, but it offers the opportunity
 to configure one or more controllers to run against the test cluster; by using this feature it is possible to use
 [envtest] for developing Cluster API integration tests.
- 
+
 ```golang
 func TestMain(m *testing.M) {
 	// Bootstrapping test environment
 	...
-	
+
 	if err := (&MyReconciler{
 		Client:  testEnv,
 		Log:     log.NullLogger{},
@@ -137,15 +139,15 @@ func TestMain(m *testing.M) {
 
 Please note that, because [envtest] uses a real kube-apiserver that is shared across many tests, the developer
 should take care of ensuring each test run in isolation from the others, by:
- 
+
 - Creating objects in separated namespaces.
 - Avoiding object name conflict.
 
 However, developers should be aware that in some ways, the test control plane will behave differently from “real”
-clusters, and that might have an impact on how you write tests. 
+clusters, and that might have an impact on how you write tests.
 
 One common example is garbage collection; because there are no controllers monitoring built-in resources, objects
-do not get deleted, even if an OwnerReference is set up; as a consequence, usually test implements code for cleaning up 
+do not get deleted, even if an OwnerReference is set up; as a consequence, usually test implements code for cleaning up
 created objects.
 
 This is an example of a test implementing those recommendations:
@@ -155,7 +157,7 @@ func TestAFunc(t *testing.T) {
 	g := NewWithT(t)
 	// Generate namespace with a random name starting with ns1; such namespace
 	// will host test objects in isolation from other tests.
-	ns1, err := testEnv.CreateNamespace(ctx, "ns1") 
+	ns1, err := testEnv.CreateNamespace(ctx, "ns1")
 	g.Expect(err).ToNot(HaveOccurred())
 	defer func() {
 		// Cleanup the test namespace
@@ -168,7 +170,7 @@ func TestAFunc(t *testing.T) {
 			Namespace: ns1.Name, // Place test objects in the test namespace
 		},
 	}
-	
+
 	// Actual test code...
 }
 ```
@@ -182,7 +184,7 @@ func TestAFunc(t *testing.T) {
 	g := NewWithT(t)
 	// Generate namespace with a random name starting with ns1; such namespace
 	// will host test objects in isolation from other tests.
-	ns1, err := testEnv.CreateNamespace(ctx, "ns1") 
+	ns1, err := testEnv.CreateNamespace(ctx, "ns1")
 	g.Expect(err).ToNot(HaveOccurred())
 	defer func() {
 		// Cleanup the test namespace
@@ -195,7 +197,7 @@ func TestAFunc(t *testing.T) {
 			Namespace:    ns1.Name, // Place test objects in the test namespace
 		},
 	}
-	
+
 	t.Run("test case 1", func(t *testing.T) {
 		g := NewWithT(t)
 		// Deep copy the object in each test case, so we prevent side effects in case the object changes.
@@ -224,7 +226,7 @@ comes with a set of limitations that could hamper the validity of a test, most n
   like e.g. `creationTimestamp`, `resourceVersion`, `generation`, `uid`
 - API calls does not execute defaulting or validation webhooks, so there are no enforced guarantee about the semantic accuracy
   of the test objects.
-  
+
 Historically, [fakeclient] is widely used in Cluster API, however, given the growing relevance of the above limitations
 with regard to some changes introduced in the v0.3.x releases (e.g. ObservedGeneration, Conditions), there is a common
 agreement among Cluster API maintainers that usage [fakeclient] should be progressively deprecated in favor of usage
@@ -233,23 +235,23 @@ of [envtest].
 ### `ginkgo`
 [Ginkgo] is a Go testing framework built to help you efficiently write expressive and comprehensive tests using Behavior-Driven Development (“BDD”) style.
 
-While [Ginkgo] is widely used in the Kubernetes ecosystem, Cluster API maintainers found the lack of integration with the 
+While [Ginkgo] is widely used in the Kubernetes ecosystem, Cluster API maintainers found the lack of integration with the
 most used golang IDE somehow limiting, mostly because:
 
 - it makes interactive debugging of tests more difficult, since you can't just run the test using the debugger directly
-- it makes it more difficult to only run a subset of tests, since you can't just run or debug individual tests using an IDE, 
+- it makes it more difficult to only run a subset of tests, since you can't just run or debug individual tests using an IDE,
   but you now need to run the tests using `make` or the `ginkgo` command line and override the focus to select individual tests
-  
+
 In Cluster API you MUST use ginkgo only for E2E tests, where it is required to leverage on the support for running specs
 in parallel; in any case, developers MUST NOT use the table driven extension DSL (`DescribeTable`, `Entry` commands)
 which is considered unintuitive.
-  
+
 ### `gomega`
 [Gomega] is a matcher/assertion library. It is usually paired with the Ginkgo BDD test framework, but it can be used with
  other test frameworks too.
- 
+
  More specifically, in order to use Gomega with go test you should
- 
+
  ```golang
  func TestFarmHasCow(t *testing.T) {
      g := NewWithT(t)
@@ -263,7 +265,7 @@ In Cluster API all the test MUST use [Gomega] assertions.
 
 [go test] testing provides support for automated testing of Go packages.
 
-In Cluster API Unit and integration test MUST use [go test]. 
+In Cluster API Unit and integration test MUST use [go test].
 
 [e2e development]: ./e2e.md
 [Ginkgo]: http://onsi.github.io/ginkgo/

exp/addons/controllers/suite_test.go

@@ -47,7 +47,7 @@ func TestAPIs(t *testing.T) {
 		[]Reporter{printer.NewlineReporter{}})
 }
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	By("bootstrapping test environment")
 	testEnv = helpers.NewTestEnvironment()
 	trckr, err := remote.NewClusterCacheTracker(log.NullLogger{}, testEnv.Manager)
@@ -66,7 +66,7 @@ var _ = BeforeSuite(func(done Done) {
 		Expect(testEnv.StartManager(ctx)).To(Succeed())
 	}()
 
-	close(done)
+	<-testEnv.Manager.Elected()
 }, 60)
 
 var _ = AfterSuite(func() {

exp/controllers/suite_test.go

@@ -45,7 +45,7 @@ func TestAPIs(t *testing.T) {
 		[]Reporter{printer.NewlineReporter{}})
 }
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	By("bootstrapping test environment")
 	testEnv = helpers.NewTestEnvironment()
 
@@ -60,7 +60,7 @@ var _ = BeforeSuite(func(done Done) {
 		Expect(testEnv.StartManager(ctx)).To(Succeed())
 	}()
 
-	close(done)
+	<-testEnv.Manager.Elected()
 }, 60)
 
 var _ = AfterSuite(func() {

util/patch/suite_test.go

@@ -49,7 +49,7 @@ func TestPatch(t *testing.T) {
 		[]Reporter{printer.NewlineReporter{}})
 }
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	By("bootstrapping test environment")
 	testEnv = helpers.NewTestEnvironment()
 
@@ -59,7 +59,7 @@ var _ = BeforeSuite(func(done Done) {
 		Expect(testEnv.StartManager(ctx)).To(Succeed())
 	}()
 
-	close(done)
+	<-testEnv.Manager.Elected()
 }, 60)
 
 var _ = AfterSuite(func() {