Respond to code review

manifests/configmap.yaml

@@ -4,8 +4,8 @@ metadata:
   name: postgres-operator
 data:
   # the env var with the same name in the operator pod may overwrite this value
-  # if neither is set, listen to the 'default' namespace
+  # if neither is set or evaluates to the empty string, listen to the operator's own namespace
-  # if set to the empty string "", listen to all namespaces
+  # if set to the "*", listen to all namespaces
   # watched_namespace: development
   service_account_name: operator
   cluster_labels: application:spilo

manifests/postgres-operator.yaml

@@ -16,7 +16,8 @@ spec:
         imagePullPolicy: IfNotPresent
         env:
         # uncomment to overwrite a similar setting from operator configmap
-        # if set to the empty string, watch the 'default' namespace
+        # if set to the empty string, watch the operator's own namespace
+        # if set to the "*", listen to all namespaces
         # - name: WATCHED_NAMESPACE
         #  valueFrom:
         #    fieldRef:

pkg/apiserver/apiserver.go

@@ -48,9 +48,9 @@ type Server struct {
 }
 
 var (
-	clusterStatusURL     = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-zA-Z][a-zA-Z0-9-]*)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/?$`)
+	clusterStatusURL     = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-z0-9]([-a-z0-9]*[a-z0-9])?)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/?$`)
-	clusterLogsURL       = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-zA-Z][a-zA-Z0-9-]*)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/logs/?$`)
+	clusterLogsURL       = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-z0-9]([-a-z0-9]*[a-z0-9])?)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/logs/?$`)
-	clusterHistoryURL    = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-zA-Z][a-zA-Z0-9-]*)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/history/?$`)
+	clusterHistoryURL    = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/(?P<namespace>[a-z0-9]([-a-z0-9]*[a-z0-9])?)/(?P<cluster>[a-zA-Z][a-zA-Z0-9-]*)/history/?$`)
 	teamURL              = regexp.MustCompile(`^/clusters/(?P<team>[a-zA-Z][a-zA-Z0-9]*)/?$`)
 	workerLogsURL        = regexp.MustCompile(`^/workers/(?P<id>\d+)/logs/?$`)
 	workerEventsQueueURL = regexp.MustCompile(`^/workers/(?P<id>\d+)/queue/?$`)

pkg/controller/controller.go

@@ -14,6 +14,7 @@ import (
 	"github.com/zalando-incubator/postgres-operator/pkg/apiserver"
 	"github.com/zalando-incubator/postgres-operator/pkg/cluster"
 	"github.com/zalando-incubator/postgres-operator/pkg/spec"
+	"github.com/zalando-incubator/postgres-operator/pkg/util"
 	"github.com/zalando-incubator/postgres-operator/pkg/util/config"
 	"github.com/zalando-incubator/postgres-operator/pkg/util/constants"
 	"github.com/zalando-incubator/postgres-operator/pkg/util/k8sutil"
@@ -97,41 +98,7 @@ func (c *Controller) initOperatorConfig() {
 		c.logger.Infoln("no ConfigMap specified. Loading default values")
 	}
 
-	watchedNsConfigMapVar, isPresentInOperatorConfigMap := configMapData["watched_namespace"]
+	configMapData["watched_namespace"] = c.getEffectiveNamespace(os.Getenv("WATCHED_NAMESPACE"), configMapData["watched_namespace"])
-	watchedNsEnvVar, isPresentInOperatorEnv := os.LookupEnv("WATCHED_NAMESPACE")
-
-	if (!isPresentInOperatorConfigMap) && (!isPresentInOperatorEnv) {
-
-		c.logger.Infof("No namespace to watch specified. By convention, the operator falls back to watching the namespace it is deployed to: '%v' \n", spec.GetOperatorNamespace())
-		configMapData["watched_namespace"] = spec.GetOperatorNamespace()
-
-	}
-
-	if (isPresentInOperatorConfigMap) && (!isPresentInOperatorEnv) {
-
-		// explicitly specify the policy for handling all namespaces
-		// note that '*' is not a valid namespace name
-		if watchedNsConfigMapVar == "*" {
-			c.logger.Infof("The watched namespace field in the operator config map evaluates to '*', meaning watching all namespaces.\n")
-			configMapData["watched_namespace"] = v1.NamespaceAll
-		}
-	}
-
-	if isPresentInOperatorEnv {
-
-		if isPresentInOperatorConfigMap {
-			c.logger.Infof("Both WATCHED_NAMESPACE=%q env var and wacthed_namespace=%q field in operator config map are defined. The env variable takes priority over the configMap param\n", watchedNsEnvVar, watchedNsConfigMapVar)
-		}
-
-		// handle all namespaces consistently
-		if watchedNsEnvVar == "*" {
-			c.logger.Infof("The watched namespace field in the operator config map evaluates to '*', meaning watching all namespaces.\n")
-			configMapData["watched_namespace"] = v1.NamespaceAll
-		} else {
-			c.logger.Infof("Watch the %q namespace specified in the env variable WATCHED_NAMESPACE\n", watchedNsEnvVar)
-			configMapData["watched_namespace"] = watchedNsEnvVar
-		}
-	}
 
 	if c.config.NoDatabaseAccess {
 		configMapData["enable_database_access"] = "false"
@@ -152,15 +119,6 @@ func (c *Controller) initController() {
 	c.initClients()
 	c.initOperatorConfig()
 
-	// earliest point where we can check if the namespace to watch actually exists
-	if c.opConfig.WatchedNamespace != v1.NamespaceAll {
-		_, err := c.KubeClient.Namespaces().Get(c.opConfig.WatchedNamespace, metav1.GetOptions{})
-		if err != nil {
-			c.logger.Fatalf("Operator was told to watch the %q namespace but was unable to find it via Kubernetes API.", c.opConfig.WatchedNamespace)
-		}
-
-	}
-
 	c.initSharedInformers()
 
 	c.logger.Infof("config: %s", c.opConfig.MustMarshal())
@@ -290,3 +248,26 @@ func (c *Controller) kubeNodesInformer(stopCh <-chan struct{}, wg *sync.WaitGrou
 
 	c.nodesInformer.Run(stopCh)
 }
+
+func (c *Controller) getEffectiveNamespace(namespaceFromEnvironment, namespaceFromConfigMap string) string {
+
+	namespace := util.Coalesce(namespaceFromEnvironment, util.Coalesce(namespaceFromConfigMap, spec.GetOperatorNamespace()))
+
+	if namespace == "*" {
+
+		namespace = v1.NamespaceAll
+		c.logger.Infof("Listening to all namespaces")
+
+	} else {
+
+		if _, err := c.KubeClient.Namespaces().Get(namespace, metav1.GetOptions{}); err != nil {
+			// the namespace may be created manually at runtime
+			c.logger.Warnf("Could not find the watched namespace %q", namespace)
+		} else {
+			c.logger.Infof("Listenting to the specific namespace %q", namespace)
+		}
+
+	}
+
+	return namespace
+}

pkg/controller/status.go

@@ -16,10 +16,6 @@ import (
 // ClusterStatus provides status of the cluster
 func (c *Controller) ClusterStatus(team, namespace, cluster string) (*spec.ClusterStatus, error) {
 
-	if namespace == "" {
-		namespace = c.opConfig.WatchedNamespace
-	}
-
 	clusterName := spec.NamespacedName{
 		Namespace: namespace,
 		Name:      team + "-" + cluster,
@@ -97,10 +93,6 @@ func (c *Controller) GetStatus() *spec.ControllerStatus {
 // ClusterLogs dumps cluster ring logs
 func (c *Controller) ClusterLogs(team, namespace, name string) ([]*spec.LogEntry, error) {
 
-	if namespace == "" {
-		namespace = c.opConfig.WatchedNamespace
-	}
-
 	clusterName := spec.NamespacedName{
 		Namespace: namespace,
 		Name:      team + "-" + name,
@@ -224,10 +216,6 @@ func (c *Controller) WorkerStatus(workerID uint32) (*spec.WorkerStatus, error) {
 // ClusterHistory dumps history of cluster changes
 func (c *Controller) ClusterHistory(team, namespace, name string) ([]*spec.Diff, error) {
 
-	if namespace == "" {
-		namespace = c.opConfig.WatchedNamespace
-	}
-
 	clusterName := spec.NamespacedName{
 		Namespace: namespace,
 		Name:      team + "-" + name,

pkg/util/config/config.go

@@ -67,7 +67,7 @@ type Config struct {
 	Resources
 	Auth
 	Scalyr
-	WatchedNamespace         string            `name:"watched_namespace"` // may be v1.NamespaceAll, meaning watch all namespaces
+	WatchedNamespace         string            `name:"watched_namespace"` // may be "*", meaning watch all namespaces
 	EtcdHost                 string            `name:"etcd_host" default:"etcd-client.default.svc.cluster.local:2379"`
 	DockerImage              string            `name:"docker_image" default:"registry.opensource.zalan.do/acid/spiloprivate-9.6:1.2-p4"`
 	ServiceAccountName       string            `name:"service_account_name" default:"operator"`

pkg/util/util.go

@@ -119,3 +119,10 @@ func MapContains(haystack, needle map[string]string) bool {
 
 	return true
 }
+
+func Coalesce(val, defaultVal string) string {
+	if val == "" {
+		return defaultVal
+	}
+	return val
+}