Introduce PGVERSION (#1172)

* introduce PGVERSION

Co-authored-by: Sergey Dudoladov <sergey.dudoladov@zalando.de>

charts/postgres-operator/values-crd.yaml

@@ -21,6 +21,8 @@ configGeneral:
   enable_crd_validation: true
   # update only the statefulsets without immediately doing the rolling update
   enable_lazy_spilo_upgrade: false
+  # set the PGVERSION env var instead of providing the version via postgresql.bin_dir in SPILO_CONFIGURATION
+  enable_pgversion_env_var: "false"
   # start any new database pod without limitations on shm memory
   enable_shm_volume: true
   # etcd connection string for Patroni. Empty uses K8s-native DCS.

charts/postgres-operator/values.yaml

@@ -24,6 +24,8 @@ configGeneral:
   enable_crd_validation: "true"
   # update only the statefulsets without immediately doing the rolling update
   enable_lazy_spilo_upgrade: "false"
+  # set the PGVERSION env var instead of providing the version via postgresql.bin_dir in SPILO_CONFIGURATION
+  enable_pgversion_env_var: "false"
   # start any new database pod without limitations on shm memory
   enable_shm_volume: "true"
   # etcd connection string for Patroni. Empty uses K8s-native DCS.

docs/reference/operator_parameters.md

@@ -118,6 +118,9 @@ Those are top-level keys, containing both leaf keys and groups.
   This option is global for an operator object, and can be overwritten by
   `enableShmVolume` parameter from Postgres manifest. The default is `true`.
 
+* **enable_pgversion_env_var**
+  With newer versions of Spilo, it is preferable to use `PGVERSION` pod environment variable instead of the setting `postgresql.bin_dir` in the `SPILO_CONFIGURATION` env variable. When this option is true, the operator sets `PGVERSION` and omits `postgresql.bin_dir` from  `SPILO_CONFIGURATION`. When false, the `postgresql.bin_dir` is set. This setting takes precedence over `PGVERSION`; see PR 222 in Spilo. The default is `false`.
+
 * **workers**
   number of working routines the operator spawns to process requests to
   create/update/delete/sync clusters concurrently. The default is `4`.

e2e/tests/test_e2e.py

@@ -1080,7 +1080,7 @@ class EndToEndTestCase(unittest.TestCase):
                 "enable_pod_antiaffinity": "false"
             }
         }
-        k8s.update_config(patch_disable_antiaffinity, "disalbe antiaffinity")
+        k8s.update_config(patch_disable_antiaffinity, "disable antiaffinity")
         k8s.wait_for_pod_start('spilo-role=master')
         k8s.wait_for_pod_start('spilo-role=replica')
         return True

manifests/configmap.yaml

@@ -45,6 +45,7 @@ data:
   # enable_postgres_team_crd_superusers: "false"
   enable_replica_load_balancer: "false"
   # enable_shm_volume: "true"
+  # enable_pgversion_env_var: "false"
   # enable_sidecars: "true"
   # enable_team_superuser: "false"
   enable_teams_api: "false"

manifests/operatorconfiguration.crd.yaml

@@ -65,6 +65,8 @@ spec:
                 type: boolean
               enable_lazy_spilo_upgrade:
                 type: boolean
+              enable_pgversion_env_var:
+                type: boolean
               enable_shm_volume:
                 type: boolean
               etcd_host:

pkg/apis/acid.zalan.do/v1/operator_configuration_type.go

@@ -167,7 +167,7 @@ type ScalyrConfiguration struct {
 	ScalyrMemoryLimit   string `json:"scalyr_memory_limit,omitempty"`
 }
 
-// Defines default configuration for connection pooler
+// ConnectionPoolerConfiguration defines default configuration for connection pooler
 type ConnectionPoolerConfiguration struct {
 	NumberOfInstances    *int32 `json:"connection_pooler_number_of_instances,omitempty"`
 	Schema               string `json:"connection_pooler_schema,omitempty"`
@@ -197,6 +197,7 @@ type OperatorLogicalBackupConfiguration struct {
 type OperatorConfigurationData struct {
 	EnableCRDValidation        *bool                              `json:"enable_crd_validation,omitempty"`
 	EnableLazySpiloUpgrade     bool                               `json:"enable_lazy_spilo_upgrade,omitempty"`
+	EnablePgVersionEnvVar      bool                               `json:"enable_pgversion_env_var,omitempty"`
 	EtcdHost                   string                             `json:"etcd_host,omitempty"`
 	KubernetesUseConfigMaps    bool                               `json:"kubernetes_use_configmaps,omitempty"`
 	DockerImage                string                             `json:"docker_image,omitempty"`

pkg/cluster/k8sres.go

@@ -189,7 +189,7 @@ func fillResourceList(spec acidv1.ResourceDescription, defaults acidv1.ResourceD
 	return requests, nil
 }
 
-func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, pamRoleName string, logger *logrus.Entry) (string, error) {
+func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, pamRoleName string, EnablePgVersionEnvVar bool, logger *logrus.Entry) (string, error) {
 	config := spiloConfiguration{}
 
 	config.Bootstrap = pgBootstrap{}
@@ -270,7 +270,14 @@ PatroniInitDBParams:
 	}
 
 	config.PgLocalConfiguration = make(map[string]interface{})
+
+	// the newer and preferred way to specify the PG version is to use the `PGVERSION` env variable
+	// setting postgresq.bin_dir in the SPILO_CONFIGURATION still works and takes precedence over PGVERSION
+	// so we add postgresq.bin_dir only if PGVERSION is unused
+	// see PR 222 in Spilo
+	if !EnablePgVersionEnvVar {
 		config.PgLocalConfiguration[patroniPGBinariesParameterName] = fmt.Sprintf(pgBinariesLocationTemplate, pg.PgVersion)
+	}
 	if len(pg.Parameters) > 0 {
 		local, bootstrap := getLocalAndBoostrapPostgreSQLParameters(pg.Parameters)
 
@@ -696,6 +703,9 @@ func (c *Cluster) generateSpiloPodEnvVars(uid types.UID, spiloConfiguration stri
 			Value: c.OpConfig.PamRoleName,
 		},
 	}
+	if c.OpConfig.EnablePgVersionEnvVar {
+		envVars = append(envVars, v1.EnvVar{Name: "PGVERSION", Value: c.Spec.PgVersion})
+	}
 	// Spilo expects cluster labels as JSON
 	if clusterLabels, err := json.Marshal(labels.Set(c.OpConfig.ClusterLabels)); err != nil {
 		envVars = append(envVars, v1.EnvVar{Name: "KUBERNETES_LABELS", Value: labels.Set(c.OpConfig.ClusterLabels).String()})
@@ -1012,7 +1022,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
 		}
 	}
 
-	spiloConfiguration, err := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.logger)
+	spiloConfiguration, err := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.OpConfig.EnablePgVersionEnvVar, c.logger)
 	if err != nil {
 		return nil, fmt.Errorf("could not generate Spilo JSON configuration: %v", err)
 	}

pkg/cluster/k8sres_test.go

@@ -93,7 +93,7 @@ func TestGenerateSpiloJSONConfiguration(t *testing.T) {
 	}
 	for _, tt := range tests {
 		cluster.OpConfig = tt.opConfig
-		result, err := generateSpiloJSONConfiguration(tt.pgParam, tt.patroni, tt.role, logger)
+		result, err := generateSpiloJSONConfiguration(tt.pgParam, tt.patroni, tt.role, false, logger)
 		if err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}

pkg/controller/operator_config.go

@@ -35,6 +35,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	// general config
 	result.EnableCRDValidation = util.CoalesceBool(fromCRD.EnableCRDValidation, util.True())
 	result.EnableLazySpiloUpgrade = fromCRD.EnableLazySpiloUpgrade
+	result.EnablePgVersionEnvVar = fromCRD.EnablePgVersionEnvVar
 	result.EtcdHost = fromCRD.EtcdHost
 	result.KubernetesUseConfigMaps = fromCRD.KubernetesUseConfigMaps
 	result.DockerImage = util.Coalesce(fromCRD.DockerImage, "registry.opensource.zalan.do/acid/spilo-12:1.6-p3")

pkg/util/config/config.go

@@ -197,6 +197,7 @@ type Config struct {
 	PostgresSuperuserTeams                 []string          `name:"postgres_superuser_teams" default:""`
 	SetMemoryRequestToLimit                bool              `name:"set_memory_request_to_limit" default:"false"`
 	EnableLazySpiloUpgrade                 bool              `name:"enable_lazy_spilo_upgrade" default:"false"`
+	EnablePgVersionEnvVar                  bool              `name:"enable_pgversion_env_var" default:"false"`
 }
 
 // MustMarshal marshals the config or panics