Add support for watching multiple namespaces

Makefile

@@ -3,6 +3,7 @@ OPERATOR_IMAGE ?= appsody/application-operator
 OPERATOR_IMAGE_TAG ?= daily
 
 WATCH_NAMESPACE ?= default
+OPERATOR_NAMESPACE ?= ${WATCH_NAMESPACE}
 
 GIT_COMMIT  ?= $(shell git rev-parse --short HEAD)
 
@@ -11,7 +12,7 @@ SRC_FILES := $(shell find . -type f -name '*.go' -not -path "./vendor/*")
 
 .DEFAULT_GOAL := help
 
-.PHONY: help setup setup-cluster tidy build unit-test test-e2e generate build-image push-image gofmt golint clean install deploy
+.PHONY: help setup setup-cluster tidy build unit-test test-e2e generate build-image push-image gofmt golint clean install-crd install-rbac install-operator install-all uninstall-all
 
 help: 
 	@grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
@@ -58,12 +59,23 @@ golint: ## Run linter on operator code
 clean: ## Clean binary artifacts
 	rm -rf build/_output
 
-install: ## Installs operator CRD in the daily directory
+install-crd: ## Installs operator CRD in the daily directory
 	kubectl apply -f deploy/releases/daily/appsody-app-crd.yaml
 
-deploy: ## Deploys operator across cluster and watches ${WATCH_NAMESPACE} namespace. If ${WATCH_NAMESPACE} is not specified, it defaults to `default` namespace
+install-rbac: ## Installs RBAC objects required for the operator to in a cluster-wide manner
+	sed -i.bak -e "s/APPSODY_OPERATOR_NAMESPACE/${OPERATOR_NAMESPACE}/" deploy/releases/daily/appsody-app-cluster-rbac.yaml
+	kubectl apply -f deploy/releases/daily/appsody-app-cluster-rbac.yaml
+
+install-operator: ## Installs operator in the ${OPERATOR_NAMESPACE} namespace and watches ${WATCH_NAMESPACE} namespace. ${WATCH_NAMESPACE} defaults to `default`. ${OPERATOR_NAMESPACE} defaults to ${WATCH_NAMESPACE}
 ifneq "${OPERATOR_IMAGE}:${OPERATOR_IMAGE_TAG}" "appsody/application-operator:daily"
 	sed -i.bak -e 's!image: appsody/application-operator:daily!image: ${OPERATOR_IMAGE}:${OPERATOR_IMAGE_TAG}!' deploy/releases/daily/appsody-app-operator.yaml
 endif
 	sed -i.bak -e "s/APPSODY_WATCH_NAMESPACE/${WATCH_NAMESPACE}/" deploy/releases/daily/appsody-app-operator.yaml
-	kubectl apply -f deploy/releases/daily/appsody-app-operator.yaml
+	kubectl apply -n ${OPERATOR_NAMESPACE} -f deploy/releases/daily/appsody-app-operator.yaml
+
+install-all: install-crd install-rbac install-operator
+
+uninstall-all:
+	kubectl delete -n ${OPERATOR_NAMESPACE} -f deploy/releases/daily/appsody-app-operator.yaml
+	kubectl delete -f deploy/releases/daily/appsody-app-cluster-rbac.yaml
+	kubectl delete -f deploy/releases/daily/appsody-app-crd.yaml

pkg/controller/appsodyapplication/appsodyapplication_controller.go

@@ -32,10 +32,8 @@ import (
 
 var log = logf.Log.WithName("controller_appsodyapplication")
 
-/**
-* USER ACTION REQUIRED: This is a scaffold file intended for the user to modify with their own Controller
-* business logic.  Delete these comments after modifying this file.*
- */
+// Holds a list of namespaces the operator will be watching
+var watchNamespaces []string
 
 // Add creates a new AppsodyApplication Controller and adds it to the Manager. The Manager will set fields on the Controller
 // and Start it when the Manager is Started.
@@ -48,13 +46,19 @@ func newReconciler(mgr manager.Manager) reconcile.Reconciler {
 	reconciler := &ReconcileAppsodyApplication{ReconcilerBase: appsodyutils.NewReconcilerBase(mgr.GetClient(), mgr.GetScheme(), mgr.GetConfig(), mgr.GetRecorder("appsody-operator")),
 		StackDefaults: map[string]appsodyv1beta1.AppsodyApplicationSpec{}, StackConstants: map[string]*appsodyv1beta1.AppsodyApplicationSpec{}}
 
+	watchNamespaces, err := appsodyutils.GetWatchNamespaces()
+	if err != nil {
+		log.Error(err, "Failed to get watch namespace")
+		os.Exit(1)
+	}
+	log.Info("newReconciler", "watchNamespaces", watchNamespaces)
+
 	ns, err := k8sutil.GetOperatorNamespace()
+	// When running the operator locally, `ns` will be empty string
 	if ns == "" {
-		ns, err = k8sutil.GetWatchNamespace()
-		if err != nil {
-			log.Error(err, "Failed to find a namespace for operator config maps")
-			os.Exit(1)
-		}
+		// If the operator is running locally, use the first namespace in the `watchNamespaces`
+		// `watchNamespaces` must have at least one item
+		ns = watchNamespaces[0]
 	}
 
 	fData, err := ioutil.ReadFile("deploy/stack_defaults.yaml")
@@ -106,23 +110,33 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}
 
+	watchNamespaces, err := appsodyutils.GetWatchNamespaces()
+	if err != nil {
+		log.Error(err, "Failed to get watch namespace")
+		os.Exit(1)
+	}
+
+	watchNamespacesMap := make(map[string]bool)
+	for _, ns := range watchNamespaces {
+		watchNamespacesMap[ns] = true
+	}
+	isClusterWide := len(watchNamespacesMap) == 1 && watchNamespacesMap[""]
+
+	log.V(1).Info("Adding a new controller", "watchNamespaces", watchNamespaces, "isClusterWide", isClusterWide)
+
 	pred := predicate.Funcs{
 		UpdateFunc: func(e event.UpdateEvent) bool {
 			// Ignore updates to CR status in which case metadata.Generation does not change
-			ns, _ := k8sutil.GetWatchNamespace()
-			return e.MetaOld.GetGeneration() != e.MetaNew.GetGeneration() && (ns == "" || e.MetaOld.GetNamespace() == ns)
+			return e.MetaOld.GetGeneration() != e.MetaNew.GetGeneration() && (isClusterWide || watchNamespacesMap[e.MetaOld.GetNamespace()])
 		},
 		CreateFunc: func(e event.CreateEvent) bool {
-			ns, _ := k8sutil.GetWatchNamespace()
-			return ns == "" || e.Meta.GetNamespace() == ns
+			return isClusterWide || watchNamespacesMap[e.Meta.GetNamespace()]
 		},
 		DeleteFunc: func(e event.DeleteEvent) bool {
-			ns, _ := k8sutil.GetWatchNamespace()
-			return ns == "" || e.Meta.GetNamespace() == ns
+			return isClusterWide || watchNamespacesMap[e.Meta.GetNamespace()]
 		},
 		GenericFunc: func(e event.GenericEvent) bool {
-			ns, _ := k8sutil.GetWatchNamespace()
-			return ns == "" || e.Meta.GetNamespace() == ns
+			return isClusterWide || watchNamespacesMap[e.Meta.GetNamespace()]
 		},
 	}
 
@@ -170,11 +184,19 @@ func (r *ReconcileAppsodyApplication) Reconcile(request reconcile.Request) (reco
 	reqLogger.Info("Reconciling AppsodyApplication")
 
 	ns, err := k8sutil.GetOperatorNamespace()
+	// When running the operator locally, `ns` will be empty string
 	if ns == "" {
-		ns, err = k8sutil.GetWatchNamespace()
-		if err != nil {
-			log.Error(err, "Failed to find a namespace for operator config maps")
+		// Since this method can be called directly from unit test, populate `watchNamespaces`.
+		if watchNamespaces == nil {
+			watchNamespaces, err = appsodyutils.GetWatchNamespaces()
+			if err != nil {
+				reqLogger.Error(err, "Error getting watch namespace")
+				return reconcile.Result{}, err
+			}
 		}
+		// If the operator is running locally, use the first namespace in the `watchNamespaces`
+		// `watchNamespaces` must have at least one item
+		ns = watchNamespaces[0]
 	}
 
 	configMap, err := r.GetAppsodyOpConfigMap("appsody-operator-defaults", ns)

pkg/controller/appsodyapplication/appsodyapplication_controller_test.go

@@ -58,6 +58,7 @@ type Test struct {
 func TestAppsodyController(t *testing.T) {
 	// Set the logger to development mode for verbose logs
 	logf.SetLogger(logf.ZapLogger(true))
+	os.Setenv("WATCH_NAMESPACE", namespace)
 
 	spec := appsodyv1beta1.AppsodyApplicationSpec{Stack: stack}
 	appsody := createAppsodyApp(name, namespace, spec)
@@ -259,8 +260,9 @@ func TestAppsodyController(t *testing.T) {
 }
 
 func TestConfigMapDefaults(t *testing.T) {
-	os.Setenv("WATCH_NAMESPACE", namespace)
+	// Set the logger to development mode for verbose logs
 	logf.SetLogger(logf.ZapLogger(true))
+	os.Setenv("WATCH_NAMESPACE", namespace)
 
 	spec := appsodyv1beta1.AppsodyApplicationSpec{Stack: stack, Service: service}
 	appsody := createAppsodyApp(name, namespace, spec)
@@ -301,8 +303,9 @@ func TestConfigMapDefaults(t *testing.T) {
 }
 
 func TestConfigMapConstants(t *testing.T) {
-	os.Setenv("WATCH_NAMESPACE", namespace)
+	// Set the logger to development mode for verbose logs
 	logf.SetLogger(logf.ZapLogger(true))
+	os.Setenv("WATCH_NAMESPACE", namespace)
 
 	spec := appsodyv1beta1.AppsodyApplicationSpec{Stack: stack}
 	appsody := createAppsodyApp(name, namespace, spec)

pkg/utils/utils.go

@@ -7,6 +7,7 @@ import (
 	appsodyv1beta1 "github.com/appsody/appsody-operator/pkg/apis/appsody/v1beta1"
 	servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	routev1 "github.com/openshift/api/route/v1"
+	"github.com/operator-framework/operator-sdk/pkg/k8sutil"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -337,7 +338,7 @@ func InitAndValidate(cr *appsodyv1beta1.AppsodyApplication, defaults appsodyv1be
 		cr.Spec.Service.Type = &st
 	}
 	if cr.Spec.Service.Port == 0 {
-		if defaults.Service.Port != 0 {
+		if defaults.Service != nil && defaults.Service.Port != 0 {
 			cr.Spec.Service.Port = defaults.Service.Port
 		} else {
 			cr.Spec.Service.Port = 8080
@@ -492,3 +493,19 @@ func SetCondition(condition appsodyv1beta1.StatusCondition, status *appsodyv1bet
 
 	status.Conditions = append(status.Conditions, condition)
 }
+
+// GetWatchNamespaces returns a slice of namespaces the operator should watch based on WATCH_NAMESPSCE value
+// WATCH_NAMESPSCE value could be empty for watching the whole cluster or a comma-separated list of namespaces
+func GetWatchNamespaces() ([]string, error) {
+	watchNamespace, err := k8sutil.GetWatchNamespace()
+	if err != nil {
+		return nil, err
+	}
+
+	var watchNamespaces []string
+	for _, ns := range strings.Split(watchNamespace, ",") {
+		watchNamespaces = append(watchNamespaces, strings.TrimSpace(ns))
+	}
+
+	return watchNamespaces, nil
+}

pkg/utils/utils_test.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"os"
 	"reflect"
 	"testing"
 
@@ -448,6 +449,43 @@ func TestSetCondition(t *testing.T) {
 	verifyTests(testSC, t)
 }
 
+func TestGetWatchNamespaces(t *testing.T) {
+	// Set the logger to development mode for verbose logs
+	logf.SetLogger(logf.ZapLogger(true))
+
+	os.Setenv("WATCH_NAMESPACE", "")
+	namespaces, err := GetWatchNamespaces()
+	configMapConstTests := []Test{
+		{"namespaces", []string{""}, namespaces},
+		{"error", nil, err},
+	}
+	verifyTests(configMapConstTests, t)
+
+	os.Setenv("WATCH_NAMESPACE", "ns1")
+	namespaces, err = GetWatchNamespaces()
+	configMapConstTests = []Test{
+		{"namespaces", []string{"ns1"}, namespaces},
+		{"error", nil, err},
+	}
+	verifyTests(configMapConstTests, t)
+
+	os.Setenv("WATCH_NAMESPACE", "ns1,ns2,ns3")
+	namespaces, err = GetWatchNamespaces()
+	configMapConstTests = []Test{
+		{"namespaces", []string{"ns1", "ns2", "ns3"}, namespaces},
+		{"error", nil, err},
+	}
+	verifyTests(configMapConstTests, t)
+
+	os.Setenv("WATCH_NAMESPACE", " ns1   ,  ns2,  ns3  ")
+	namespaces, err = GetWatchNamespaces()
+	configMapConstTests = []Test{
+		{"namespaces", []string{"ns1", "ns2", "ns3"}, namespaces},
+		{"error", nil, err},
+	}
+	verifyTests(configMapConstTests, t)
+}
+
 // Helper Functions
 func createAppsodyApp(n, ns string, spec appsodyv1beta1.AppsodyApplicationSpec) *appsodyv1beta1.AppsodyApplication {
 	app := &appsodyv1beta1.AppsodyApplication{
@@ -459,7 +497,7 @@ func createAppsodyApp(n, ns string, spec appsodyv1beta1.AppsodyApplicationSpec)
 
 func verifyTests(tests []Test, t *testing.T) {
 	for _, tt := range tests {
-		if tt.actual != tt.expected {
+		if !reflect.DeepEqual(tt.actual, tt.expected) {
 			t.Errorf("%s test expected: (%v) actual: (%v)", tt.test, tt.expected, tt.actual)
 		}
 	}