refactor spilo env var generation (zalando#1848)

* refactor spilo env generation
* enhance docs on env vars
* add unit test for appendEnvVar

Author: FxKu

docs/administrator.md

@@ -601,15 +601,39 @@ spec:
 
 ## Custom Pod Environment Variables
 
-It is possible to configure a ConfigMap as well as a Secret which are used by
-the Postgres pods as an additional provider for environment variables. One use
-case is a customized Spilo image configured by extra environment variables.
-Another case could be to provide custom cloud provider or backup settings.
-
-In general the Operator will give preference to the globally configured
-variables, to not have the custom ones interfere with core functionality.
-Variables with the 'WAL_' and 'LOG_' prefix can be overwritten though, to
-allow backup and log shipping to be specified differently.
+The operator will assign a set of environment variables to the database pods
+that cannot be overridden to guarantee core functionality. Only variables with
+'WAL_' and 'LOG_' prefixes can be customized to allow for backup and log
+shipping to be specified differently. There are three ways to specify extra
+environment variables (or override existing ones) for database pods:
+
+* [Via ConfigMap](#via-configmap)
+* [Via Secret](#via-secret)
+* [Via Postgres Cluster Manifest](#via-postgres-cluster-manifest)
+
+The first two options must be referenced from the operator configuration
+making them global settings for all Postgres cluster the operator watches.
+One use case is a customized Spilo image that must be configured by extra
+environment variables. Another case could be to provide custom cloud
+provider or backup settings.
+
+The last options allows for specifying environment variables individual to
+every cluster via the `env` section in the manifest. For example, if you use
+individual backup locations for each of your clusters. Or you want to disable
+WAL archiving for a certain cluster by setting `WAL_S3_BUCKET`, `WAL_GS_BUCKET`
+or `AZURE_STORAGE_ACCOUNT` to an empty string.
+
+The operator will give precedence to environment variables in the following
+order (e.g. a variable defined in 4. overrides a variable with the same name
+in 5.):
+
+1. Assigned by the operator
+2. Clone section (with WAL settings from operator config when `s3_wal_path` is empty)
+3. Standby section
+4. `env` section in cluster manifest
+5. Pod environment secret via operator config
+6. Pod environment config map via operator config
+7. WAL and logical backup settings from operator config
 
 ### Via ConfigMap
 
@@ -706,7 +730,7 @@ data:
 The key-value pairs of the Secret are all accessible as environment variables
 to the Postgres StatefulSet/pods.
 
-### For individual cluster
+### Via Postgres Cluster Manifest
 
 It is possible to define environment variables directly in the Postgres cluster
 manifest to configure it individually. The variables must be listed under the
@@ -951,6 +975,10 @@ When the `AWS_REGION` is set, `AWS_ENDPOINT` and `WALE_S3_ENDPOINT` are
 generated automatically. `WALG_S3_PREFIX` is identical to `WALE_S3_PREFIX`.
 `SCOPE` is the Postgres cluster name.
 
+:warning: If both `AWS_REGION` and `AWS_ENDPOINT` or `WALE_S3_ENDPOINT` are
+defined backups with WAL-E will fail. You can fix it by switching to WAL-G
+with `USE_WALG_BACKUP: "true"`.
+
 ### Google Cloud Platform setup
 
 To configure the operator on GCP these prerequisites that are needed:

docs/reference/operator_parameters.md

@@ -645,7 +645,10 @@ yet officially supported.
   empty.
 
 * **aws_region**
-  AWS region used to store EBS volumes. The default is `eu-central-1`.
+  AWS region used to store EBS volumes. The default is `eu-central-1`. Note,
+  this option is not meant for specifying the AWS region for backups and
+  restore, since it can be separate from the EBS region. You have to define
+  AWS_REGION as a [custom environment variable](../administrator.md#custom-pod-environment-variables).
 
 * **additional_secret_mount**
   Additional Secret (aws or gcp credentials) to mount in the pod.

docs/user.md

@@ -766,15 +766,15 @@ spec:
     uid: "efd12e58-5786-11e8-b5a7-06148230260c"
     cluster: "acid-minimal-cluster"
     timestamp: "2017-12-19T12:40:33+01:00"
-    s3_wal_path: "s3://<bucketname>/spilo/<source_db_cluster>/<UID>/wal/<PGVERSION>"
 ```
 
 Here `cluster` is a name of a source cluster that is going to be cloned. A new
 cluster will be cloned from S3, using the latest backup before the `timestamp`.
-Note, that a time zone is required for `timestamp` in the format of +00:00 which
-is UTC. You can specify the `s3_wal_path` of the source cluster or let the
-operator try to find it based on the configured `wal_[s3|gs]_bucket` and the
-specified `uid`. You can find the UID of the source cluster in its metadata:
+Note, a time zone is required for `timestamp` in the format of `+00:00` (UTC).
+
+The operator will try to find the WAL location based on the configured
+`wal_[s3|gs]_bucket` or `wal_az_storage_account` and the specified `uid`.
+You can find the UID of the source cluster in its metadata:
 
 ```yaml
 apiVersion: acid.zalan.do/v1
@@ -784,6 +784,14 @@ metadata:
   uid: efd12e58-5786-11e8-b5a7-06148230260c
 ```
 
+If your source cluster uses a WAL location different from the global
+configuration you can specify the full path under `s3_wal_path`. For
+[Google Cloud Platform](administrator.md#google-cloud-platform-setup)
+or [Azure](administrator.md#azure-setup)
+it can only be set globally with [custom Pod environment variables](administrator.md#custom-pod-environment-variables)
+or locally in the Postgres manifest's [`env`](administrator.md#via-postgres-cluster-manifest) section.
+
+
 For non AWS S3 following settings can be set to support cloning from other S3
 implementations:
 
@@ -793,6 +801,7 @@ spec:
     uid: "efd12e58-5786-11e8-b5a7-06148230260c"
     cluster: "acid-minimal-cluster"
     timestamp: "2017-12-19T12:40:33+01:00"
+    s3_wal_path: "s3://custom/path/to/bucket"
     s3_endpoint: https://s3.acme.org
     s3_access_key_id: 0123456789abcdef0123456789abcdef
     s3_secret_access_key: 0123456789abcdef0123456789abcdef
@@ -864,22 +873,28 @@ the PostgreSQL version between source and target cluster has to be the same.
 
 To start a cluster as standby, add the following `standby` section in the YAML
 file. You can stream changes from archived WAL files (AWS S3 or Google Cloud
-Storage) or from a remote primary where you specify the host address and port.
-If you leave out the port, Patroni will use `"5432"`. Only one option can be
-specfied in the manifest:
+Storage) or from a remote primary. Only one option can be specfied in the
+manifest:
 
 ```yaml
 spec:
   standby:
     s3_wal_path: "s3://<bucketname>/spilo/<source_db_cluster>/<UID>/wal/<PGVERSION>"
 ```
 
+For GCS, you have to define STANDBY_GOOGLE_APPLICATION_CREDENTIALS as a
+[custom pod environment variable](administrator.md#custom-pod-environment-variables).
+It is not set from the config to allow for overridding.
+
 ```yaml
 spec:
   standby:
     gs_wal_path: "gs://<bucketname>/spilo/<source_db_cluster>/<UID>/wal/<PGVERSION>"
 ```
 
+For a remote primary you specify the host address and optionally the port.
+If you leave out the port Patroni will use `"5432"`.
+
 ```yaml
 spec:
   standby:

pkg/cluster/cluster.go

@@ -1318,7 +1318,7 @@ func (c *Cluster) initAdditionalOwnerRoles() {
 			}
 		}
 
-		if len(memberOf) > 1 {
+		if len(memberOf) > 0 {
 			namespace := c.Namespace
 			additionalOwnerPgUser := spec.PgUser{
 				Origin:    spec.RoleOriginSpilo,

pkg/cluster/cluster_test.go

@@ -26,6 +26,8 @@ import (
 const (
 	superUserName       = "postgres"
 	replicationUserName = "standby"
+	exampleSpiloConfig  = `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_connections":"100","max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`
+	spiloConfigDiff     = `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`
 )
 
 var logger = logrus.New().WithField("test", "cluster")
@@ -957,7 +959,7 @@ func TestCompareEnv(t *testing.T) {
 				},
 				{
 					Name:  "SPILO_CONFIGURATION",
-					Value: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_connections":"100","max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`,
+					Value: exampleSpiloConfig,
 				},
 			},
 			ExpectedResult: true,
@@ -978,7 +980,7 @@ func TestCompareEnv(t *testing.T) {
 				},
 				{
 					Name:  "SPILO_CONFIGURATION",
-					Value: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`,
+					Value: spiloConfigDiff,
 				},
 			},
 			ExpectedResult: true,
@@ -999,7 +1001,7 @@ func TestCompareEnv(t *testing.T) {
 				},
 				{
 					Name:  "SPILO_CONFIGURATION",
-					Value: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`,
+					Value: exampleSpiloConfig,
 				},
 			},
 			ExpectedResult: false,
@@ -1024,7 +1026,7 @@ func TestCompareEnv(t *testing.T) {
 				},
 				{
 					Name:  "SPILO_CONFIGURATION",
-					Value: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_connections":"100","max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`,
+					Value: exampleSpiloConfig,
 				},
 			},
 			ExpectedResult: false,
@@ -1041,7 +1043,7 @@ func TestCompareEnv(t *testing.T) {
 				},
 				{
 					Name:  "SPILO_CONFIGURATION",
-					Value: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/12/bin","parameters":{"autovacuum_analyze_scale_factor":"0.1"},"pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"test":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_connections":"100","max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`,
+					Value: exampleSpiloConfig,
 				},
 			},
 			ExpectedResult: false,