fix: parametrize the namespace of the prometheus operator

This commit allows running the prometheus operator in a namespace other
than default. While this was possible before, it was broken because the
service account namespace in the cluster role binding was always set as
default.

When installed through OLM, the namespace argument will be set as the current
namespace. This means that the prometheus operator will be deployed in
the same namespace as the monitoring stack operator.

assets/prometheus-operator/deployment.yaml

@@ -1,7 +1,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: prometheus-operator
+  name: monitoring-stack-operator-prometheus-operator
 spec:
   replicas: 1
   selector:
@@ -20,7 +20,7 @@ spec:
         app.kubernetes.io/part-of: monitoring-stack-operator
         app.kubernetes.io/version: 0.50.0
     spec:
-      serviceAccountName: prometheus-operator
+      serviceAccountName: monitoring-stack-operator-prometheus-operator
       containers:
         - args:
             - --prometheus-config-reloader=quay.io/prometheus-operator/prometheus-config-reloader:v0.50.0

assets/prometheus-operator/service-account.yaml

@@ -5,4 +5,4 @@ metadata:
     app.kubernetes.io/component: controller
     app.kubernetes.io/name: prometheus-operator
     app.kubernetes.io/part-of: monitoring-stack-monitoring
-  name: prometheus-operator
+  name: monitoring-stack-operator-prometheus-operator

cmd/operator/main.go

@@ -48,6 +48,10 @@ func main() {
 
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 
+	setupLog.Info("running with arguments",
+		"namespace", namespace,
+		"metrics-bind-address", metricsAddr)
+
 	poOpts := poctrl.Options{
 		Namespace:  namespace,
 		AssetsPath: "./assets/prometheus-operator/",

deploy/olm/kustomization.yaml

@@ -10,11 +10,12 @@ images:
   - name: monitoring-stack-operator
     newName: quay.io/sthaha/monitoring-stack-operator
     newTag: 0.0.1
+
 patches:
 - patch: |-
     - op: add
-      path: /spec/template/spec/containers/0/args
-      value: [--deploy-prometheus-operator-crds=false]
+      path: /spec/template/spec/containers/0/args/-
+      value: --deploy-prometheus-operator-crds=false
   target:
     kind: Deployment
     name: monitoring-stack-operator

deploy/operator/deployment.yaml

@@ -26,6 +26,13 @@ spec:
         - name: operator
           image: monitoring-stack-operator:0.0.1
           imagePullPolicy: IfNotPresent
+          args:
+            - --namespace=$(NAMESPACE)
+          env:
+          - name: NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
           securityContext:
             allowPrivilegeEscalation: false
           resources:

pkg/controllers/prometheus-operator/controller.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"rhobs/monitoring-stack-operator/pkg/assets"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/go-logr/logr"
@@ -71,6 +73,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request ctrlruntime.Request)
 		resource.SetNamespace(r.opts.Namespace)
 		r.logger.Info("Reconciling resource",
 			"Kind", resource.GetObjectKind().GroupVersionKind().Kind,
+			"Namespace", resource.GetNamespace(),
 			"Name", resource.GetName())
 		if err := r.k8sClient.Patch(ctx, resource, client.Apply, fieldOwner); err != nil {
 			return reconcile.Result{}, err
@@ -81,7 +84,7 @@ func (r *reconciler) Reconcile(ctx context.Context, request ctrlruntime.Request)
 }
 
 func (r *reconciler) loadStaticResources() ([]client.Object, error) {
-	resources := []assets.Asset{
+	staticAssets := []assets.Asset{
 		{
 			File:   "service-account.yaml",
 			Object: &v1.ServiceAccount{},
@@ -90,10 +93,6 @@ func (r *reconciler) loadStaticResources() ([]client.Object, error) {
 			File:   "cluster-role.yaml",
 			Object: &authorizationv1.ClusterRole{},
 		},
-		{
-			File:   "cluster-role-binding.yaml",
-			Object: &authorizationv1.ClusterRoleBinding{},
-		},
 		{
 			File:   "deployment.yaml",
 			Object: &appsv.Deployment{},
@@ -111,8 +110,36 @@ func (r *reconciler) loadStaticResources() ([]client.Object, error) {
 			assets.NewCRDAsset("crds/servicemonitors.yaml"),
 			assets.NewCRDAsset("crds/thanosrulers.yaml"),
 		}
-		resources = append(crds, resources...)
+		staticAssets = append(crds, staticAssets...)
+	}
+
+	resources, err := r.assetLoader.Load(staticAssets)
+	if err != nil {
+		return nil, err
+	}
+
+	crb := &authorizationv1.ClusterRoleBinding{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "rbac.authorization.k8s.io/v1",
+			Kind:       "ClusterRoleBinding",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "monitoring-stack-operator-prometheus-operator",
+		},
+		Subjects: []authorizationv1.Subject{
+			{
+				Kind:      "ServiceAccount",
+				Name:      "monitoring-stack-operator-prometheus-operator",
+				Namespace: r.opts.Namespace,
+			},
+		},
+		RoleRef: authorizationv1.RoleRef{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "ClusterRole",
+			Name:     "monitoring-stack-operator-prometheus-operator",
+		},
 	}
+	resources = append(resources, crb)
 
-	return r.assetLoader.Load(resources)
+	return resources, nil
 }

test/e2e/main_test.go

@@ -9,25 +9,45 @@ import (
 	"rhobs/monitoring-stack-operator/test/e2e/framework"
 	"testing"
 
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 var (
 	f *framework.Framework
-
-	// TODO(fpetkovski): change once we are able to deploy the operator to a different namespace
-	e2eTestNamespace = "default"
 )
 
+const e2eTestNamespace = "e2e-tests"
+
 func TestMain(m *testing.M) {
+	// Deferred calls are not executed on os.Exit from TestMain.
+	// As a workaround, we call another function in which we can add deferred calls.
+	// http://blog.englund.nu/golang,/testing/2017/03/12/using-defer-in-testmain.html
+	code := main(m)
+	os.Exit(code)
+}
+
+func main(m *testing.M) int {
 	setLogger()
-	op := createOperator()
+	op, err := createOperator()
+	if err != nil {
+		log.Println(err)
+		return 1
+	}
 	setupFramework(op)
 
+	cleanup, err := createTestNamespace()
+	if err != nil {
+		log.Println(err)
+		return 1
+	}
+	defer cleanup()
+
 	go runOperator(op, ctrl.SetupSignalHandler())
-	m.Run()
-	os.Exit(0)
+	return m.Run()
 }
 
 func runOperator(op *operator.Operator, ctx context.Context) {
@@ -44,20 +64,42 @@ func setupFramework(op *operator.Operator) {
 }
 
 func setLogger() {
-	opts := zap.Options{}
+	opts := zap.Options{
+		Development: true,
+	}
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
 }
 
-func createOperator() *operator.Operator {
+func createOperator() (*operator.Operator, error) {
 	op, err := operator.New("", prometheus_operator.Options{
 		Namespace:  e2eTestNamespace,
 		AssetsPath: "../../assets/prometheus-operator/",
 		DeployCRDs: true,
 	})
 	if err != nil {
-		log.Println(err)
-		os.Exit(1)
+		return nil, err
+	}
+
+	return op, nil
+}
+
+func createTestNamespace() (func(), error) {
+	ns := &v1.Namespace{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "v1",
+			Kind:       "Namespace",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name: e2eTestNamespace,
+		},
+	}
+	if err := f.K8sClient.Create(context.Background(), ns); err != nil {
+		return nil, err
+	}
+
+	cleanup := func() {
+		f.K8sClient.Delete(context.Background(), ns)
 	}
 
-	return op
+	return cleanup, nil
 }

test/e2e/prometheus_operator_test.go

@@ -49,9 +49,9 @@ func TestPrometheusOperatorForNonOwnedResources(t *testing.T) {
 		{
 			name: "Operator should not reconcile resources which it does not own",
 			scenario: func(t *testing.T) {
-				f.AssertResourceNeverExists(prometheusStsName, "default", &appsv1.StatefulSet{})(t)
-				f.AssertResourceNeverExists(alertmanagerStsName, "default", &appsv1.StatefulSet{})(t)
-				f.AssertResourceNeverExists(thanosRulerStsName, "default", &appsv1.StatefulSet{})(t)
+				f.AssertResourceNeverExists(prometheusStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
+				f.AssertResourceNeverExists(alertmanagerStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
+				f.AssertResourceNeverExists(thanosRulerStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
 			},
 		},
 	}
@@ -77,9 +77,9 @@ func TestPrometheusOperatorForOwnedResources(t *testing.T) {
 		{
 			name: "Operator should reconcile resources which it does owns",
 			scenario: func(t *testing.T) {
-				f.AssertResourceEventuallyExists(prometheusStsName, "default", &appsv1.StatefulSet{})(t)
-				f.AssertResourceEventuallyExists(alertmanagerStsName, "default", &appsv1.StatefulSet{})(t)
-				f.AssertResourceEventuallyExists(thanosRulerStsName, "default", &appsv1.StatefulSet{})(t)
+				f.AssertResourceEventuallyExists(prometheusStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
+				f.AssertResourceEventuallyExists(alertmanagerStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
+				f.AssertResourceEventuallyExists(thanosRulerStsName, e2eTestNamespace, &appsv1.StatefulSet{})(t)
 			},
 		},
 	}