Allow user env vars to override operator-generated ones

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)))

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
 

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,13 +1053,56 @@ 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
 }
 
 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)
 		}
 	}

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 {