code-refactoring: enable route and prometheus API checks (#1014)

* enable route and prometheus API checks

---------

Signed-off-by: Jaideep Rao <jaideep.r97@gmail.com>

controllers/argocd/argocd_controller.go

@@ -283,7 +283,7 @@ func (r *ArgoCDReconciler) reconcileControllers() error {
 
 func (r *ArgoCDReconciler) InitializeControllerReconcilers() {
 	r.SecretController = &secret.SecretReconciler{
-		Client:            &r.Client,
+		Client:            r.Client,
 		Scheme:            r.Scheme,
 		Instance:          r.Instance,
 		ClusterScoped:     r.ClusterScoped,

controllers/argocd/cluster.go

@@ -5,27 +5,33 @@ import (
 
 	"github.com/argoproj-labs/argocd-operator/common"
 	"github.com/argoproj-labs/argocd-operator/pkg/cluster"
+	"github.com/argoproj-labs/argocd-operator/pkg/monitoring"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
 )
 
 // InspectCluster will verify the availability of extra features on the cluster, such as Prometheus and OpenShift Routes.
-func InspectCluster() {
-	// if err := monitoring.VerifyPrometheusAPI(); err != nil {
-	// 	// TO DO: log error verifying prometheus API (warn)
-	// }
+func InspectCluster() error {
+	var inspectError error
 
-	// if err := networking.VerifyRouteAPI(); err != nil {
-	// 	// TO DO: log error verifying route API (warn)
-	// }
+	if err := monitoring.VerifyPrometheusAPI(); err != nil {
+		inspectError = err
+	}
+
+	if err := networking.VerifyRouteAPI(); err != nil {
+		inspectError = err
+	}
 
 	if err := workloads.VerifyTemplateAPI(); err != nil {
-		// TO DO: log error verifying template API (warn)
+		inspectError = err
 	}
 
 	if err := cluster.VerifyVersionAPI(); err != nil {
-		// TO DO: log error verifying version API (warn)
+		inspectError = err
 	}
+
+	return inspectError
 }
 
 func GetClusterConfigNamespaces() string {

controllers/argocd/prometheus.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -28,8 +29,6 @@ import (
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 )
 
-var prometheusAPIFound = false
-
 // getPrometheusHost will return the hostname value for Prometheus.
 func getPrometheusHost(cr *argoproj.ArgoCD) string {
 	host := util.NameWithSuffix(cr.Name, "prometheus")
@@ -50,11 +49,6 @@ func getPrometheusReplicas(cr *argoproj.ArgoCD) *int32 {
 	return &replicas
 }
 
-// IsPrometheusAPIAvailable returns true if the Prometheus API is present.
-func IsPrometheusAPIAvailable() bool {
-	return prometheusAPIFound
-}
-
 // hasPrometheusSpecChanged will return true if the supported properties differs in the actual versus the desired state.
 func hasPrometheusSpecChanged(actual *monitoringv1.Prometheus, desired *argoproj.ArgoCD) bool {
 	// Replica count
@@ -74,16 +68,6 @@ func hasPrometheusSpecChanged(actual *monitoringv1.Prometheus, desired *argoproj
 	return false
 }
 
-// verifyPrometheusAPI will verify that the Prometheus API is present.
-func verifyPrometheusAPI() error {
-	found, err := util.VerifyAPI(monitoringv1.SchemeGroupVersion.Group, monitoringv1.SchemeGroupVersion.Version)
-	if err != nil {
-		return err
-	}
-	prometheusAPIFound = found
-	return nil
-}
-
 // newPrometheus returns a new Prometheus instance for the given ArgoCD.
 func newPrometheus(cr *argoproj.ArgoCD) *monitoringv1.Prometheus {
 	return &monitoringv1.Prometheus{

controllers/argocd/route.go

@@ -28,23 +28,6 @@ import (
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 )
 
-var routeAPIFound = false
-
-// IsRouteAPIAvailable returns true if the Route API is present.
-func IsRouteAPIAvailable() bool {
-	return routeAPIFound
-}
-
-// verifyRouteAPI will verify that the Route API is present.
-func verifyRouteAPI() error {
-	found, err := util.VerifyAPI(routev1.GroupName, routev1.GroupVersion.Version)
-	if err != nil {
-		return err
-	}
-	routeAPIFound = found
-	return nil
-}
-
 // newRoute returns a new Route instance for the given ArgoCD.
 func newRoute(cr *argoproj.ArgoCD) *routev1.Route {
 	return &routev1.Route{

controllers/argocd/route_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -23,7 +24,11 @@ import (
 )
 
 func TestReconcileRouteSetLabels(t *testing.T) {
-	routeAPIFound = true
+	networking.SetRouteAPIFound(true)
+	defer func() {
+		networking.SetRouteAPIFound(false)
+	}()
+
 	ctx := context.Background()
 	logf.SetLogger(ZapLogger(true))
 	argoCD := makeArgoCD(func(a *argoproj.ArgoCD) {
@@ -58,7 +63,11 @@ func TestReconcileRouteSetLabels(t *testing.T) {
 
 }
 func TestReconcileRouteSetsInsecure(t *testing.T) {
-	routeAPIFound = true
+	networking.SetRouteAPIFound(true)
+	defer func() {
+		networking.SetRouteAPIFound(false)
+	}()
+
 	ctx := context.Background()
 	logf.SetLogger(ZapLogger(true))
 	argoCD := makeArgoCD(func(a *argoproj.ArgoCD) {
@@ -129,7 +138,10 @@ func TestReconcileRouteSetsInsecure(t *testing.T) {
 }
 
 func TestReconcileRouteUnsetsInsecure(t *testing.T) {
-	routeAPIFound = true
+	networking.SetRouteAPIFound(true)
+	defer func() {
+		networking.SetRouteAPIFound(false)
+	}()
 	ctx := context.Background()
 	logf.SetLogger(ZapLogger(true))
 	argoCD := makeArgoCD(func(a *argoproj.ArgoCD) {

controllers/argocd/secret/secret.go

@@ -8,7 +8,7 @@ import (
 )
 
 type SecretReconciler struct {
-	Client            *client.Client
+	Client            client.Client
 	Scheme            *runtime.Scheme
 	Instance          *argoproj.ArgoCD
 	ClusterScoped     bool

controllers/argocd/service.go

@@ -25,6 +25,7 @@ import (
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 )
 
@@ -325,7 +326,7 @@ func ensureAutoTLSAnnotation(svc *corev1.Service, secretName string, enabled boo
 	var autoTLSAnnotationName, autoTLSAnnotationValue string
 
 	// We currently only support OpenShift for automatic TLS
-	if IsRouteAPIAvailable() {
+	if networking.IsRouteAPIAvailable() {
 		autoTLSAnnotationName = common.ServiceBetaOpenshiftKeyCertSecret
 		if svc.Annotations == nil {
 			svc.Annotations = make(map[string]string)

controllers/argocd/service_test.go

@@ -6,12 +6,16 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 )
 
 func TestEnsureAutoTLSAnnotation(t *testing.T) {
 	a := makeTestArgoCD()
 	t.Run("Ensure annotation will be set for OpenShift", func(t *testing.T) {
-		routeAPIFound = true
+		networking.SetRouteAPIFound(true)
+		defer func() {
+			networking.SetRouteAPIFound(false)
+		}()
 		svc := newService(a)
 
 		// Annotation is inserted, update is required
@@ -26,7 +30,10 @@ func TestEnsureAutoTLSAnnotation(t *testing.T) {
 		assert.Equal(t, needUpdate, false)
 	})
 	t.Run("Ensure annotation will be unset for OpenShift", func(t *testing.T) {
-		routeAPIFound = true
+		networking.SetRouteAPIFound(true)
+		defer func() {
+			networking.SetRouteAPIFound(false)
+		}()
 		svc := newService(a)
 		svc.Annotations = make(map[string]string)
 		svc.Annotations[common.ServiceBetaOpenshiftKeyCertSecret] = "some-secret"
@@ -42,7 +49,7 @@ func TestEnsureAutoTLSAnnotation(t *testing.T) {
 		assert.Equal(t, needUpdate, false)
 	})
 	t.Run("Ensure annotation will not be set for non-OpenShift", func(t *testing.T) {
-		routeAPIFound = false
+		networking.SetRouteAPIFound(false)
 		svc := newService(a)
 		needUpdate := ensureAutoTLSAnnotation(svc, "some-secret", true)
 		assert.Equal(t, needUpdate, false)

controllers/argocd/status.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
 )
@@ -330,7 +331,7 @@ func (r *ArgoCDReconciler) reconcileStatusNotifications(cr *argoproj.ArgoCD) err
 func (r *ArgoCDReconciler) reconcileStatusHost(cr *argoproj.ArgoCD) error {
 	cr.Status.Host = ""
 
-	if (cr.Spec.Server.Route.Enabled || cr.Spec.Server.Ingress.Enabled) && IsRouteAPIAvailable() {
+	if (cr.Spec.Server.Route.Enabled || cr.Spec.Server.Ingress.Enabled) && networking.IsRouteAPIAvailable() {
 		route := newRouteWithSuffix("server", cr)
 
 		// The Red Hat OpenShift ingress controller implementation is designed to watch ingress objects and create one or more routes

controllers/argocd/status_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
 
 	oappsv1 "github.com/openshift/api/apps/v1"
@@ -173,7 +174,10 @@ func TestArgoCDReconciler_reconcileStatusHost(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 
-			routeAPIFound = test.testRouteAPIFound
+			networking.SetRouteAPIFound(test.testRouteAPIFound)
+			defer func() {
+				networking.SetRouteAPIFound(false)
+			}()
 
 			a := makeTestArgoCD(func(a *argoproj.ArgoCD) {
 				a.Spec.Server.Route.Enabled = test.routeEnabled

controllers/argocd/util.go

@@ -27,6 +27,8 @@ import (
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/pkg/monitoring"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	util "github.com/argoproj-labs/argocd-operator/pkg/util"
 	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
 
@@ -259,7 +261,7 @@ func (r *ArgoCDReconciler) getArgoServerURI(cr *argoproj.ArgoCD) string {
 	}
 
 	// Use Route host if available, override Ingress if both exist
-	if IsRouteAPIAvailable() {
+	if networking.IsRouteAPIAvailable() {
 		route := newRouteWithSuffix("server", cr)
 		if util.IsObjectFound(r.Client, cr.Namespace, route.Name, route) {
 			host = route.Spec.Host
@@ -698,14 +700,14 @@ func (r *ArgoCDReconciler) reconcileResources(cr *argoproj.ArgoCD) error {
 		return err
 	}
 
-	if IsRouteAPIAvailable() {
+	if networking.IsRouteAPIAvailable() {
 		log.Info("reconciling routes")
 		if err := r.reconcileRoutes(cr); err != nil {
 			return err
 		}
 	}
 
-	if IsPrometheusAPIAvailable() {
+	if monitoring.IsPrometheusAPIAvailable() {
 		log.Info("reconciling prometheus")
 		if err := r.reconcilePrometheus(cr); err != nil {
 			return err
@@ -981,12 +983,12 @@ func (r *ArgoCDReconciler) setResourceWatches(bldr *builder.Builder, clusterReso
 	// This sets the flags that are used in subsequent checks
 	InspectCluster()
 
-	if IsRouteAPIAvailable() {
+	if networking.IsRouteAPIAvailable() {
 		// Watch OpenShift Route sub-resources owned by ArgoCD instances.
 		bldr.Owns(&routev1.Route{})
 	}
 
-	if IsPrometheusAPIAvailable() {
+	if monitoring.IsPrometheusAPIAvailable() {
 		// Watch Prometheus sub-resources owned by ArgoCD instances.
 		bldr.Owns(&monitoringv1.Prometheus{})
 

controllers/argocd/util_test.go

@@ -12,6 +12,7 @@ import (
 
 	argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
 	"github.com/argoproj-labs/argocd-operator/common"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/util"
 
 	v1 "k8s.io/api/core/v1"
@@ -213,20 +214,16 @@ var argoServerURITests = []struct {
 	},
 }
 
-func setRouteAPIFound(t *testing.T, routeEnabled bool) {
-	routeAPIEnabledTemp := routeAPIFound
-	t.Cleanup(func() {
-		routeAPIFound = routeAPIEnabledTemp
-	})
-	routeAPIFound = routeEnabled
-}
-
 func TestGetArgoServerURI(t *testing.T) {
 	for _, tt := range argoServerURITests {
 		t.Run(tt.name, func(t *testing.T) {
 			cr := makeTestArgoCD(tt.opts...)
 			r := &ArgoCDReconciler{}
-			setRouteAPIFound(t, tt.routeEnabled)
+			networking.SetRouteAPIFound(tt.routeEnabled)
+			defer func() {
+				networking.SetRouteAPIFound(false)
+			}()
+
 			result := r.getArgoServerURI(cr)
 			if result != tt.want {
 				t.Errorf("%s test failed, got=%q want=%q", tt.name, result, tt.want)

main.go

@@ -40,6 +40,8 @@ import (
 	"github.com/argoproj-labs/argocd-operator/controllers/argocd"
 	"github.com/argoproj-labs/argocd-operator/controllers/argocdexport"
 	"github.com/argoproj-labs/argocd-operator/pkg/cluster"
+	"github.com/argoproj-labs/argocd-operator/pkg/monitoring"
+	"github.com/argoproj-labs/argocd-operator/pkg/networking"
 	"github.com/argoproj-labs/argocd-operator/pkg/workloads"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
@@ -113,7 +115,10 @@ func main() {
 	printVersion()
 
 	// Inspect cluster to verify availability of extra features
-	argocd.InspectCluster()
+	err = argocd.InspectCluster()
+	if err != nil {
+		setupLog.Error(err, "error verifying one or more APIs on the cluster")
+	}
 
 	namespace, err := k8sutil.GetWatchNamespace()
 	if err != nil {
@@ -161,15 +166,15 @@ func main() {
 	}
 
 	// Setup Scheme for Prometheus if available.
-	if argocd.IsPrometheusAPIAvailable() {
+	if monitoring.IsPrometheusAPIAvailable() {
 		if err := monitoringv1.AddToScheme(mgr.GetScheme()); err != nil {
 			setupLog.Error(err, "")
 			os.Exit(1)
 		}
 	}
 
 	// Setup Scheme for OpenShift Routes if available.
-	if argocd.IsRouteAPIAvailable() {
+	if networking.IsRouteAPIAvailable() {
 		if err := routev1.Install(mgr.GetScheme()); err != nil {
 			setupLog.Error(err, "")
 			os.Exit(1)