From c5b3cc9c3aeedab392b1eb098c8cc56c1e782130 Mon Sep 17 00:00:00 2001 From: Pavel Zaytsev Date: Wed, 12 Nov 2025 14:53:33 -0800 Subject: [PATCH 1/4] Allow user env vars to override operator-generated ones This change modifies appendEnvVars() to allow environment variables defined in the PostgreSQL CRD spec.env to override operator-generated environment variables (like SPILO_CONFIGURATION) instead of being silently ignored. Previously, if an env var already existed in the list, user-provided values were skipped. Now, user values take precedence and replace the operator-generated ones. This enables users to customize SPILO_CONFIGURATION and other operator-managed env vars through the CRD, which is useful for adding custom Patroni DCS configuration like ignore_slots. --- pkg/cluster/k8sres.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e05a54553..0c316aead 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1088,7 +1088,20 @@ func (c *Cluster) generateSpiloPodEnvVars( func appendEnvVars(envs []v1.EnvVar, appEnv ...v1.EnvVar) []v1.EnvVar { collectedEnvs := envs for _, env := range appEnv { - if !isEnvVarPresent(collectedEnvs, env.Name) { + // Check if env var already exists + existingIdx := -1 + for i, existing := range collectedEnvs { + if strings.EqualFold(existing.Name, env.Name) { + existingIdx = i + break + } + } + + if existingIdx >= 0 { + // Replace existing env var (user override takes precedence) + collectedEnvs[existingIdx] = env + } else { + // Add new env var collectedEnvs = append(collectedEnvs, env) } } From c7d481740a44a427d36d3b3e7eef673b16d5fea6 Mon Sep 17 00:00:00 2001 From: Pavel Zaytsev Date: Wed, 10 Dec 2025 15:32:46 -0800 Subject: [PATCH 2/4] Add e2e test for user env var override functionality This test verifies that user-provided environment variables from spec.env can override operator-generated ones (like SPILO_CONFIGURATION), which is useful for customizing Patroni DCS configuration such as ignore_slots. --- e2e/tests/k8s_api.py | 10 +++++++ e2e/tests/test_e2e.py | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 1f42ad4bc..12ae97189 100644 --- a/e2e/tests/k8s_api.py +++ b/e2e/tests/k8s_api.py @@ -177,6 +177,16 @@ def count_pods_with_env_variable(self, env_variable_key, labels, namespace='defa return pod_count + def get_env_variable_value(self, env_variable_key, labels, namespace='default'): + """Get the value of an environment variable from the first pod matching the labels.""" + pods = self.api.core_v1.list_namespaced_pod(namespace, label_selector=labels).items + if not pods: + return None + for env in pods[0].spec.containers[0].env: + if env.name == env_variable_key: + return env.value + return None + def count_pods_with_rolling_update_flag(self, labels, namespace='default'): pods = self.api.core_v1.list_namespaced_pod(namespace, label_selector=labels).items return len(list(filter(lambda x: "zalando-postgres-operator-rolling-update-required" in x.metadata.annotations, pods))) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index f473b5cc4..2386b3a64 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -2547,6 +2547,69 @@ def assert_distributed_pods(self, target_nodes, cluster_labels='cluster-name=aci return True + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + def test_user_env_var_override(self): + ''' + Test that user-provided environment variables from spec.env can override + operator-generated ones (like SPILO_CONFIGURATION). This is useful for + customizing Patroni DCS configuration such as ignore_slots. + ''' + k8s = self.k8s + cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster' + + # Custom SPILO_CONFIGURATION with ignore_slots for Patroni + custom_spilo_config = '{"bootstrap":{"dcs":{"ignore_slots":{"type":"logical"}}}}' + + try: + # Patch the cluster to add custom env var that should override operator defaults + pg_patch_env = { + "spec": { + "env": [ + { + "name": "SPILO_CONFIGURATION", + "value": custom_spilo_config + } + ] + } + } + + k8s.api.custom_objects_api.patch_namespaced_custom_object( + "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_env) + + # Wait for operator to process the change + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, + "Operator does not get in sync after env var patch") + + # Wait for pods to be updated with the new env var + k8s.wait_for_pod_start(cluster_label) + k8s.wait_for_running_pods(cluster_label, 2) + + # Verify that SPILO_CONFIGURATION env var exists in pods + self.eventuallyEqual(lambda: k8s.count_pods_with_env_variable("SPILO_CONFIGURATION", cluster_label), 2, + "SPILO_CONFIGURATION env variable not found in pods") + + # Verify that the user-provided value overrides the operator-generated one + actual_value = k8s.get_env_variable_value("SPILO_CONFIGURATION", cluster_label) + self.assertIsNotNone(actual_value, "SPILO_CONFIGURATION value is None") + self.assertIn("ignore_slots", actual_value, + "User-provided SPILO_CONFIGURATION with ignore_slots was not applied") + + # Clean up: remove the custom env var + pg_patch_remove_env = { + "spec": { + "env": None + } + } + k8s.api.custom_objects_api.patch_namespaced_custom_object( + "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_remove_env) + + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, + "Operator does not get in sync after removing env var") + + except timeout_decorator.TimeoutError: + print('Operator log: {}'.format(k8s.get_operator_log())) + raise + def check_cluster_child_resources_owner_references(self, cluster_name, cluster_namespace='default', inverse=False): k8s = self.k8s From bbe1acd94b9c064a0be78f501548299b10cb6e37 Mon Sep 17 00:00:00 2001 From: Pavel Zaytsev Date: Thu, 18 Dec 2025 15:47:56 -0800 Subject: [PATCH 3/4] Fix unit tests --- pkg/cluster/k8sres.go | 61 +++++++++++++++++++------------------- pkg/cluster/k8sres_test.go | 34 +++++++++++++-------- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 06c50ac6e..bb6c7b3e1 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1021,36 +1021,7 @@ func (c *Cluster) generateSpiloPodEnvVars( envVars = append(envVars, v1.EnvVar{Name: "KUBERNETES_USE_CONFIGMAPS", Value: "true"}) } - // fetch cluster-specific variables that will override all subsequent global variables - if len(spec.Env) > 0 { - envVars = appendEnvVars(envVars, spec.Env...) - } - - if spec.Clone != nil && spec.Clone.ClusterName != "" { - envVars = append(envVars, c.generateCloneEnvironment(spec.Clone)...) - } - - if spec.StandbyCluster != nil { - envVars = append(envVars, c.generateStandbyEnvironment(spec.StandbyCluster)...) - } - - // fetch variables from custom environment Secret - // that will override all subsequent global variables - secretEnvVarsList, err := c.getPodEnvironmentSecretVariables() - if err != nil { - return nil, err - } - envVars = appendEnvVars(envVars, secretEnvVarsList...) - - // fetch variables from custom environment ConfigMap - // that will override all subsequent global variables - configMapEnvVarsList, err := c.getPodEnvironmentConfigMapVariables() - if err != nil { - return nil, err - } - envVars = appendEnvVars(envVars, configMapEnvVarsList...) - - // global variables derived from operator configuration + // global variables derived from operator configuration (lowest priority - can be overridden) opConfigEnvVars := make([]v1.EnvVar, 0) if c.OpConfig.WALES3Bucket != "" { opConfigEnvVars = append(opConfigEnvVars, v1.EnvVar{Name: "WAL_S3_BUCKET", Value: c.OpConfig.WALES3Bucket}) @@ -1082,6 +1053,36 @@ func (c *Cluster) generateSpiloPodEnvVars( envVars = appendEnvVars(envVars, opConfigEnvVars...) + // fetch variables from custom environment ConfigMap + // these will override operator configuration defaults + configMapEnvVarsList, err := c.getPodEnvironmentConfigMapVariables() + if err != nil { + return nil, err + } + envVars = appendEnvVars(envVars, configMapEnvVarsList...) + + // fetch variables from custom environment Secret + // these will override configmap and operator configuration + secretEnvVarsList, err := c.getPodEnvironmentSecretVariables() + if err != nil { + return nil, err + } + envVars = appendEnvVars(envVars, secretEnvVarsList...) + + // Clone and Standby environments override configmap/secret for their specific variables + if spec.Clone != nil && spec.Clone.ClusterName != "" { + envVars = appendEnvVars(envVars, c.generateCloneEnvironment(spec.Clone)...) + } + + if spec.StandbyCluster != nil { + envVars = appendEnvVars(envVars, c.generateStandbyEnvironment(spec.StandbyCluster)...) + } + + // fetch cluster-specific variables from spec.env (highest priority - overrides everything) + if len(spec.Env) > 0 { + envVars = appendEnvVars(envVars, spec.Env...) + } + return envVars, nil } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 6bd87366d..9fced91d4 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "sort" + "strings" "testing" "time" @@ -567,13 +568,6 @@ func testEnvs(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresRole) func TestGenerateSpiloPodEnvVars(t *testing.T) { var dummyUUID = "efd12e58-5786-11e8-b5a7-06148230260c" - expectedClusterNameLabel := []ExpectedValue{ - { - envIndex: 5, - envVarConstant: "KUBERNETES_SCOPE_LABEL", - envVarValue: "cluster-name", - }, - } expectedSpiloWalPathCompat := []ExpectedValue{ { envIndex: 12, @@ -784,7 +778,7 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) { expectedValues: expectedValuesGCPCreds, }, { - subTest: "will not override global config KUBERNETES_SCOPE_LABEL parameter", + subTest: "will override global config KUBERNETES_SCOPE_LABEL parameter with user value", opConfig: config.Config{ Resources: config.Resources{ ClusterNameLabel: "cluster-name", @@ -795,7 +789,13 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) { }, cloneDescription: &acidv1.CloneDescription{}, standbyDescription: &acidv1.StandbyDescription{}, - expectedValues: expectedClusterNameLabel, + expectedValues: []ExpectedValue{ + { + envIndex: 5, + envVarConstant: "KUBERNETES_SCOPE_LABEL", + envVarValue: "my-scope-label", + }, + }, pgsql: acidv1.Postgresql{ Spec: acidv1.PostgresSpec{ Env: []v1.EnvVar{ @@ -1010,11 +1010,19 @@ func TestGenerateSpiloPodEnvVars(t *testing.T) { assert.NoError(t, err) for _, ev := range tt.expectedValues { - env := actualEnvs[ev.envIndex] + // Find the environment variable by name instead of using index + var env *v1.EnvVar + for i := range actualEnvs { + if strings.EqualFold(actualEnvs[i].Name, ev.envVarConstant) { + env = &actualEnvs[i] + break + } + } - if env.Name != ev.envVarConstant { - t.Errorf("%s %s: expected env name %s, have %s instead", - t.Name(), tt.subTest, ev.envVarConstant, env.Name) + if env == nil { + t.Errorf("%s %s: expected env variable %s not found", + t.Name(), tt.subTest, ev.envVarConstant) + continue } if ev.envVarValueRef != nil { From f228027182065a6d7c9cfa1b9df74d0bae9d3bc2 Mon Sep 17 00:00:00 2001 From: Pavel Zaytsev Date: Tue, 23 Dec 2025 14:09:39 -0800 Subject: [PATCH 4/4] Cluster fixes --- pkg/cluster/cluster.go | 4 ++-- pkg/cluster/cluster_test.go | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index b6a4e24a8..225564733 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -716,12 +716,12 @@ func compareSpiloConfiguration(configa, configb string) bool { if err != nil { return false } - oa.Bootstrap.DCS = patroniDCS{} err = json.Unmarshal([]byte(configb), &ob) if err != nil { return false } - ob.Bootstrap.DCS = patroniDCS{} + // Compare the full configuration including DCS section + // This allows user-provided DCS settings (like ignore_slots) to trigger pod restarts return reflect.DeepEqual(oa, ob) } diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index d78d4c92e..ee20d0434 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1202,8 +1202,9 @@ func TestCompareSpiloConfiguration(t *testing.T) { true, }, { + // This config has different max_connections (200 vs 100) in DCS, so it should not be equal `{"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"}],"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"postgresql":{"parameters":{"max_connections":"200","max_locks_per_transaction":"64","max_worker_processes":"4"}}}}}`, - true, + false, }, { `{}`, @@ -1249,6 +1250,7 @@ func TestCompareEnv(t *testing.T) { ExpectedResult: true, }, { + // spiloConfigDiff has different DCS settings, so it should not be equal Envs: []v1.EnvVar{ { Name: "VARIABLE1", @@ -1267,7 +1269,7 @@ func TestCompareEnv(t *testing.T) { Value: spiloConfigDiff, }, }, - ExpectedResult: true, + ExpectedResult: false, }, { Envs: []v1.EnvVar{