From ac89488ee6dd0cbdb1f38ab3e1b98d7ffdd0a814 Mon Sep 17 00:00:00 2001 From: Anand Kumar Singh Date: Mon, 29 Jun 2026 08:54:36 +0530 Subject: [PATCH 1/2] feat: use namespace annotation for infra node scheduling Replace per-pod infra NodeSelector with openshift.io/node-selector annotation on default namespace. OpenShift admission controller applies selector to all pods in namespace automatically. Tolerations and custom NodeSelector remain at pod level. assited-by: claude-code Signed-off-by: Anand Kumar Singh --- common/common.go | 4 + controllers/consoleplugin.go | 3 - controllers/consoleplugin_test.go | 8 +- controllers/gitopsservice_controller.go | 49 +++++--- controllers/gitopsservice_controller_test.go | 57 ++++++++- test/e2e/gitopsservice_test.go | 44 ++----- .../e2e/ginkgo/fixture/namespace/fixture.go | 21 ++++ .../1-028-validate_run_on_infra_test.go | 109 +++++------------- .../1-071_validate_node_selectors_test.go | 33 +++--- 9 files changed, 168 insertions(+), 160 deletions(-) diff --git a/common/common.go b/common/common.go index 2cbf75c3fe7..14421398885 100644 --- a/common/common.go +++ b/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 diff --git a/controllers/consoleplugin.go b/controllers/consoleplugin.go index 80673fdd228..db27fa86f5c 100644 --- a/controllers/consoleplugin.go +++ b/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) } diff --git a/controllers/consoleplugin_test.go b/controllers/consoleplugin_test.go index c6f3743e946..8624f383d89 100644 --- a/controllers/consoleplugin_test.go +++ b/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()) } diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index 86ccff10385..e2858b82595 100644 --- a/controllers/gitopsservice_controller.go +++ b/controllers/gitopsservice_controller.go @@ -249,6 +249,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 +258,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 +422,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 +452,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 +471,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 +673,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 +1020,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 +} diff --git a/controllers/gitopsservice_controller_test.go b/controllers/gitopsservice_controller_test.go index d0447e89962..65860dfeff7 100644 --- a/controllers/gitopsservice_controller_test.go +++ b/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) { diff --git a/test/e2e/gitopsservice_test.go b/test/e2e/gitopsservice_test.go index 4596384fbab..210c5368457 100644 --- a/test/e2e/gitopsservice_test.go +++ b/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()) }) diff --git a/test/openshift/e2e/ginkgo/fixture/namespace/fixture.go b/test/openshift/e2e/ginkgo/fixture/namespace/fixture.go index bb94bc8cbe6..4999f5c8e45 100644 --- a/test/openshift/e2e/ginkgo/fixture/namespace/fixture.go +++ b/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() diff --git a/test/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.go b/test/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.go index 0190160ea67..9a37854b1ca 100644 --- a/test/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.go +++ b/test/openshift/e2e/ginkgo/sequential/1-028-validate_run_on_infra_test.go @@ -7,11 +7,13 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" gitopsoperatorv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1" + "github.com/redhat-developer/gitops-operator/common" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" argocdFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/argocd" deploymentFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/deployment" gitopsserviceFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/gitopsservice" k8sFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/k8s" + namespaceFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/namespace" statefulsetFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/statefulset" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" appsv1 "k8s.io/api/apps/v1" @@ -55,72 +57,34 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { }) }() - By("verifying the openshift-gitops resources have the run on infra labels and tolerations applied") + By("verifying the openshift-gitops namespace has the infra node-selector annotation") + openshiftGitopsNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops"}} + Eventually(openshiftGitopsNS).Should(namespaceFixture.HaveAnnotation(common.InfraNodeSelectorAnnotation, common.InfraNodeSelectorAnnotationValue)) + + By("verifying the openshift-gitops resources have tolerations applied") clusterDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "openshift-gitops"}} - Eventually(clusterDepl).Should( - And( - deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - deploymentFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(clusterDepl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) serverDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-server", Namespace: "openshift-gitops"}} - Eventually(serverDepl).Should( - And( - deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - deploymentFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(serverDepl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) repoServerDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-repo-server", Namespace: "openshift-gitops"}} - Eventually(repoServerDepl).Should( - And( - deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - deploymentFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(repoServerDepl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) dexServerDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-dex-server", Namespace: "openshift-gitops"}} - Eventually(dexServerDepl).Should( - And( - deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - deploymentFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(dexServerDepl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) redisServerDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-redis", Namespace: "openshift-gitops"}} - Eventually(redisServerDepl).Should( - And( - deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - deploymentFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(redisServerDepl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) appControllerSS := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-application-controller", Namespace: "openshift-gitops"}} - Eventually(appControllerSS).Should( - And( - statefulsetFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - }), - statefulsetFixture.HaveTolerations([]corev1.Toleration{ - {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), - )) + Eventually(appControllerSS).Should(statefulsetFixture.HaveTolerations([]corev1.Toleration{ + {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) By("creating a simple namespace-scoped Argo CD instance in another namespace") @@ -136,43 +100,26 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { // verifyNotPresentInNamespace verifies that the various Argo CD resources in the namespace do not have 'run on infra' set verifyNotPresentInNamespace := func(ns string) { + nsObj := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}} + Eventually(nsObj).Should(namespaceFixture.NotHaveAnnotation(common.InfraNodeSelectorAnnotation)) + serverDepl = &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-server", Namespace: ns}} - Eventually(serverDepl).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) Eventually(serverDepl).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) repoServer := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-repo-server", Namespace: ns}} - Eventually(repoServer).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) Eventually(repoServer).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) dexServer := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-dex-server", Namespace: ns}} - Eventually(dexServer).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) Eventually(dexServer).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) redisDepl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "argocd-redis", Namespace: ns}} - Eventually(redisDepl).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) Eventually(redisDepl).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) appControllerSS = &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "argocd-application-controller", Namespace: ns}} - Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}})) @@ -194,20 +141,16 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { gs.Spec.Tolerations = nil }) - By("verifying that the resources in openshift-gitops no longer have run on infra tolerations/label set") + By("verifying that the openshift-gitops namespace no longer has the infra annotation") + Eventually(openshiftGitopsNS).Should(namespaceFixture.NotHaveAnnotation(common.InfraNodeSelectorAnnotation)) - clusterDepl = &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "openshift-gitops"}} - Eventually(clusterDepl).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "node-role.kubernetes.io/infra": "", - "kubernetes.io/os": "linux", - })) + By("verifying that the resources in openshift-gitops no longer have run on infra tolerations set") + clusterDepl = &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "cluster", Namespace: "openshift-gitops"}} Eventually(clusterDepl).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{ {Key: "infra", Effect: "NoSchedule", Value: "reserved"}}), ) - verifyNotPresentInNamespace("openshift-gitops") - // This is required, otherwise StatefulSet will be stuck for every subsequent test. This was ported from kuttl (but we delete the SS rather than updating its replicas) appControllerSS = &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops-application-controller", Namespace: "openshift-gitops"}} diff --git a/test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go b/test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go index beb189fefaa..cb083665702 100644 --- a/test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go +++ b/test/openshift/e2e/ginkgo/sequential/1-071_validate_node_selectors_test.go @@ -6,10 +6,12 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" gitopsoperatorv1alpha1 "github.com/redhat-developer/gitops-operator/api/v1alpha1" + "github.com/redhat-developer/gitops-operator/common" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture" deploymentFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/deployment" gitopsserviceFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/gitopsservice" k8sFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/k8s" + namespaceFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/namespace" statefulsetFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/statefulset" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" appsv1 "k8s.io/api/apps/v1" @@ -90,14 +92,17 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { Value: "reserved"}} }) - By("ensuring Deployments and StatefulSets pick up the change to nodeSelector and tolerations") + By("ensuring the openshift-gitops namespace has the infra node-selector annotation") + openshiftGitopsNS := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "openshift-gitops"}} + Eventually(openshiftGitopsNS).Should(namespaceFixture.HaveAnnotation(common.InfraNodeSelectorAnnotation, common.InfraNodeSelectorAnnotationValue)) + + By("ensuring Deployments and StatefulSets still have the custom nodeSelector and tolerations") for _, deploymentName := range deploymentNameList { depl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: deploymentName, Namespace: "openshift-gitops"}} Eventually(depl).Should(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "kubernetes.io/os": "linux", - "key1": "value1", - "node-role.kubernetes.io/infra": "", + "kubernetes.io/os": "linux", + "key1": "value1", })) Eventually(depl).Should(deploymentFixture.HaveTolerations([]corev1.Toleration{{ Effect: "NoSchedule", @@ -107,9 +112,8 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { } Eventually(appControllerSS).Should(statefulsetFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "kubernetes.io/os": "linux", - "key1": "value1", - "node-role.kubernetes.io/infra": "", + "kubernetes.io/os": "linux", + "key1": "value1", })) Eventually(appControllerSS).Should(statefulsetFixture.HaveTolerations([]corev1.Toleration{{ Effect: "NoSchedule", @@ -125,14 +129,16 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { gs.Spec.NodeSelector = nil }) - By("ensuring Deployments and StatefulSets have the nodeSelector and tolerations removed") + By("ensuring the openshift-gitops namespace no longer has the infra annotation") + Eventually(openshiftGitopsNS).Should(namespaceFixture.NotHaveAnnotation(common.InfraNodeSelectorAnnotation)) + + By("ensuring Deployments and StatefulSets have the custom nodeSelector and tolerations removed") for _, deploymentName := range deploymentNameList { depl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: deploymentName, Namespace: "openshift-gitops"}} Eventually(depl).ShouldNot(deploymentFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "kubernetes.io/os": "linux", - "key1": "value1", - "node-role.kubernetes.io/infra": "", + "kubernetes.io/os": "linux", + "key1": "value1", })) Eventually(depl).ShouldNot(deploymentFixture.HaveTolerations([]corev1.Toleration{{ Effect: "NoSchedule", @@ -142,9 +148,8 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { } Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveTemplateSpecNodeSelector(map[string]string{ - "kubernetes.io/os": "linux", - "key1": "value1", - "node-role.kubernetes.io/infra": "", + "kubernetes.io/os": "linux", + "key1": "value1", })) Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveTolerations([]corev1.Toleration{{ Effect: "NoSchedule", From 9ef0c49fac059ce0a712313935c7252821b55217 Mon Sep 17 00:00:00 2001 From: Anand Kumar Singh Date: Mon, 29 Jun 2026 10:58:07 +0530 Subject: [PATCH 2/2] add log for gitops-service skip creation if already exists Signed-off-by: Anand Kumar Singh --- controllers/gitopsservice_controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index e2858b82595..7c3169ef7f1 100644 --- a/controllers/gitopsservice_controller.go +++ b/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).