Feat: RunOnInfra to use openshift.io/node-selector annotation on namespace

common/common.go

@@ -37,6 +37,10 @@ const (
 	DefaultDynamicPluginStartOCPVersion = "4.15.0"
 	// ImagePullPolicyEnvVar is the environment variable for configuring image pull policy
 	ImagePullPolicy = "IMAGE_PULL_POLICY"
+	// InfraNodeSelectorAnnotation is the OpenShift namespace annotation that applies a default node selector to all pods
+	InfraNodeSelectorAnnotation = "openshift.io/node-selector"
+	// InfraNodeSelectorAnnotationValue is the value for the infra node selector annotation
+	InfraNodeSelectorAnnotationValue = "node-role.kubernetes.io/infra="
 )
 
 // InfraNodeSelector returns openshift label for infrastructure nodes

controllers/consoleplugin.go

@@ -347,9 +347,6 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
 
 	newPluginDeployment.Spec.Template.Spec.NodeSelector = argocommon.DefaultNodeSelector()
 
-	if cr.Spec.RunOnInfra {
-		newPluginDeployment.Spec.Template.Spec.NodeSelector[common.InfraNodeLabelSelector] = ""
-	}
 	if len(cr.Spec.NodeSelector) > 0 {
 		newPluginDeployment.Spec.Template.Spec.NodeSelector = argocdutil.AppendStringMap(newPluginDeployment.Spec.Template.Spec.NodeSelector, cr.Spec.NodeSelector)
 	}

controllers/consoleplugin_test.go

@@ -2,7 +2,6 @@ package controllers
 
 import (
 	"context"
-	"maps"
 	"testing"
 
 	argocommon "github.com/argoproj-labs/argocd-operator/common"
@@ -959,7 +958,7 @@ func TestPlugin_reconcileDeployment_changedDNSPolicy(t *testing.T) {
 	}
 }
 
-func TestPlugin_reconcileDeployment_changedInfraNodeSelector(t *testing.T) {
+func TestPlugin_reconcileDeployment_infraNodeSelectorNotInPodSpec(t *testing.T) {
 
 	gitopsService := &pipelinesv1alpha1.GitopsService{
 		ObjectMeta: metav1.ObjectMeta{
@@ -983,9 +982,8 @@ func TestPlugin_reconcileDeployment_changedInfraNodeSelector(t *testing.T) {
 	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: gitopsPluginName, Namespace: serviceNamespace}, deployment)
 	assertNoError(t, err)
 
-	nSelector := common.InfraNodeSelector()
-	maps.Copy(nSelector, argocommon.DefaultNodeSelector())
-	assert.DeepEqual(t, deployment.Spec.Template.Spec.NodeSelector, nSelector)
+	// RunOnInfra no longer sets infra NodeSelector on pod spec — namespace annotation handles it
+	assert.DeepEqual(t, deployment.Spec.Template.Spec.NodeSelector, argocommon.DefaultNodeSelector())
 	assert.DeepEqual(t, deployment.Spec.Template.Spec.Tolerations, deploymentDefaultTolerations())
 }
 

controllers/gitopsservice_controller.go

@@ -95,10 +95,15 @@ func (r *ReconcileGitopsService) SetupWithManager(mgr ctrl.Manager) error {
 		},
 	}
 
+	// check if the gitops service instance already exists, do nothing if exists, create if it doesn't
 	gitopsServiceRef := newGitopsService()
 	err := r.Client.Create(context.TODO(), gitopsServiceRef)
 	if err != nil {
-		reqLogger.Error(err, "Failed to create GitOps service instance")
+		if errors.IsAlreadyExists(err) {
+			reqLogger.Info("GitOps service instance already exists, skipping creation")
+		} else {
+			reqLogger.Error(err, "Failed to create GitOps service instance")
+		}
 	}
 
 	bldr := ctrl.NewControllerManagedBy(mgr).
@@ -249,6 +254,7 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil
 	if err != nil {
 		if errors.IsNotFound(err) {
 			reqLogger.Info("Creating a new Namespace", "Name", namespace)
+			ensureInfraNodeSelectorAnnotation(namespaceRef, instance.Spec.RunOnInfra)
 			err = r.Client.Create(ctx, namespaceRef)
 			if err != nil {
 				return reconcile.Result{}, err
@@ -257,9 +263,13 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil
 			return reconcile.Result{}, err
 		}
 	} else {
-		needsUpdate, updateNameSpace := ensurePodSecurityLabels(namespaceRef)
+		needsUpdate := false
+		labelUpdate, namespaceRef := ensurePodSecurityLabels(namespaceRef)
+		needsUpdate = needsUpdate || labelUpdate
+		annotationUpdate, namespaceRef := ensureInfraNodeSelectorAnnotation(namespaceRef, instance.Spec.RunOnInfra)
+		needsUpdate = needsUpdate || annotationUpdate
 		if needsUpdate {
-			err = r.Client.Update(context.TODO(), updateNameSpace)
+			err = r.Client.Update(context.TODO(), namespaceRef)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
@@ -417,6 +427,7 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
 	if err != nil {
 		if errors.IsNotFound(err) {
 			reqLogger.Info("Creating a new Namespace", "Name", argocdNS.Name)
+			ensureInfraNodeSelectorAnnotation(argocdNS, instance.Spec.RunOnInfra)
 			err = r.Client.Create(context.TODO(), argocdNS)
 			if err != nil {
 				return reconcile.Result{}, err
@@ -446,9 +457,13 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
 			}
 		}
 
-		needsUpdate, updateNameSpace := ensurePodSecurityLabels(argocdNS)
+		needsUpdate := false
+		labelUpdate, argocdNS := ensurePodSecurityLabels(argocdNS)
+		needsUpdate = needsUpdate || labelUpdate
+		annotationUpdate, argocdNS := ensureInfraNodeSelectorAnnotation(argocdNS, instance.Spec.RunOnInfra)
+		needsUpdate = needsUpdate || annotationUpdate
 		if needsUpdate {
-			err = r.Client.Update(context.TODO(), updateNameSpace)
+			err = r.Client.Update(context.TODO(), argocdNS)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
@@ -461,16 +476,6 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli
 		return reconcile.Result{}, err
 	}
 
-	//to add infra nodeselector to default argocd pods
-	if instance.Spec.RunOnInfra {
-		if defaultArgoCDInstance.Spec.NodePlacement == nil {
-			defaultArgoCDInstance.Spec.NodePlacement = &argoapp.ArgoCDNodePlacementSpec{
-				NodeSelector: common.InfraNodeSelector(),
-			}
-		} else {
-			defaultArgoCDInstance.Spec.NodePlacement.NodeSelector = argocdutil.AppendStringMap(defaultArgoCDInstance.Spec.NodePlacement.NodeSelector, common.InfraNodeSelector())
-		}
-	}
 	if len(instance.Spec.NodeSelector) > 0 {
 		if defaultArgoCDInstance.Spec.NodePlacement == nil {
 			defaultArgoCDInstance.Spec.NodePlacement = &argoapp.ArgoCDNodePlacementSpec{
@@ -673,9 +678,6 @@ func (r *ReconcileGitopsService) reconcileBackend(gitopsserviceNamespacedName ty
 		if err := controllerutil.SetControllerReference(instance, deploymentObj, r.Scheme); err != nil {
 			return reconcile.Result{}, err
 		}
-		if instance.Spec.RunOnInfra {
-			deploymentObj.Spec.Template.Spec.NodeSelector[common.InfraNodeLabelSelector] = ""
-		}
 		if len(instance.Spec.NodeSelector) > 0 {
 			deploymentObj.Spec.Template.Spec.NodeSelector = argocdutil.AppendStringMap(deploymentObj.Spec.Template.Spec.NodeSelector, instance.Spec.NodeSelector)
 		}
@@ -1023,3 +1025,21 @@ func ensurePodSecurityLabels(namespace *corev1.Namespace) (bool, *corev1.Namespa
 	}
 	return false, namespace
 }
+
+func ensureInfraNodeSelectorAnnotation(namespace *corev1.Namespace, runOnInfra bool) (bool, *corev1.Namespace) {
+	if namespace.Annotations == nil {
+		namespace.Annotations = make(map[string]string)
+	}
+	if runOnInfra {
+		if namespace.Annotations[common.InfraNodeSelectorAnnotation] != common.InfraNodeSelectorAnnotationValue {
+			namespace.Annotations[common.InfraNodeSelectorAnnotation] = common.InfraNodeSelectorAnnotationValue
+			return true, namespace
+		}
+	} else {
+		if _, exists := namespace.Annotations[common.InfraNodeSelectorAnnotation]; exists {
+			delete(namespace.Annotations, common.InfraNodeSelectorAnnotation)
+			return true, namespace
+		}
+	}
+	return false, namespace
+}

controllers/gitopsservice_controller_test.go

@@ -25,7 +25,6 @@ import (
 	argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1"
 	argocommon "github.com/argoproj-labs/argocd-operator/common"
 	"github.com/argoproj-labs/argocd-operator/controllers/argocd"
-	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 	configv1 "github.com/openshift/api/config/v1"
 	consolev1 "github.com/openshift/api/console/v1"
 	routev1 "github.com/openshift/api/route/v1"
@@ -628,21 +627,69 @@ func TestReconcile_InfrastructureNode(t *testing.T) {
 	_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
 	assertNoError(t, err)
 
+	// Namespace should have infra node-selector annotation
+	ns := &corev1.Namespace{}
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceNamespace}, ns)
+	assertNoError(t, err)
+	assert.Equal(t, ns.Annotations[common.InfraNodeSelectorAnnotation], common.InfraNodeSelectorAnnotationValue)
+
+	// Backend deployment should NOT have infra NodeSelector in pod spec (namespace annotation handles it)
 	deployment := appsv1.Deployment{}
 	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName, Namespace: serviceNamespace}, &deployment)
 	assertNoError(t, err)
-	nSelector := common.InfraNodeSelector()
-	argoutil.AppendStringMap(nSelector, argocommon.DefaultNodeSelector())
-	assert.DeepEqual(t, deployment.Spec.Template.Spec.NodeSelector, nSelector)
+	assert.DeepEqual(t, deployment.Spec.Template.Spec.NodeSelector, argocommon.DefaultNodeSelector())
 	assert.DeepEqual(t, deployment.Spec.Template.Spec.Tolerations, deploymentDefaultTolerations())
 
+	// ArgoCD instance should only have Tolerations in NodePlacement, not infra NodeSelector
 	argoCD := &argoapp.ArgoCD{}
 	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace},
 		argoCD)
 	assertNoError(t, err)
-	assert.DeepEqual(t, argoCD.Spec.NodePlacement.NodeSelector, common.InfraNodeSelector())
+	assert.Assert(t, argoCD.Spec.NodePlacement != nil)
+	assert.Assert(t, argoCD.Spec.NodePlacement.NodeSelector == nil)
 	assert.DeepEqual(t, argoCD.Spec.NodePlacement.Tolerations, deploymentDefaultTolerations())
+}
+
+func TestReconcile_InfrastructureNode_AnnotationRemoved(t *testing.T) {
+	logf.SetLogger(argocd.ZapLogger(true))
+	s := scheme.Scheme
+	addKnownTypesToScheme(s)
+	gitopsService := &pipelinesv1alpha1.GitopsService{
+		ObjectMeta: v1.ObjectMeta{
+			Name: serviceName,
+		},
+		Spec: pipelinesv1alpha1.GitopsServiceSpec{
+			RunOnInfra: true,
+		},
+	}
+	fakeClient := fake.NewFakeClient(gitopsService)
+	reconciler := newReconcileGitOpsService(fakeClient, s)
+
+	_, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
+	assertNoError(t, err)
+
+	// Verify annotation present
+	ns := &corev1.Namespace{}
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceNamespace}, ns)
+	assertNoError(t, err)
+	assert.Equal(t, ns.Annotations[common.InfraNodeSelectorAnnotation], common.InfraNodeSelectorAnnotationValue)
 
+	// Toggle RunOnInfra off
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName}, gitopsService)
+	assertNoError(t, err)
+	gitopsService.Spec.RunOnInfra = false
+	err = fakeClient.Update(context.TODO(), gitopsService)
+	assertNoError(t, err)
+
+	_, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test"))
+	assertNoError(t, err)
+
+	// Verify annotation removed
+	ns = &corev1.Namespace{}
+	err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceNamespace}, ns)
+	assertNoError(t, err)
+	_, hasAnnotation := ns.Annotations[common.InfraNodeSelectorAnnotation]
+	assert.Assert(t, !hasAnnotation)
 }
 
 func TestReconcile_PSSLabels(t *testing.T) {

test/e2e/gitopsservice_test.go

@@ -22,13 +22,10 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
-	"reflect"
 	"strings"
 	"time"
 
 	argoapp "github.com/argoproj-labs/argocd-operator/api/v1beta1"
-	"github.com/argoproj-labs/argocd-operator/common"
-	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
@@ -623,7 +620,6 @@ var _ = Describe("GitOpsServiceController", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			gitopsService.Spec.RunOnInfra = true
-			nodeSelector := argoutil.AppendStringMap(gitopscommon.InfraNodeSelector(), common.DefaultNodeSelector())
 
 			err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
 				updateErr := k8sClient.Update(context.TODO(), gitopsService)
@@ -636,26 +632,16 @@ var _ = Describe("GitOpsServiceController", func() {
 			Expect(err).NotTo(HaveOccurred())
 
 			Eventually(func() bool {
-				deployment := &appsv1.Deployment{}
-
-				if err = k8sClient.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: argoCDNamespace}, deployment); err != nil {
+				ns := &corev1.Namespace{}
+				if err = k8sClient.Get(context.TODO(), types.NamespacedName{Name: argoCDNamespace}, ns); err != nil {
 					fmt.Println(err)
 					return false
 				}
 
-				if !reflect.DeepEqual(deployment.Spec.Template.Spec.NodeSelector, nodeSelector) {
-					fmt.Println("NodeSelectors are not equal")
+				if ns.Annotations == nil {
 					return false
 				}
-
-				argocd := &argoapp.ArgoCD{}
-
-				if err := k8sClient.Get(context.TODO(), types.NamespacedName{Name: argoCDInstanceName, Namespace: argoCDNamespace}, argocd); err != nil {
-					fmt.Println(err)
-					return false
-				}
-
-				return reflect.DeepEqual(argocd.Spec.NodePlacement.NodeSelector, gitopscommon.InfraNodeSelector())
+				return ns.Annotations[gitopscommon.InfraNodeSelectorAnnotation] == gitopscommon.InfraNodeSelectorAnnotationValue
 
 			}, time.Second*180, time.Second*5).Should(BeTrue())
 
@@ -673,26 +659,18 @@ var _ = Describe("GitOpsServiceController", func() {
 			Expect(err).ToNot(HaveOccurred())
 
 			Eventually(func() bool {
-				deployment := &appsv1.Deployment{}
-				err = k8sClient.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: argoCDNamespace}, deployment)
+				ns := &corev1.Namespace{}
+				err = k8sClient.Get(context.TODO(), types.NamespacedName{Name: argoCDNamespace}, ns)
 				if err != nil {
-					GinkgoWriter.Println("Unable to get Deployment", err)
+					GinkgoWriter.Println("Unable to get Namespace", err)
 					return false
 				}
 
-				if len(deployment.Spec.Template.Spec.NodeSelector) != 1 {
-					GinkgoWriter.Println("expected one nodeSelector in deployment")
-					return false
-				}
-
-				argocd := &argoapp.ArgoCD{}
-				err = k8sClient.Get(context.TODO(), types.NamespacedName{Name: argoCDInstanceName, Namespace: argoCDNamespace}, argocd)
-				if err != nil {
-					GinkgoWriter.Println("Unable to get ArgoCD", err)
-					return false
+				if ns.Annotations == nil {
+					return true
 				}
-
-				return argocd.Spec.NodePlacement == nil
+				_, exists := ns.Annotations[gitopscommon.InfraNodeSelectorAnnotation]
+				return !exists
 			}, "3m", "5s").Should(BeTrue())
 
 		})

test/openshift/e2e/ginkgo/fixture/namespace/fixture.go

@@ -28,6 +28,27 @@ func HaveLabel(key string, value string) matcher.GomegaMatcher {
 	})
 }
 
+func HaveAnnotation(key string, value string) matcher.GomegaMatcher {
+	return fetchNamespace(func(ns *corev1.Namespace) bool {
+		GinkgoWriter.Println("Namespace - HaveAnnotation: Key:", key, "Expected:", value, "/ Actual:", ns.Annotations[key])
+		if ns.Annotations == nil {
+			return false
+		}
+		return ns.Annotations[key] == value
+	})
+}
+
+func NotHaveAnnotation(key string) matcher.GomegaMatcher {
+	return fetchNamespace(func(ns *corev1.Namespace) bool {
+		GinkgoWriter.Println("Namespace - NotHaveAnnotation: Key:", key, "/ Present:", ns.Annotations[key])
+		if ns.Annotations == nil {
+			return true
+		}
+		_, exists := ns.Annotations[key]
+		return !exists
+	})
+}
+
 // Update will keep trying to update object until it succeeds, or times out.
 func Update(obj *corev1.Namespace, modify func(*corev1.Namespace)) {
 	k8sClient, _ := utils.GetE2ETestKubeClient()