Fix failing of tests

Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
argoproj-labs · Mar 11, 2025 · 9d83876 · 9d83876
1 parent 2ecc87f
commit 9d83876
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 65 deletions.
diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go
@@ -243,9 +243,22 @@ func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Re
 		return reconcile.Result{}, argocd, err
 	}
 
-	//Handle NamespaceManagement reconciliation
-	if err = r.reconcileNamespaceManagement(argocd); err != nil {
-		return reconcile.Result{}, argocd, err
+	// Handle NamespaceManagement reconciliation and check if Namespace Management is enabled via the Subscription env variable.
+	if isNamespaceManagementEnabled() {
+		if err := r.reconcileNamespaceManagement(argocd); err != nil {
+			return reconcile.Result{}, argocd, err
+		}
+	} else if argocd.Spec.NamespaceManagement != nil {
+		k8sClient, err := initK8sClient()
+		if err != nil {
+			log.Error(err, "Failed to initialize Kubernetes client")
+			return reconcile.Result{}, argocd, err
+		}
+
+		if err := r.handleFeatureDisable(argocd, k8sClient); err != nil {
+			log.Error(err, "Failed to disable NamespaceManagement feature")
+			return reconcile.Result{}, argocd, err
+		}
 	}
 
 	if err := r.reconcileResources(argocd); err != nil {

diff --git a/controllers/argocd/namespaceManagement.go b/controllers/argocd/namespaceManagement.go
@@ -25,19 +25,6 @@ func (r *ReconcileArgoCD) reconcileNamespaceManagement(argocd *argoproj.ArgoCD)
 	var errorMessages []string
 	var managedNamespaces []corev1.Namespace
 
-	// Check if Namespace Management is explicitly enabled via the Subscription env variable.
-	if !isNamespaceManagementEnabled() {
-		k8sClient, err := initK8sClient()
-		if err != nil {
-			log.Error(err, "Failed to initialize Kubernetes client")
-			return err
-		}
-		if err := r.handleFeatureDisable(k8sClient); err != nil {
-			return err
-		}
-		return nil
-	}
-
 	// List all NamespaceManagement CRs
 	nmList := &argoproj.NamespaceManagementList{}
 	if err := r.Client.List(ctx, nmList); err != nil {
@@ -123,7 +110,7 @@ func isNamespaceManagementEnabled() bool {
 
 // If the EnableManagedNamespace feature is disabled, clean up the RBACs associated with the managed namespaces
 // and remove the corresponding fields from the ArgoCD and NamespaceManagement CRs.
-func (r *ReconcileArgoCD) handleFeatureDisable(k8sClient kubernetes.Interface) error {
+func (r *ReconcileArgoCD) handleFeatureDisable(argocd *argoproj.ArgoCD, k8sClient kubernetes.Interface) error {
 	ctx := context.TODO()
 
 	// Check if NamespaceManagement CRs exist and if any ArgoCD instance has namespace management enabled.
@@ -132,57 +119,36 @@ func (r *ReconcileArgoCD) handleFeatureDisable(k8sClient kubernetes.Interface) e
 		return err
 	}
 
-	argoCDList := &argoproj.ArgoCDList{}
-	if err := r.Client.List(ctx, argoCDList); err != nil {
-		return err
-	}
-
-	hasNamespaceManagement := false
-	for _, argoCD := range argoCDList.Items {
-		if argoCD.Spec.NamespaceManagement != nil {
-			hasNamespaceManagement = true
-			break // No need to check further, we found one
-		}
-	}
-
-	// Exit if no NamespaceManagement CRs exist or no ArgoCD instance has namespace management enabled.
-	if len(nsMgmtList.Items) == 0 && !hasNamespaceManagement {
-		log.Info("No NamespaceManagement CRs and ArgoCD instances with namespace management found, skipping feature disable")
-		return nil
-	}
-
-	for _, argoCD := range argoCDList.Items {
-		if argoCD.Spec.NamespaceManagement == nil {
-			continue // Skip ArgoCD instances without namespaceManagement
-		}
-
-		for _, ns := range argoCD.Spec.NamespaceManagement {
-			// First delete the RBACs associated with the namespaces present in .spec.namespaceManagement field
-			if err := deleteRBACsForNamespace(ns.Name, k8sClient); err != nil {
-				log.Error(err, fmt.Sprintf("failed to delete RBACs for namespace: %s", ns.Name))
-			} else {
-				log.Info(fmt.Sprintf("Successfully removed the RBACs for namespace: %s", ns.Name))
-			}
-
-			err := deleteManagedNamespaceFromClusterSecret(argoCD.Namespace, ns.Name, k8sClient)
-			if err != nil {
-				log.Error(err, fmt.Sprintf("unable to delete namespace %s from cluster secret", ns.Name))
-			} else {
-				log.Info(fmt.Sprintf("Successfully deleted namespace %s from cluster secret", ns.Name))
-			}
+	// First delete the RBACs associated with the namespaces present in .spec.namespaceManagement field
+	for _, ns := range argocd.Spec.NamespaceManagement {
+		if err := deleteRBACsForNamespace(ns.Name, k8sClient); err != nil {
+			log.Error(err, fmt.Sprintf("failed to delete RBACs for namespace: %s", ns.Name))
+		} else {
+			log.Info(fmt.Sprintf("Successfully removed the RBACs for namespace: %s", ns.Name))
 		}
 
-		// Remove .spec.namespaceManagement
-		argoCDCopy := argoCD.DeepCopy()
-		argoCDCopy.Spec.NamespaceManagement = nil
-		if err := r.Client.Update(ctx, argoCDCopy); err != nil {
-			log.Error(err, "Failed to update ArgoCD CR", "namespace", argoCD.Namespace)
+		err := deleteManagedNamespaceFromClusterSecret(argocd.Namespace, ns.Name, k8sClient)
+		if err != nil {
+			log.Error(err, fmt.Sprintf("unable to delete namespace %s from cluster secret", ns.Name))
 		} else {
-			log.Info("Removed .spec.namespaceManagement from ArgoCD CR", "namespace", argoCD.Namespace)
+			log.Info(fmt.Sprintf("Successfully deleted namespace %s from cluster secret", ns.Name))
 		}
 	}
 
+	// Remove .spec.namespaceManagement
+	argocdCopy := argocd.DeepCopy()
+	argocdCopy.Spec.NamespaceManagement = nil
+	if err := r.Client.Update(ctx, argocdCopy); err != nil {
+		log.Error(err, "Failed to update argocd CR", "namespace", argocd.Namespace)
+		return err
+	} else {
+		log.Info("Removed .spec.namespaceManagement from argocd CR", "namespace", argocd.Namespace)
+	}
+
 	for _, nsMgmt := range nsMgmtList.Items {
+		if nsMgmt.Spec.ManagedBy != argocd.Namespace {
+			continue
+		}
 		// First delete the RBACs associated with the namespaces present in NamespaceManagement
 		if err := deleteRBACsForNamespace(nsMgmt.Namespace, k8sClient); err != nil {
 			log.Error(err, fmt.Sprintf("failed to delete RBACs for namespace: %s", nsMgmt.Namespace))
@@ -207,8 +173,6 @@ func (r *ReconcileArgoCD) handleFeatureDisable(k8sClient kubernetes.Interface) e
 				log.Info("Removed .spec.managedBy from NamespaceManagement CR", "namespace", nsMgmt.Namespace)
 			}
 		}
-
 	}
-
 	return nil
 }
diff --git a/controllers/argocd/namespaceManagement_test.go b/controllers/argocd/namespaceManagement_test.go
@@ -121,7 +121,7 @@ func TestHandleFeatureDisable_NoNamespaceManagement(t *testing.T) {
 	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
 	r := makeTestReconciler(cl, sch)
 
-	err := r.handleFeatureDisable(testClient)
+	err := r.handleFeatureDisable(a, testClient)
 	// Assert: Should return no error since there are no NamespaceManagement CR and ArgoCD .spec.NamespaceManagement field is nil
 	assert.NoError(t, err)
 }
@@ -156,7 +156,7 @@ func TestHandleFeatureDisable_WithNamespaceManagement(t *testing.T) {
 	err := r.Client.Create(context.TODO(), nm)
 	assert.NoError(t, err)
 
-	err = r.handleFeatureDisable(testClient)
+	err = r.handleFeatureDisable(a, testClient)
 	// Assert: Should return no error and attempt updates
 	assert.NoError(t, err)