fix the logic for applicationset resources reconcilation when spec.ap…

…plicationset.enabled is false (#1089) * fix the logic for applicationset resources reconciliation when spec.applicationset.enabled is false Signed-off-by: ishitasequeira <[email protected]> Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * fix tests Signed-off-by: ishitasequeira <[email protected]> Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * delete repo server when repo.enabled is set to false Signed-off-by: ishitasequeira <[email protected]> Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * Update status.Phase based on component enabled flag Signed-off-by: ishitasequeira <[email protected]> Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * Added kuttl tests Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * Added namespace creation step Signed-off-by: Raghavi Shirur <[email protected]> Signed-off-by: ishitasequeira <[email protected]> * delete services created for resources Signed-off-by: ishitasequeira <[email protected]> * delete server deployment when enabled flag set to false Signed-off-by: ishitasequeira <[email protected]> * fix e2e test Signed-off-by: ishitasequeira <[email protected]> * fix log message Signed-off-by: ishitasequeira <[email protected]> * revert kuttl test timeout Signed-off-by: ishitasequeira <[email protected]> * Added test for reverse scenario Signed-off-by: Raghavi Shirur <[email protected]> * Dir rename Signed-off-by: Raghavi Shirur <[email protected]> * Added e2e test for ha mode Signed-off-by: Raghavi Shirur <[email protected]> --------- Signed-off-by: ishitasequeira <[email protected]> Signed-off-by: Raghavi Shirur <[email protected]> Co-authored-by: Raghavi Shirur <[email protected]>
argoproj-labs · Dec 1, 2023 · c238af6 · c238af6
1 parent a78a842
commit c238af6
Show file tree

Hide file tree

Showing 39 changed files with 1,000 additions and 27 deletions.
diff --git a/controllers/argocd/applicationset.go b/controllers/argocd/applicationset.go
@@ -110,8 +110,10 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
 
 	podSpec := &deploy.Spec.Template.Spec
 
-	podSpec.ServiceAccountName = sa.ObjectMeta.Name
-
+	// sa would be nil when spec.applicationset.enabled = false
+	if sa != nil {
+		podSpec.ServiceAccountName = sa.ObjectMeta.Name
+	}
 	podSpec.Volumes = []corev1.Volume{
 		{
 			Name: "ssh-known-hosts",
@@ -181,7 +183,7 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
 
 	if existing := newDeploymentWithSuffix("applicationset-controller", "controller", cr); argoutil.IsObjectFound(r.Client, cr.Namespace, existing.Name, existing) {
 
-		if !cr.Spec.ApplicationSet.IsEnabled() {
+		if cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
 			err := r.Client.Delete(context.TODO(), existing)
 			return err
 		}
@@ -425,19 +427,16 @@ func (r *ReconcileArgoCD) reconcileApplicationSetRole(cr *argoproj.ArgoCD) (*v1.
 		if !errors.IsNotFound(err) {
 			return nil, fmt.Errorf("failed to reconcile the role for the service account associated with %s : %s", role.Name, err)
 		}
+		if errors.IsNotFound(err) && cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
+			return nil, nil
+		}
 		if err = controllerutil.SetControllerReference(cr, role, r.Scheme); err != nil {
 			return nil, err
 		}
-		if cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
-			err1 := r.Client.Delete(context.TODO(), role)
-			return nil, err1
-		}
 		return role, r.Client.Create(context.TODO(), role)
 	}
-
 	if cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
-		err := r.Client.Delete(context.TODO(), role)
-		return nil, err
+		return nil, r.Client.Delete(context.TODO(), role)
 	}
 
 	role.Rules = policyRules
@@ -458,17 +457,16 @@ func (r *ReconcileArgoCD) reconcileApplicationSetRoleBinding(cr *argoproj.ArgoCD
 	roleBindingExists := true
 	if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: roleBinding.Name, Namespace: cr.Namespace}, roleBinding); err != nil {
 		if !errors.IsNotFound(err) {
-			if cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
-				return nil
-			}
 			return fmt.Errorf("failed to get the rolebinding associated with %s : %s", name, err)
 		}
+		if errors.IsNotFound(err) && cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
+			return nil
+		}
 		roleBindingExists = false
 	}
 
 	if cr.Spec.ApplicationSet != nil && !cr.Spec.ApplicationSet.IsEnabled() {
-		err := r.Client.Delete(context.TODO(), roleBinding)
-		return err
+		return r.Client.Delete(context.TODO(), roleBinding)
 	}
 
 	setAppSetLabels(&roleBinding.ObjectMeta)
@@ -563,6 +561,7 @@ func (r *ReconcileArgoCD) reconcileApplicationSetService(cr *argoproj.ArgoCD) er
 				return err
 			}
 		}
+		return nil
 	} else {
 		if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
 			return nil // Service found, do nothing

diff --git a/controllers/argocd/applicationset_test.go b/controllers/argocd/applicationset_test.go
@@ -555,6 +555,7 @@ func setProxyEnvVars(t *testing.T) {
 func TestReconcileApplicationSet_Service(t *testing.T) {
 	logf.SetLogger(ZapLogger(true))
 	a := makeTestArgoCD()
+	a.Spec.ApplicationSet = &argoproj.ArgoCDApplicationSet{}
 
 	resObjs := []client.Object{a}
 	subresObjs := []client.Object{a}

diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go
@@ -1128,7 +1128,7 @@ func (r *ReconcileArgoCD) reconcileRepoDeployment(cr *argoproj.ArgoCD, useTLSFor
 		if !cr.Spec.Repo.IsEnabled() {
 			log.Info("Existing ArgoCD Repo Server found but should be disabled. Deleting Repo Server")
 			// Delete existing deployment for ArgoCD Repo Server, if any ..
-			return nil
+			return r.Client.Delete(context.TODO(), existing)
 		}
 
 		changed := false
@@ -1328,7 +1328,7 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
 		if !cr.Spec.Server.IsEnabled() {
 			log.Info("Existing ArgoCD Server found but should be disabled. Deleting ArgoCD Server")
 			// Delete existing deployment for ArgoCD Server, if any ..
-			return nil
+			return r.Client.Delete(context.TODO(), existing)
 		}
 		actualImage := existing.Spec.Template.Spec.Containers[0].Image
 		desiredImage := getArgoContainerImage(cr)
@@ -1375,8 +1375,8 @@ func (r *ReconcileArgoCD) reconcileServerDeployment(cr *argoproj.ArgoCD, useTLSF
 		return nil // Deployment found with nothing to do, move along...
 	}
 
-	if !cr.Spec.Controller.IsEnabled() {
-		log.Info("ArgoCD Repo Server disabled. Skipping starting repo server.")
+	if !cr.Spec.Server.IsEnabled() {
+		log.Info("ArgoCD Server disabled. Skipping starting argocd server.")
 		return nil
 	}
 

diff --git a/controllers/argocd/service.go b/controllers/argocd/service.go
@@ -133,13 +133,13 @@ func (r *ReconcileArgoCD) reconcileRedisHAAnnounceServices(cr *argoproj.ArgoCD)
 	for i := int32(0); i < common.ArgoCDDefaultRedisHAReplicas; i++ {
 		svc := newServiceWithSuffix(fmt.Sprintf("redis-ha-announce-%d", i), "redis", cr)
 		if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
-			if !cr.Spec.HA.Enabled {
+			if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 				return r.Client.Delete(context.TODO(), svc)
 			}
 			return nil // Service found, do nothing
 		}
 
-		if !cr.Spec.HA.Enabled {
+		if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 			return nil //return as Ha is not enabled do nothing
 		}
 
@@ -183,13 +183,13 @@ func (r *ReconcileArgoCD) reconcileRedisHAAnnounceServices(cr *argoproj.ArgoCD)
 func (r *ReconcileArgoCD) reconcileRedisHAMasterService(cr *argoproj.ArgoCD) error {
 	svc := newServiceWithSuffix("redis-ha", "redis", cr)
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
-		if !cr.Spec.HA.Enabled {
+		if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 			return r.Client.Delete(context.TODO(), svc)
 		}
 		return nil // Service found, do nothing
 	}
 
-	if !cr.Spec.HA.Enabled {
+	if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 		return nil //return as Ha is not enabled do nothing
 	}
 
@@ -222,7 +222,7 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyService(cr *argoproj.ArgoCD) erro
 	svc := newServiceWithSuffix("redis-ha-haproxy", "redis", cr)
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
 
-		if !cr.Spec.HA.Enabled {
+		if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 			return r.Client.Delete(context.TODO(), svc)
 		}
 
@@ -232,7 +232,7 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyService(cr *argoproj.ArgoCD) erro
 		return nil // Service found, do nothing
 	}
 
-	if !cr.Spec.HA.Enabled {
+	if !cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 		return nil //return as Ha is not enabled do nothing
 	}
 
@@ -279,6 +279,9 @@ func (r *ReconcileArgoCD) reconcileRedisService(cr *argoproj.ArgoCD) error {
 	svc := newServiceWithSuffix("redis", "redis", cr)
 
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
+		if !cr.Spec.Redis.IsEnabled() {
+			return r.Client.Delete(context.TODO(), svc)
+		}
 		if ensureAutoTLSAnnotation(svc, common.ArgoCDRedisServerTLSSecretName, cr.Spec.Redis.WantsAutoTLS()) {
 			return r.Client.Update(context.TODO(), svc)
 		}
@@ -288,7 +291,7 @@ func (r *ReconcileArgoCD) reconcileRedisService(cr *argoproj.ArgoCD) error {
 		return nil // Service found, do nothing
 	}
 
-	if cr.Spec.HA.Enabled {
+	if cr.Spec.HA.Enabled || !cr.Spec.Redis.IsEnabled() {
 		return nil //return as Ha is enabled do nothing
 	}
 
@@ -358,12 +361,19 @@ func (r *ReconcileArgoCD) reconcileRepoService(cr *argoproj.ArgoCD) error {
 	svc := newServiceWithSuffix("repo-server", "repo-server", cr)
 
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
+		if !cr.Spec.Repo.IsEnabled() {
+			return r.Client.Delete(context.TODO(), svc)
+		}
 		if ensureAutoTLSAnnotation(svc, common.ArgoCDRepoServerTLSSecretName, cr.Spec.Repo.WantsAutoTLS()) {
 			return r.Client.Update(context.TODO(), svc)
 		}
 		return nil // Service found, do nothing
 	}
 
+	if !cr.Spec.Repo.IsEnabled() {
+		return nil
+	}
+
 	ensureAutoTLSAnnotation(svc, common.ArgoCDRepoServerTLSSecretName, cr.Spec.Repo.WantsAutoTLS())
 
 	svc.Spec.Selector = map[string]string{
@@ -420,12 +430,19 @@ func (r *ReconcileArgoCD) reconcileServerMetricsService(cr *argoproj.ArgoCD) err
 func (r *ReconcileArgoCD) reconcileServerService(cr *argoproj.ArgoCD) error {
 	svc := newServiceWithSuffix("server", "server", cr)
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) {
+		if !cr.Spec.Server.IsEnabled() {
+			return r.Client.Delete(context.TODO(), svc)
+		}
 		if ensureAutoTLSAnnotation(svc, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS()) {
 			return r.Client.Update(context.TODO(), svc)
 		}
 		return nil // Service found, do nothing
 	}
 
+	if !cr.Spec.Repo.IsEnabled() {
+		return nil
+	}
+
 	ensureAutoTLSAnnotation(svc, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS())
 
 	svc.Spec.Ports = []corev1.ServicePort{

diff --git a/controllers/argocd/status.go b/controllers/argocd/status.go
@@ -207,7 +207,10 @@ func (r *ReconcileArgoCD) reconcileStatusSSO(cr *argoproj.ArgoCD) error {
 func (r *ReconcileArgoCD) reconcileStatusPhase(cr *argoproj.ArgoCD) error {
 	var phase string
 
-	if cr.Status.ApplicationController == "Running" && cr.Status.Redis == "Running" && cr.Status.Repo == "Running" && cr.Status.Server == "Running" {
+	if ((!cr.Spec.Controller.IsEnabled() && cr.Status.ApplicationController == "Unknown") || cr.Status.ApplicationController == "Running") &&
+		((!cr.Spec.Redis.IsEnabled() && cr.Status.Redis == "Unknown") || cr.Status.Redis == "Running") &&
+		((!cr.Spec.Repo.IsEnabled() && cr.Status.Repo == "Unknown") || cr.Status.Repo == "Running") &&
+		((!cr.Spec.Server.IsEnabled() && cr.Status.Server == "Unknown") || cr.Status.Server == "Running") {
 		phase = "Available"
 	} else {
 		phase = "Pending"

diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-errors.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-errors.yaml
@@ -0,0 +1,63 @@
+apiVersion: argoproj.io/v1beta1
+kind: ArgoCD
+metadata:
+  name: argocd-test
+  namespace: test
+status:
+  phase: Available
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-redis
+  namespace: test
+
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-repo-server
+  namespace: test
+
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-server
+  namespace: test
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test-server
+  namespace: test
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test-application-controller
+  namespace: test
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test-argocd-application-controller
+  namespace: test
+
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: argocd-test-argocd-server
+  namespace: test
+
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: argocd-test-argocd-redis-ha
+  namespace: test
+
diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-install.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-install.yaml
@@ -0,0 +1,22 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: test
+---
+
+apiVersion: argoproj.io/v1beta1
+kind: ArgoCD
+metadata:
+  name: argocd-test
+  namespace: test
+spec:
+  controller:
+    enabled: false
+  redis:
+    enabled: false
+  repo:
+    enabled: false
+  server:
+    enabled: false
+  applicationSet:
+    enabled: false
diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-assert.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-assert.yaml
@@ -0,0 +1,25 @@
+apiVersion: argoproj.io/v1beta1
+kind: ArgoCD
+metadata:
+  name: argocd-test
+  namespace: test
+status:
+  phase: Available
+---
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: argocd-test-application-controller
+  namespace: test
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test-argocd-application-controller
+  namespace: test
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test-argocd-application-controller
+  namespace: test
diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-errors.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-errors.yaml
@@ -0,0 +1,34 @@
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-redis
+  namespace: test
+
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-repo-server
+  namespace: test
+
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test-server
+  namespace: test
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test-server
+  namespace: test
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test-repo-server
+  namespace: test