Fix bug where ArgoCD removes nodePlacement stanza from configuration (#740)

* Fix bug where ArgoCD removes nodePlacement stanza from configuration

Signed-off-by: Rizwana777 <[email protected]>

* Add unit test for argocd nodeplacement

Signed-off-by: Rizwana777 <[email protected]>

---------

Signed-off-by: Rizwana777 <[email protected]>

Author: Rizwana777

controllers/argocd_metrics_controller_test.go

@@ -29,7 +29,6 @@ import (
 	is "gotest.tools/assert/cmp"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -349,7 +348,7 @@ func TestReconciler_add_dashboard(t *testing.T) {
 
 	// Need to create one configmap to test update existing versus create
 	cm := corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: v1.ObjectMeta{
 			Name:      "gitops-overview",
 			Namespace: dashboardNamespace,
 		},

controllers/gitopsservice_controller.go

@@ -423,7 +423,6 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
 		}
 	} else {
 		changed := false
-
 		if existingArgoCD.Spec.ApplicationSet != nil {
 			if existingArgoCD.Spec.ApplicationSet.Resources == nil {
 				existingArgoCD.Spec.ApplicationSet.Resources = defaultArgoCDInstance.Spec.ApplicationSet.Resources
@@ -472,7 +471,14 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
 			changed = true
 		}
 
-		if !reflect.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) {
+		// if user is patching nodePlacement through GitopsService CR, then existingArgoCD NodePlacement is updated.
+		if defaultArgoCDInstance.Spec.NodePlacement != nil {
+			if !reflect.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) {
+				existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement
+				changed = true
+			}
+			// Handle the case where NodePlacement should be removed
+		} else if existingArgoCD.Spec.NodePlacement != nil {
 			existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement
 			changed = true
 		}

controllers/gitopsservice_controller_test.go

@@ -37,7 +37,6 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	resourcev1 "k8s.io/apimachinery/pkg/api/resource"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -93,6 +92,62 @@ func TestImageFromEnvVariable(t *testing.T) {
 
 }
 
+func TestReconcileDefaultForArgoCDNodeplacement(t *testing.T) {
+	logf.SetLogger(argocd.ZapLogger(true))
+	s := scheme.Scheme
+	addKnownTypesToScheme(s)
+
+	var err error
+
+	gitopsService := &pipelinesv1alpha1.GitopsService{
+		ObjectMeta: v1.ObjectMeta{
+			Name: serviceName,
+		},
+		Spec: pipelinesv1alpha1.GitopsServiceSpec{
+			NodeSelector: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+
+	fakeClient := fake.NewFakeClient(gitopsService)
+	reconciler := newReconcileGitOpsService(fakeClient, s)
+
+	existingArgoCD := &argoapp.ArgoCD{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      serviceNamespace,
+			Namespace: serviceNamespace,
+		},
+		Spec: argoapp.ArgoCDSpec{
+			Server: argoapp.ArgoCDServerSpec{
+				Route: argoapp.ArgoCDRouteSpec{
+					Enabled: true,
+				},
+			},
+			ApplicationSet: &argoapp.ArgoCDApplicationSet{},
+			SSO: &argoapp.ArgoCDSSOSpec{
+				Provider: "dex",
+				Dex: &argoapp.ArgoCDDexSpec{
+					Config: "test-config",
+				},
+			},
+		},
+	}
+
+	err = fakeClient.Create(context.TODO(), existingArgoCD)
+	assertNoError(t, err)
+
+	_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
+	assertNoError(t, err)
+
+	// verify whether existingArgoCD NodePlacement is updated when user is patching nodePlacement through GitopsService CR
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace},
+		existingArgoCD)
+	assertNoError(t, err)
+	assert.Check(t, existingArgoCD.Spec.NodePlacement != nil)
+	assert.DeepEqual(t, existingArgoCD.Spec.NodePlacement.NodeSelector, gitopsService.Spec.NodeSelector)
+}
+
 // If the DISABLE_DEFAULT_ARGOCD_INSTANCE is set, ensure that the default ArgoCD instance is not created.
 func TestReconcileDisableDefault(t *testing.T) {
 
@@ -553,14 +608,14 @@ func TestReconcile_VerifyResourceQuotaDeletionForUpgrade(t *testing.T) {
 
 	// Create namespace object for default ArgoCD instance and set resource quota to it.
 	defaultArgoNS := &corev1.Namespace{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: v1.ObjectMeta{
 			Name: serviceNamespace,
 		},
 	}
 	fakeClient.Create(context.TODO(), defaultArgoNS)
 
 	dummyResourceObj := &corev1.ResourceQuota{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: v1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-compute-resources", serviceNamespace),
 			Namespace: serviceNamespace,
 		},
@@ -613,7 +668,7 @@ func TestReconcile_InfrastructureNode(t *testing.T) {
 	s := scheme.Scheme
 	addKnownTypesToScheme(s)
 	gitopsService := &pipelinesv1alpha1.GitopsService{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: v1.ObjectMeta{
 			Name: serviceName,
 		},
 		Spec: pipelinesv1alpha1.GitopsServiceSpec{