feat: haveACMHub and usage refactor

Return only true when the acm hub is managed by the pattern.
This makes the edge case where there is an externally managed acm and
managed clusters, cleaner. Prevents accidentally removing them during
uninstall.

Author: darkdoc

internal/controller/acm.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"strings"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -30,12 +31,16 @@ import (
 func haveACMHub(r *PatternReconciler) bool {
 	gvrMCH := schema.GroupVersionResource{Group: "operator.open-cluster-management.io", Version: "v1", Resource: "multiclusterhubs"}
 
-	_, err := r.dynamicClient.Resource(gvrMCH).Namespace("open-cluster-management").Get(context.Background(), "multiclusterhub", metav1.GetOptions{})
+	mch, err := r.dynamicClient.Resource(gvrMCH).Namespace("open-cluster-management").Get(context.Background(), "multiclusterhub", metav1.GetOptions{})
 	if err != nil {
 		log.Printf("Error obtaining hub: %s\n", err)
 		return false
 	}
-	return true
+
+	return strings.EqualFold(
+		mch.GetAnnotations()["patterns.gitops.validatedpatterns.io/managed"],
+		"true",
+	)
 }
 
 // listManagedClusters lists all ManagedCluster resources (excluding local-cluster)

internal/controller/acm_test.go

@@ -41,7 +41,7 @@ var _ = Describe("HaveACMHub", func() {
 
 	})
 
-	Context("when the ACM Hub exists", func() {
+	Context("when the ACM Hub exists, and owned by the pattern operator", func() {
 		BeforeEach(func() {
 			hub := &unstructured.Unstructured{
 				Object: map[string]any{
@@ -50,6 +50,9 @@ var _ = Describe("HaveACMHub", func() {
 					"metadata": map[string]any{
 						"name":      "multiclusterhub",
 						"namespace": "open-cluster-management",
+						"annotations": map[string]any{
+							"patterns.gitops.validatedpatterns.io/managed": "true",
+						},
 					},
 				},
 			}
@@ -63,21 +66,30 @@ var _ = Describe("HaveACMHub", func() {
 		})
 	})
 
-	Context("when the ACM Hub does not exist", func() {
+	Context("when the ACM Hub exists, and NOT owned by the pattern operator", func() {
+		BeforeEach(func() {
+			hub := &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "operator.open-cluster-management.io/v1",
+					"kind":       "MultiClusterHub",
+					"metadata": map[string]any{
+						"name":      "multiclusterhub",
+						"namespace": "open-cluster-management",
+					},
+				},
+			}
+			_, err := dynamicClient.Resource(gvrMCH).Namespace("open-cluster-management").Create(context.Background(), hub, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+		})
+
 		It("should return false", func() {
 			result := haveACMHub(patternReconciler)
 			Expect(result).To(BeFalse())
 		})
 	})
 
-	Context("when there is an error listing ConfigMaps", func() {
-		BeforeEach(func() {
-			kubeClient.PrependReactor("list", "configmaps", func(testing.Action) (handled bool, ret runtime.Object, err error) {
-				return true, nil, fmt.Errorf("config map error")
-			})
-		})
-
-		It("should return false and log the error", func() {
+	Context("when the ACM Hub does not exist", func() {
+		It("should return false", func() {
 			result := haveACMHub(patternReconciler)
 			Expect(result).To(BeFalse())
 		})

internal/controller/pattern_controller.go

@@ -721,17 +721,21 @@ func (r *PatternReconciler) deleteHubApps(targetApp, app *argoapi.Application, n
 	// Delete managed clusters (excluding local-cluster)
 	// These must be removed before hub deletion can proceed because ACM won't delete properly if they exist
 	// we do not care about the error, since we might be on a standalone cluster
-	managedClusters, _ := r.listManagedClusters(context.Background())
+	// Only do this if the pattern is in charge of the acm hub
 
-	if len(managedClusters) > 0 {
-		deletedCount, err := r.deleteManagedClusters(context.TODO())
-		if err != nil {
-			return fmt.Errorf("failed to delete managed clusters: %w", err)
-		}
+	if haveACMHub(r) {
+		managedClusters, _ := r.listManagedClusters(context.Background())
+
+		if len(managedClusters) > 0 {
+			deletedCount, err := r.deleteManagedClusters(context.TODO())
+			if err != nil {
+				return fmt.Errorf("failed to delete managed clusters: %w", err)
+			}
 
-		if deletedCount > 0 {
-			log.Printf("Deleted %d managed cluster(s), waiting for them to be fully removed", deletedCount)
-			return fmt.Errorf("deleted %d managed cluster(s), waiting for removal to complete before proceeding with hub deletion", deletedCount)
+			if deletedCount > 0 {
+				log.Printf("Deleted %d managed cluster(s), waiting for them to be fully removed", deletedCount)
+				return fmt.Errorf("deleted %d managed cluster(s), waiting for removal to complete before proceeding with hub deletion", deletedCount)
+			}
 		}
 	}
 
@@ -784,17 +788,9 @@ func (r *PatternReconciler) finalizeObject(instance *api.Pattern) error {
 		// Initialize deletion phase if not set
 		if qualifiedInstance.Status.DeletionPhase == api.InitializeDeletion {
 			log.Printf("Initializing deletion phase")
-			if haveACMHub(r) {
-				if err := r.updateDeletionPhase(qualifiedInstance, api.DeleteSpokeChildApps); err != nil {
-					return err
-				}
-			} else {
-				// There is no acm/spoke, we can directly start cleaning up child apps (from hub)
-				if err := r.updateDeletionPhase(qualifiedInstance, api.DeleteHubChildApps); err != nil {
-					return err
-				}
+			if err := r.updateDeletionPhase(qualifiedInstance, api.DeleteSpokeChildApps); err != nil {
+				return err
 			}
-
 			return fmt.Errorf("initialized deletion phase, requeueing now")
 		}