Add managers cleanup and change Eventually asserting for mutex objects

Author: kurlov

pkg/reconciler/reconciler_test.go

@@ -1512,83 +1512,96 @@ var _ = Describe("Reconciler", func() {
 		var (
 			ctx                   context.Context
 			cancel                context.CancelFunc
-			mgr                   manager.Manager
-			reconciledCRs         []string
-			anotherReconciledCRs  []string
 			matchingLabels        map[string]string
 			anotherMatchingLabels map[string]string
-			labeledObj            *unstructured.Unstructured
-			anotherObj            *unstructured.Unstructured
-			labeledObjKey         types.NamespacedName
-			anotherObjKey         types.NamespacedName
 			mu                    sync.Mutex
+			doneChans             []chan struct{}
 		)
 
 		BeforeEach(func() {
-			ctx, cancel = context.WithCancel(context.Background())
-
-			mu.Lock()
-			reconciledCRs = nil
-			anotherReconciledCRs = nil
-			mu.Unlock()
-
 			matchingLabels = map[string]string{"app": "foo"}
 			anotherMatchingLabels = map[string]string{"app": "bar"}
+			doneChans = nil
+			ctx, cancel = context.WithCancel(context.Background())
 
-			trackingHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error {
-				mu.Lock()
-				defer mu.Unlock()
-				if !slices.Contains(reconciledCRs, obj.GetName()) {
-					reconciledCRs = append(reconciledCRs, obj.GetName())
-				}
-				return nil
-			})
-			mgr = setupManagerWithSelectorAndPostHook(ctx, trackingHook, matchingLabels)
-
-			labeledObj = testutil.BuildTestCR(gvk)
-			labeledObj.SetName("labeled-cr")
-			labeledObj.SetLabels(matchingLabels)
-			labeledObjKey = types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()}
+			cleanupMgr := getManagerOrFail()
+			cleanupClient := cleanupMgr.GetClient()
+			crList := &unstructured.UnstructuredList{}
+			crList.SetGroupVersionKind(gvk)
+			Expect(cleanupClient.List(ctx, crList)).To(Succeed())
+
+			for i := range crList.Items {
+				cr := &crList.Items[i]
+				// Remove all finalizers to allow CR deletion
+				cr.SetFinalizers([]string{})
+				Expect(cleanupClient.Update(ctx, cr)).To(Succeed())
+				Expect(cleanupClient.Delete(ctx, cr)).To(Succeed())
+			}
 
-			anotherObj = testutil.BuildTestCR(gvk)
-			anotherObj.SetName("another-cr")
-			anotherObjKey = types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()}
+			// Wait for all CRs to be deleted
+			Eventually(func() bool {
+				crList := &unstructured.UnstructuredList{}
+				crList.SetGroupVersionKind(gvk)
+				if err := cleanupClient.List(ctx, crList); err != nil {
+					return false
+				}
+				return len(crList.Items) == 0
+			}, "10s", "100ms").Should(BeTrue())
 		})
 
 		AfterEach(func() {
-			By("ensuring the labeled CR is deleted", func() {
-				ensureDeleteCR(ctx, mgr, labeledObjKey, labeledObj)
-			})
-
-			By("ensuring the unlabeled CR is deleted", func() {
-				ensureDeleteCR(ctx, mgr, anotherObjKey, anotherObj)
-			})
-			cancel()
+			// Cancel the context to stop all managers
+			if cancel != nil {
+				cancel()
+			}
+			// Wait for all managers to shut down completely
+			for _, done := range doneChans {
+				select {
+				case <-done:
+					// Manager has shut down
+				case <-time.After(5 * time.Second):
+					// Timeout waiting for manager shutdown
+				}
+			}
 		})
 
 		It("should only reconcile CRs matching the label selector", func() {
+			labeledObj := testutil.BuildTestCR(gvk)
+			labeledObj.SetName("labeled-cr-test1")
+			labeledObj.SetLabels(matchingLabels)
+			labeledObjKey := types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()}
+
+			anotherObj := testutil.BuildTestCR(gvk)
+			anotherObj.SetName("another-cr-test1")
+			anotherObjKey := types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()}
+
+			var reconciledCRs []string
+			postHook := makePostHook(&mu, &reconciledCRs)
+			mgr, done := setupManagerWithSelectorAndPostHook(ctx, postHook, matchingLabels)
+			doneChans = append(doneChans, done)
+
 			By("creating a CR without matching labels", func() {
 				Expect(mgr.GetClient().Create(ctx, anotherObj)).To(Succeed())
 			})
 
 			By("verifying that the labeled reconciler does not reconcile CR without labels", func() {
-				Consistently(func() []string {
+				Consistently(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return reconciledCRs
-				}, "2s", "100ms").Should(BeEmpty())
+					return len(reconciledCRs) == 0
+				}, "2s", "100ms").Should(BeTrue())
 			})
 
 			By("creating a CR with matching labels", func() {
 				Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed())
 			})
 
 			By("verifying only the labeled CR was reconciled", func() {
-				Eventually(func() []string {
+				Eventually(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return reconciledCRs
-				}).Should(HaveExactElements(labeledObjKey.Name))
+					return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name
+				}).Should(BeTrue())
 			})
 
 			By("updating the unlabeled CR to have matching labels", func() {
@@ -1598,43 +1611,53 @@ var _ = Describe("Reconciler", func() {
 			})
 
 			By("verifying that both CRs were reconciled after setting label to the unlabeled CR", func() {
-				Eventually(func() []string {
+				Eventually(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return reconciledCRs
-				}, "10s", "100ms").Should(ContainElements(labeledObjKey.Name, anotherObjKey.Name))
+					return len(reconciledCRs) == 2 &&
+						slices.Contains(reconciledCRs, labeledObjKey.Name) &&
+						slices.Contains(reconciledCRs, anotherObjKey.Name)
+				}, "10s", "100ms").Should(BeTrue())
 			})
 		})
 
 		It("should reconcile CRs independently when using two managers with different label selectors", func() {
-			By("creating another manager with a different label selector", func() {
-				postHook := hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error {
-					mu.Lock()
-					defer mu.Unlock()
-					if !slices.Contains(anotherReconciledCRs, obj.GetName()) {
-						anotherReconciledCRs = append(anotherReconciledCRs, obj.GetName())
-					}
-					return nil
-				})
-				_ = setupManagerWithSelectorAndPostHook(ctx, postHook, anotherMatchingLabels)
-			})
+			labeledObj := testutil.BuildTestCR(gvk)
+			labeledObj.SetName("labeled-cr-test2")
+			labeledObj.SetLabels(matchingLabels)
+			labeledObjKey := types.NamespacedName{Namespace: labeledObj.GetNamespace(), Name: labeledObj.GetName()}
+
+			anotherObj := testutil.BuildTestCR(gvk)
+			anotherObj.SetName("another-cr-test2")
+			anotherObjKey := types.NamespacedName{Namespace: anotherObj.GetNamespace(), Name: anotherObj.GetName()}
+
+			var reconciledCRs []string
+			var anotherReconciledCRs []string
+
+			postHook := makePostHook(&mu, &reconciledCRs)
+			mgr, done := setupManagerWithSelectorAndPostHook(ctx, postHook, matchingLabels)
+			doneChans = append(doneChans, done)
+
+			postHook2 := makePostHook(&mu, &anotherReconciledCRs)
+			_, done2 := setupManagerWithSelectorAndPostHook(ctx, postHook2, anotherMatchingLabels)
+			doneChans = append(doneChans, done2)
 
 			By("creating a CR with matching labels for the first manager", func() {
 				Expect(mgr.GetClient().Create(ctx, labeledObj)).To(Succeed())
 			})
 
 			By("verifying that only the first manager reconciled the CR", func() {
-				Eventually(func() []string {
+				Eventually(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return reconciledCRs
-				}, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name))
+					return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name
+				}, "10s", "100ms").Should(BeTrue())
 
-				Consistently(func() []string {
+				Consistently(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return anotherReconciledCRs
-				}, "2s", "100ms").Should(BeEmpty())
+					return len(anotherReconciledCRs) == 0
+				}, "2s", "100ms").Should(BeTrue())
 			})
 
 			By("creating a CR with matching labels for the second manager", func() {
@@ -1645,17 +1668,17 @@ var _ = Describe("Reconciler", func() {
 			})
 
 			By("verifying that both managers reconcile only matching labels CRs", func() {
-				Eventually(func() []string {
+				Eventually(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return reconciledCRs
-				}, "10s", "100ms").Should(HaveExactElements(labeledObjKey.Name))
+					return len(reconciledCRs) == 1 && reconciledCRs[0] == labeledObjKey.Name
+				}, "10s", "100ms").Should(BeTrue())
 
-				Eventually(func() []string {
+				Eventually(func() bool {
 					mu.Lock()
 					defer mu.Unlock()
-					return anotherReconciledCRs
-				}, "10s", "100ms").Should(HaveExactElements(anotherObjKey.Name))
+					return len(anotherReconciledCRs) == 1 && anotherReconciledCRs[0] == anotherObjKey.Name
+				}, "10s", "100ms").Should(BeTrue())
 			})
 		})
 	})
@@ -1859,18 +1882,18 @@ func verifyEvent(ctx context.Context, cl client.Reader, obj metav1.Object, event
 	Message: %q`, eventType, reason, message))
 }
 
-func ensureDeleteCR(ctx context.Context, mgr manager.Manager, crKey types.NamespacedName, cr *unstructured.Unstructured) {
-	err := mgr.GetAPIReader().Get(ctx, crKey, cr)
-	if apierrors.IsNotFound(err) {
-		return
-	}
-	Expect(err).ToNot(HaveOccurred())
-	cr.SetFinalizers([]string{})
-	Expect(mgr.GetClient().Update(ctx, cr)).To(Succeed())
-	Expect(mgr.GetClient().Delete(ctx, cr)).To(Succeed())
+func makePostHook(mu *sync.Mutex, reconciledCRs *[]string) hook.PostHook {
+	return hook.PostHookFunc(func(obj *unstructured.Unstructured, _ release.Release, _ logr.Logger) error {
+		mu.Lock()
+		defer mu.Unlock()
+		if !slices.Contains(*reconciledCRs, obj.GetName()) {
+			*reconciledCRs = append(*reconciledCRs, obj.GetName())
+		}
+		return nil
+	})
 }
 
-func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) manager.Manager {
+func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.PostHook, matchingLabels map[string]string) (manager.Manager, chan struct{}) {
 	mgr := getManagerOrFail()
 	r, err := New(
 		WithGroupVersionKind(gvk),
@@ -1880,9 +1903,12 @@ func setupManagerWithSelectorAndPostHook(ctx context.Context, postHook hook.Post
 	)
 	Expect(err).ToNot(HaveOccurred())
 	Expect(r.SetupWithManager(mgr)).To(Succeed())
+
+	done := make(chan struct{})
 	go func() {
+		defer close(done)
 		Expect(mgr.Start(ctx)).To(Succeed())
 	}()
 	Expect(mgr.GetCache().WaitForCacheSync(ctx)).To(BeTrue())
-	return mgr
+	return mgr, done
 }