From fee15d54054ecc16e72131f632323f4451f061d9 Mon Sep 17 00:00:00 2001 From: John Sanda Date: Wed, 7 Apr 2021 23:06:10 -0400 Subject: [PATCH] use client.New for tests to get consistent reads --- .../cassandradatacenter_controller_test.go | 36 +++---- controllers/reaper_controller_test.go | 101 +++++++++++------- controllers/suite_test.go | 7 +- 3 files changed, 85 insertions(+), 59 deletions(-) diff --git a/controllers/cassandradatacenter_controller_test.go b/controllers/cassandradatacenter_controller_test.go index 72b02e8..7130fb3 100644 --- a/controllers/cassandradatacenter_controller_test.go +++ b/controllers/cassandradatacenter_controller_test.go @@ -34,9 +34,9 @@ var _ = Describe("CassandraDatacenter controller testing", func() { Name: ControllerTestNamespace, }, } - Expect(k8sClient.Create(context.Background(), testNamespace)).Should(Succeed()) + Expect(testClient.Create(context.Background(), testNamespace)).Should(Succeed()) reaper := createReaper(ControllerTestNamespace) - Expect(k8sClient.Create(context.Background(), reaper)).Should(Succeed()) + Expect(testClient.Create(context.Background(), reaper)).Should(Succeed()) cassdcKey := types.NamespacedName{ Name: CassandraControllerDatacenterName, @@ -56,17 +56,17 @@ var _ = Describe("CassandraDatacenter controller testing", func() { Size: 3, }, } - Expect(k8sClient.Create(context.Background(), testDc)).Should(Succeed()) + Expect(testClient.Create(context.Background(), testDc)).Should(Succeed()) Eventually(func() bool { result := &cassdcapi.CassandraDatacenter{} - _ = k8sClient.Get(context.Background(), cassdcKey, result) + _ = testClient.Get(context.Background(), cassdcKey, result) return result.Spec.Size == testDc.Spec.Size }, timeout, interval).Should(BeTrue()) By("making Reaper ready") patch := client.MergeFrom(reaper.DeepCopy()) reaper.Status.Ready = true - k8sClient.Status().Patch(context.Background(), reaper, patch) + testClient.Status().Patch(context.Background(), reaper, patch) key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} verifyReaperReady(key) @@ -77,7 +77,7 @@ var _ = Describe("CassandraDatacenter controller testing", func() { } fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err != nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err != nil { return false } return len(fetchedReaper.Status.Clusters) == 0 @@ -88,10 +88,10 @@ var _ = Describe("CassandraDatacenter controller testing", func() { testDc.Annotations = map[string]string{ "reaper.cassandra-reaper.io/instance": "notHere", } - Expect(k8sClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) + Expect(testClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) Eventually(func() bool { result := &cassdcapi.CassandraDatacenter{} - _ = k8sClient.Get(context.Background(), cassdcKey, result) + _ = testClient.Get(context.Background(), cassdcKey, result) v, _ := result.Annotations["reaper.cassandra-reaper.io/instance"] return v == "notHere" }).Should(BeTrue()) @@ -104,7 +104,7 @@ var _ = Describe("CassandraDatacenter controller testing", func() { key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err != nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err != nil { return false } return len(fetchedReaper.Status.Clusters) == 0 @@ -116,10 +116,10 @@ var _ = Describe("CassandraDatacenter controller testing", func() { testDc.Annotations = map[string]string{ "reaper.cassandra-reaper.io/instance": reaper.Name, } - Expect(k8sClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) + Expect(testClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) Eventually(func() bool { result := &cassdcapi.CassandraDatacenter{} - _ = k8sClient.Get(context.Background(), cassdcKey, result) + _ = testClient.Get(context.Background(), cassdcKey, result) v, _ := result.Annotations["reaper.cassandra-reaper.io/instance"] return v == reaper.Name }, timeout, interval).Should(BeTrue()) @@ -128,7 +128,7 @@ var _ = Describe("CassandraDatacenter controller testing", func() { Consistently(func() bool { key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err != nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err != nil { return false } if len(fetchedReaper.Status.Clusters) == 0 && len(reaperManager.ClustersManaged) == 0 { @@ -143,7 +143,7 @@ var _ = Describe("CassandraDatacenter controller testing", func() { Eventually(func() bool { key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err != nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err != nil { return false } for _, v := range fetchedReaper.Status.Clusters { @@ -162,15 +162,15 @@ var _ = Describe("CassandraDatacenter controller testing", func() { By("ensuring cluster status is re-added even if removed") currentReaper := &api.Reaper{} - Expect(k8sClient.Get(context.Background(), key, currentReaper)).Should(Succeed()) + Expect(testClient.Get(context.Background(), key, currentReaper)).Should(Succeed()) patchClusters := client.MergeFrom(currentReaper.DeepCopy()) currentReaper.Status.Clusters = make([]string, 0) - Expect(k8sClient.Status().Patch(context.Background(), currentReaper, patchClusters)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), currentReaper, patchClusters)).Should(Succeed()) Eventually(func() bool { key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err == nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err == nil { if len(fetchedReaper.Status.Clusters) == 0 { return true } @@ -181,13 +181,13 @@ var _ = Describe("CassandraDatacenter controller testing", func() { // Add unnecessary stuff to CassandraDatacenter to trigger reconcile testDcPatch = client.MergeFrom(testDc.DeepCopy()) testDc.Annotations["useless/update"] = "true" - Expect(k8sClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) + Expect(testClient.Patch(context.Background(), testDc, testDcPatch)).Should(Succeed()) // Wait for the cluster to be re-added Eventually(func() bool { key := types.NamespacedName{Namespace: reaper.Namespace, Name: ReaperName} fetchedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, fetchedReaper); err != nil { + if err := testClient.Get(context.Background(), key, fetchedReaper); err != nil { return false } for _, v := range fetchedReaper.Status.Clusters { diff --git a/controllers/reaper_controller_test.go b/controllers/reaper_controller_test.go index f405236..3a282f6 100644 --- a/controllers/reaper_controller_test.go +++ b/controllers/reaper_controller_test.go @@ -40,7 +40,7 @@ var _ = Describe("Reaper controller", func() { Name: ReaperNamespace, }, } - Expect(k8sClient.Create(context.Background(), testNamespace)).Should(Succeed()) + Expect(testClient.Create(context.Background(), testNamespace)).Should(Succeed()) i = i + 1 cassdcKey := types.NamespacedName{Name: CassandraDatacenterName, Namespace: ReaperNamespace} @@ -57,7 +57,7 @@ var _ = Describe("Reaper controller", func() { }, Status: cassdcapi.CassandraDatacenterStatus{}, } - Expect(k8sClient.Create(context.Background(), testDc)).Should(Succeed()) + Expect(testClient.Create(context.Background(), testDc)).Should(Succeed()) patchCassdc := client.MergeFrom(testDc.DeepCopy()) testDc.Status.CassandraOperatorProgress = cassdcapi.ProgressReady @@ -69,9 +69,9 @@ var _ = Describe("Reaper controller", func() { } cassdc := &cassdcapi.CassandraDatacenter{} - Expect(k8sClient.Status().Patch(context.Background(), testDc, patchCassdc)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), testDc, patchCassdc)).Should(Succeed()) Eventually(func() bool { - err := k8sClient.Get(context.Background(), cassdcKey, cassdc) + err := testClient.Get(context.Background(), cassdcKey, cassdc) if err != nil { return false } @@ -82,14 +82,14 @@ var _ = Describe("Reaper controller", func() { Specify("create a new Reaper instance", func() { By("create the Reaper object") reaper := createReaper(ReaperNamespace) - Expect(k8sClient.Create(context.Background(), reaper)).Should(Succeed()) + Expect(testClient.Create(context.Background(), reaper)).Should(Succeed()) By("check that the service is created") serviceKey := types.NamespacedName{Namespace: ReaperNamespace, Name: reconcile.GetServiceName(reaper.Name)} service := &corev1.Service{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), serviceKey, service) + err := testClient.Get(context.Background(), serviceKey, service) if err != nil { return false } @@ -104,7 +104,7 @@ var _ = Describe("Reaper controller", func() { job := &v1batch.Job{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), jobKey, job) + err := testClient.Get(context.Background(), jobKey, job) if err != nil { return false } @@ -117,7 +117,7 @@ var _ = Describe("Reaper controller", func() { Type: v1batch.JobComplete, Status: corev1.ConditionTrue, }) - Expect(k8sClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) Expect(len(job.OwnerReferences)).Should(Equal(1)) Expect(job.OwnerReferences[0].UID).Should(Equal(reaper.GetUID())) @@ -127,7 +127,7 @@ var _ = Describe("Reaper controller", func() { deployment := &appsv1.Deployment{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), deploymentKey, deployment) + err := testClient.Get(context.Background(), deploymentKey, deployment) if err != nil { return false } @@ -140,7 +140,8 @@ var _ = Describe("Reaper controller", func() { By("update deployment to be ready") patchDeploymentStatus(deployment, 1, 1) - verifyReaperReady(types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName}) + reaperKey := types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName} + verifyReaperReady(reaperKey) // Now simulate the Reaper app entering a state in which its readiness probe fails. This // should cause the deployment to have its status updated. The Reaper object's .Status.Ready @@ -148,16 +149,7 @@ var _ = Describe("Reaper controller", func() { By("update deployment to be not ready") patchDeploymentStatus(deployment, 1, 0) - reaperKey := types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName} - updatedReaper := &api.Reaper{} - Eventually(func() bool { - err := k8sClient.Get(context.Background(), reaperKey, updatedReaper) - if err != nil { - return false - } - ctrl.Log.WithName("test").Info("after update", "updatedReaper", updatedReaper) - return updatedReaper.Status.Ready == false - }, timeout, interval).Should(BeTrue(), "reaper status should have been updated") + verifyReaperNotReady(reaperKey) }) Specify("create a new Reaper instance when objects exist", func() { @@ -185,7 +177,7 @@ var _ = Describe("Reaper controller", func() { }, }, } - Expect(k8sClient.Create(context.Background(), service)).Should(Succeed()) + Expect(testClient.Create(context.Background(), service)).Should(Succeed()) By("create the schema job") jobKey := types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName + "-schema"} @@ -211,7 +203,7 @@ var _ = Describe("Reaper controller", func() { }, }, } - Expect(k8sClient.Create(context.Background(), job)).Should(Succeed()) + Expect(testClient.Create(context.Background(), job)).Should(Succeed()) // We need to mock the job completion in order for the deployment to get created jobPatch := client.MergeFrom(job.DeepCopy()) @@ -219,7 +211,7 @@ var _ = Describe("Reaper controller", func() { Type: v1batch.JobComplete, Status: corev1.ConditionTrue, }) - Expect(k8sClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) By("create the deployment") // We can use a fake deployment here with only the required properties set. Since the deployment @@ -264,7 +256,7 @@ var _ = Describe("Reaper controller", func() { }, }, } - Expect(k8sClient.Create(context.Background(), deployment)).Should(Succeed()) + Expect(testClient.Create(context.Background(), deployment)).Should(Succeed()) // We need to mock the deployment being ready in order for Reaper status to be updated By("update deployment to be ready") @@ -272,7 +264,7 @@ var _ = Describe("Reaper controller", func() { By("create the Reaper object") reaper := createReaper(ReaperNamespace) - Expect(k8sClient.Create(context.Background(), reaper)).Should(Succeed()) + Expect(testClient.Create(context.Background(), reaper)).Should(Succeed()) verifyReaperReady(types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName}) }) @@ -283,7 +275,7 @@ var _ = Describe("Reaper controller", func() { reaper.Spec.ServerConfig.AutoScheduling = &api.AutoScheduler{ Enabled: true, } - Expect(k8sClient.Create(context.Background(), reaper)).Should(Succeed()) + Expect(testClient.Create(context.Background(), reaper)).Should(Succeed()) // Remove unnecessary parts, verify that the deployment has envVar set for autoscheduling By("check that the schema job is created") @@ -291,7 +283,7 @@ var _ = Describe("Reaper controller", func() { job := &v1batch.Job{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), jobKey, job) + err := testClient.Get(context.Background(), jobKey, job) if err != nil { return false } @@ -304,14 +296,14 @@ var _ = Describe("Reaper controller", func() { Type: v1batch.JobComplete, Status: corev1.ConditionTrue, }) - Expect(k8sClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) By("check that the deployment is created") deploymentKey := types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName} deployment := &appsv1.Deployment{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), deploymentKey, deployment) + err := testClient.Get(context.Background(), deploymentKey, deployment) if err != nil { return false } @@ -342,19 +334,19 @@ var _ = Describe("Reaper controller", func() { "password": []byte("james"), }, } - Expect(k8sClient.Create(context.Background(), &secret)).Should(Succeed()) + Expect(testClient.Create(context.Background(), &secret)).Should(Succeed()) By("create the Reaper object and modify it") reaper := createReaper(ReaperNamespace) reaper.Spec.ServerConfig.CassandraBackend.CassandraUserSecretName = "top-secret-cass" - Expect(k8sClient.Create(context.Background(), reaper)).Should(Succeed()) + Expect(testClient.Create(context.Background(), reaper)).Should(Succeed()) By("check that the schema job is created") jobKey := types.NamespacedName{Namespace: reaper.Namespace, Name: reaper.Name + "-schema"} job := &v1batch.Job{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), jobKey, job) + err := testClient.Get(context.Background(), jobKey, job) if err != nil { return false } @@ -376,14 +368,14 @@ var _ = Describe("Reaper controller", func() { Type: v1batch.JobComplete, Status: corev1.ConditionTrue, }) - Expect(k8sClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), job, jobPatch)).Should(Succeed()) By("check that the deployment is created") deploymentKey := types.NamespacedName{Namespace: ReaperNamespace, Name: ReaperName} deployment := &appsv1.Deployment{} Eventually(func() bool { - err := k8sClient.Get(context.Background(), deploymentKey, deployment) + err := testClient.Get(context.Background(), deploymentKey, deployment) if err != nil { return false } @@ -423,21 +415,54 @@ func createReaper(namespace string) *api.Reaper { } } +type ReaperConditionFunc func(reaper *api.Reaper) bool + func verifyReaperReady(key types.NamespacedName) { By("check that the reaper is ready") + waitForReaperCondition(key, func(reaper *api.Reaper) bool { + return reaper.Status.Ready + }) + //Eventually(func() bool { + // updatedReaper := &api.Reaper{} + // if err := testClient.Get(context.Background(), key, updatedReaper); err != nil { + // return false + // } + // ctrl.Log.WithName("test").Info("after update", "updatedReaper", updatedReaper) + // return updatedReaper.Status.Ready + //}, timeout, interval).Should(BeTrue()) +} + +func verifyReaperNotReady(key types.NamespacedName) { + By("check that the reaper is not ready") + waitForReaperCondition(key, func(reaper *api.Reaper) bool { + return !reaper.Status.Ready + }) + //Eventually(func() bool { + // updatedReaper := &api.Reaper{} + // err := testClient.Get(context.Background(), key, updatedReaper) + // if err != nil { + // return false + // } + // ctrl.Log.WithName("test").Info("after update", "updatedReaper", updatedReaper) + // return updatedReaper.Status.Ready == false + //}, timeout, interval).Should(BeTrue(), "reaper status should have been updated") +} + +func waitForReaperCondition(key types.NamespacedName, condition ReaperConditionFunc) { Eventually(func() bool { updatedReaper := &api.Reaper{} - if err := k8sClient.Get(context.Background(), key, updatedReaper); err != nil { + err := testClient.Get(context.Background(), key, updatedReaper) + if err != nil { return false } ctrl.Log.WithName("test").Info("after update", "updatedReaper", updatedReaper) - return updatedReaper.Status.Ready - }, timeout, interval).Should(BeTrue()) + return condition(updatedReaper) + }, timeout, interval).Should(BeTrue(), "reaper status should have been updated") } func patchDeploymentStatus(deployment *appsv1.Deployment, replicas, readyReplicas int32) { deploymentPatch := client.MergeFrom(deployment.DeepCopy()) deployment.Status.Replicas = replicas deployment.Status.ReadyReplicas = readyReplicas - Expect(k8sClient.Status().Patch(context.Background(), deployment, deploymentPatch)).Should(Succeed()) + Expect(testClient.Status().Patch(context.Background(), deployment, deploymentPatch)).Should(Succeed()) } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 9a8241d..d865edd 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -45,7 +45,7 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var cfg *rest.Config -var k8sClient client.Client +var testClient client.Client var testEnv *envtest.Environment var reaperManager *TestReaperManager @@ -115,8 +115,9 @@ var _ = BeforeSuite(func(done Done) { Expect(err).ToNot(HaveOccurred()) }() - k8sClient = k8sManager.GetClient() - Expect(k8sClient).ToNot(BeNil()) + testClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).To(BeNil()) + Expect(testClient).ToNot(BeNil()) close(done) }, 60)