fix: Add Server Service check to apply the changes in case the servic…

…e exists (#1546) * Add Server Service check to apply the changes in case the service exists Signed-off-by: nmirasch <[email protected]> * Add changes to existing service and update Signed-off-by: nmirasch <[email protected]> * Add testing Signed-off-by: nmirasch <[email protected]> --------- Signed-off-by: nmirasch <[email protected]>
argoproj-labs · Sep 27, 2024 · da6b8a7 · da6b8a7
1 parent 6ece288
commit da6b8a7
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 14 deletions.
diff --git a/controllers/argocd/service.go b/controllers/argocd/service.go
@@ -17,6 +17,7 @@ package argocd
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -422,20 +423,6 @@ func (r *ReconcileArgoCD) reconcileServerMetricsService(cr *argoproj.ArgoCD) err
 // reconcileServerService will ensure that the Service is present for the Argo CD server component.
 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(r.Client, 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(r.Client, svc, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS())
 
 	svc.Spec.Ports = []corev1.ServicePort{
@@ -461,6 +448,29 @@ func (r *ReconcileArgoCD) reconcileServerService(cr *argoproj.ArgoCD) error {
 	if err := controllerutil.SetControllerReference(cr, svc, r.Scheme); err != nil {
 		return err
 	}
+
+	existingSVC := &corev1.Service{}
+	if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, existingSVC) {
+		changed := false
+		if !cr.Spec.Server.IsEnabled() {
+			return r.Client.Delete(context.TODO(), svc)
+		}
+		if ensureAutoTLSAnnotation(r.Client, existingSVC, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS()) {
+			changed = true
+		}
+		if !reflect.DeepEqual(svc.Spec.Type, existingSVC.Spec.Type) {
+			existingSVC.Spec.Type = svc.Spec.Type
+			changed = true
+		}
+		if changed {
+			return r.Client.Update(context.TODO(), existingSVC)
+		}
+		return nil
+	}
+
+	if !cr.Spec.Server.IsEnabled() {
+		return nil
+	}
 	return r.Client.Create(context.TODO(), svc)
 }
 

diff --git a/controllers/argocd/service_test.go b/controllers/argocd/service_test.go
@@ -6,8 +6,10 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
@@ -81,3 +83,51 @@ func TestEnsureAutoTLSAnnotation(t *testing.T) {
 		assert.Equal(t, needUpdate, false)
 	})
 }
+
+func TestReconcileServerService(t *testing.T) {
+	a := makeTestArgoCD()
+	resObjs := []client.Object{a}
+	subresObjs := []client.Object{a}
+	runtimeObjs := []runtime.Object{}
+	sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+	r := makeTestReconciler(cl, sch)
+	a = makeTestArgoCD(func(a *argoproj.ArgoCD) {
+		a.Spec.Server.Service.Type = "NodePort"
+	})
+	serverService := newServiceWithSuffix("server", "server", a)
+	t.Run("Server Service Created when the Server Service is not found ", func(t *testing.T) {
+		err := r.Client.Get(context.TODO(), types.NamespacedName{
+			Name:      "argocd-server",
+			Namespace: testNamespace,
+		}, serverService)
+		assert.True(t, errors.IsNotFound(err))
+
+		err = r.reconcileServerService(a)
+		assert.NoError(t, err)
+
+		err = r.Client.Get(context.TODO(), types.NamespacedName{
+			Name:      "argocd-server",
+			Namespace: testNamespace,
+		}, serverService)
+		assert.NoError(t, err)
+		assert.Equal(t, a.Spec.Server.Service.Type, serverService.Spec.Type)
+	})
+
+	t.Run("Server Service Type update ", func(t *testing.T) {
+		// Reconcile with previous existing Server Service with a different Type
+		a.Spec.Server.Service.Type = "ClusterIP"
+		assert.NotEqual(t, a.Spec.Server.Service.Type, serverService.Spec.Type)
+
+		err := r.reconcileServerService(a)
+		assert.NoError(t, err)
+
+		// Existing Server is found and has the argoCD new Server Service Type
+		err = r.Client.Get(context.TODO(), types.NamespacedName{
+			Name:      "argocd-server",
+			Namespace: testNamespace,
+		}, serverService)
+		assert.NoError(t, err)
+		assert.Equal(t, a.Spec.Server.Service.Type, serverService.Spec.Type)
+	})
+}