Connection pooler for replica (zalando#1127)

* Enable connection pooler for replica
* Refactor code for connection pooler
  - Move all the relevant code to a separate file
  - Move all the related tests to a separate file
  - Avoid using cluster where not required
  - Simplify the logic in sync and other methods
  - Cleanup of duplicated or unused code
* Fix labels for the replica pods
* Update deleteConnectionPooler to include role
* Adding test cases and other changes
   - Fix unit test and delete secret when required only
   - Make sure we use empty fresh cluster for every test case.
* enhance e2e test
* Disable pooler in complete manifest as this is source for e2e too an creates unnecessary pooler setups.

Co-authored-by: Rafia Sabih <[email protected]>
Co-authored-by: Jan Mussler <[email protected]>

Author: 3 people

charts/postgres-operator/crds/postgresqls.yaml

@@ -190,6 +190,8 @@ spec:
                 type: string
               enableConnectionPooler:
                 type: boolean
+              enableReplicaConnectionPooler:
+                type: boolean
               enableLogicalBackup:
                 type: boolean
               enableMasterLoadBalancer:
@@ -603,4 +605,4 @@ spec:
           status:
             type: object
             additionalProperties:
-              type: string
+              type: string

docs/reference/cluster_manifest.md

@@ -151,10 +151,15 @@ These parameters are grouped directly under  the `spec` key in the manifest.
   configured (so you can override the operator configuration). Optional.
 
 * **enableConnectionPooler**
-  Tells the operator to create a connection pooler with a database. If this
-  field is true, a connection pooler deployment will be created even if
+  Tells the operator to create a connection pooler with a database for the master
+  service. If this field is true, a connection pooler deployment will be created even if
   `connectionPooler` section is empty. Optional, not set by default.
 
+* **enableReplicaConnectionPooler**
+  Tells the operator to create a connection pooler with a database for the replica
+  service. If this field is true, a connection pooler deployment for replica
+  will be created even if `connectionPooler` section is empty. Optional, not set by default.
+
 * **enableLogicalBackup**
   Determines if the logical backup of this cluster should be taken and uploaded
   to S3. Default: false. Optional.
@@ -241,10 +246,10 @@ explanation of `ttl` and `loop_wait` parameters.
 
 * **synchronous_mode**
   Patroni `synchronous_mode` parameter value. The default is set to `false`. Optional.
-  
+
 * **synchronous_mode_strict**
   Patroni `synchronous_mode_strict` parameter value. Can be used in addition to `synchronous_mode`. The default is set to `false`. Optional.
-  
+
 ## Postgres container resources
 
 Those parameters define [CPU and memory requests and limits](https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/)
@@ -397,8 +402,10 @@ CPU and memory limits for the sidecar container.
 
 Parameters are grouped under the `connectionPooler` top-level key and specify
 configuration for connection pooler. If this section is not empty, a connection
-pooler will be created for a database even if `enableConnectionPooler` is not
-present.
+pooler will be created for master service only even if `enableConnectionPooler`
+is not present. But if this section is present then it defines the configuration
+for both master and replica pooler services (if `enableReplicaConnectionPooler`
+ is enabled).
 
 * **numberOfInstances**
   How many instances of connection pooler to create.

docs/user.md

@@ -807,11 +807,17 @@ manifest:
 ```yaml
 spec:
   enableConnectionPooler: true
+  enableReplicaConnectionPooler: true
 ```
 
 This will tell the operator to create a connection pooler with default
 configuration, through which one can access the master via a separate service
-`{cluster-name}-pooler`. In most of the cases the
+`{cluster-name}-pooler`. With the first option, connection pooler for master service
+is created and with the second option, connection pooler for replica is created.
+Note that both of these flags are independent of each other and user can set or
+unset any of them as per their requirements without any effect on the other.
+
+In most of the cases the
 [default configuration](reference/operator_parameters.md#connection-pooler-configuration)
 should be good enough. To configure a new connection pooler individually for
 each Postgres cluster, specify:

e2e/scripts/watch_objects.sh

@@ -8,7 +8,9 @@ kubectl get statefulset -o jsonpath='{.items..metadata.annotations.zalando-postg
 echo
 echo
 echo 'Pods'
-kubectl get pods -l application=spilo -l name=postgres-operator -l application=db-connection-pooler -o wide --all-namespaces
+kubectl get pods -l application=spilo -o wide --all-namespaces
+echo
+kubectl get pods -l application=db-connection-pooler -o wide --all-namespaces
 echo
 echo 'Statefulsets'
 kubectl get statefulsets --all-namespaces

e2e/tests/k8s_api.py

@@ -1,19 +1,14 @@
 import json
-import unittest
 import time
-import timeout_decorator
 import subprocess
 import warnings
-import os
-import yaml
 
-from datetime import datetime
 from kubernetes import client, config
 from kubernetes.client.rest import ApiException
 
 
 def to_selector(labels):
-    return ",".join(["=".join(l) for l in labels.items()])
+    return ",".join(["=".join(lbl) for lbl in labels.items()])
 
 
 class K8sApi:
@@ -43,8 +38,8 @@ class K8s:
 
     def __init__(self, labels='x=y', namespace='default'):
         self.api = K8sApi()
-        self.labels=labels
-        self.namespace=namespace
+        self.labels = labels
+        self.namespace = namespace
 
     def get_pg_nodes(self, pg_cluster_name, namespace='default'):
         master_pod_node = ''
@@ -81,7 +76,7 @@ def get_operator_pod(self):
             'default', label_selector='name=postgres-operator'
         ).items
 
-        pods = list(filter(lambda x: x.status.phase=='Running', pods))
+        pods = list(filter(lambda x: x.status.phase == 'Running', pods))
 
         if len(pods):
             return pods[0]
@@ -110,7 +105,6 @@ def wait_for_pod_start(self, pod_labels, namespace='default'):
 
             time.sleep(self.RETRY_TIMEOUT_SEC)
 
-
     def get_service_type(self, svc_labels, namespace='default'):
         svc_type = ''
         svcs = self.api.core_v1.list_namespaced_service(namespace, label_selector=svc_labels, limit=1).items
@@ -213,8 +207,8 @@ def wait_for_logical_backup_job_creation(self):
         self.wait_for_logical_backup_job(expected_num_of_jobs=1)
 
     def delete_operator_pod(self, step="Delete operator pod"):
-        # patching the pod template in the deployment restarts the operator pod
-        self.api.apps_v1.patch_namespaced_deployment("postgres-operator","default", {"spec":{"template":{"metadata":{"annotations":{"step":"{}-{}".format(step, datetime.fromtimestamp(time.time()))}}}}})
+             # patching the pod template in the deployment restarts the operator pod
+        self.api.apps_v1.patch_namespaced_deployment("postgres-operator", "default", {"spec": {"template": {"metadata": {"annotations": {"step": "{}-{}".format(step, time.time())}}}}})
         self.wait_for_operator_pod_start()
 
     def update_config(self, config_map_patch, step="Updating operator deployment"):
@@ -237,7 +231,7 @@ def exec_with_kubectl(self, pod, cmd):
 
     def get_patroni_state(self, pod):
         r = self.exec_with_kubectl(pod, "patronictl list -f json")
-        if not r.returncode == 0 or not r.stdout.decode()[0:1]=="[":
+        if not r.returncode == 0 or not r.stdout.decode()[0:1] == "[":
             return []
         return json.loads(r.stdout.decode())
 
@@ -248,7 +242,7 @@ def get_operator_state(self):
         pod = pod.metadata.name
 
         r = self.exec_with_kubectl(pod, "curl localhost:8080/workers/all/status/")
-        if not r.returncode == 0 or not r.stdout.decode()[0:1]=="{":
+        if not r.returncode == 0 or not r.stdout.decode()[0:1] == "{":
             return None
 
         return json.loads(r.stdout.decode())
@@ -277,7 +271,7 @@ def get_effective_pod_image(self, pod_name, namespace='default'):
         '''
         pod = self.api.core_v1.list_namespaced_pod(
             namespace, label_selector="statefulset.kubernetes.io/pod-name=" + pod_name)
-        
+
         if len(pod.items) == 0:
             return None
         return pod.items[0].spec.containers[0].image
@@ -305,8 +299,8 @@ class K8sBase:
 
     def __init__(self, labels='x=y', namespace='default'):
         self.api = K8sApi()
-        self.labels=labels
-        self.namespace=namespace
+        self.labels = labels
+        self.namespace = namespace
 
     def get_pg_nodes(self, pg_cluster_labels='cluster-name=acid-minimal-cluster', namespace='default'):
         master_pod_node = ''
@@ -434,10 +428,10 @@ def count_deployments_with_label(self, labels, namespace='default'):
     def count_pdbs_with_label(self, labels, namespace='default'):
         return len(self.api.policy_v1_beta1.list_namespaced_pod_disruption_budget(
             namespace, label_selector=labels).items)
-  
+
     def count_running_pods(self, labels='application=spilo,cluster-name=acid-minimal-cluster', namespace='default'):
         pods = self.api.core_v1.list_namespaced_pod(namespace, label_selector=labels).items
-        return len(list(filter(lambda x: x.status.phase=='Running', pods)))
+        return len(list(filter(lambda x: x.status.phase == 'Running', pods)))
 
     def wait_for_pod_failover(self, failover_targets, labels, namespace='default'):
         pod_phase = 'Failing over'
@@ -484,14 +478,14 @@ def exec_with_kubectl(self, pod, cmd):
 
     def get_patroni_state(self, pod):
         r = self.exec_with_kubectl(pod, "patronictl list -f json")
-        if not r.returncode == 0 or not r.stdout.decode()[0:1]=="[":
+        if not r.returncode == 0 or not r.stdout.decode()[0:1] == "[":
             return []
         return json.loads(r.stdout.decode())
 
     def get_patroni_running_members(self, pod):
         result = self.get_patroni_state(pod)
-        return list(filter(lambda x: x["State"]=="running", result))
-    
+        return list(filter(lambda x: x["State"] == "running", result))
+
     def get_statefulset_image(self, label_selector="application=spilo,cluster-name=acid-minimal-cluster", namespace='default'):
         ssets = self.api.apps_v1.list_namespaced_stateful_set(namespace, label_selector=label_selector, limit=1)
         if len(ssets.items) == 0:
@@ -505,7 +499,7 @@ def get_effective_pod_image(self, pod_name, namespace='default'):
         '''
         pod = self.api.core_v1.list_namespaced_pod(
             namespace, label_selector="statefulset.kubernetes.io/pod-name=" + pod_name)
-        
+
         if len(pod.items) == 0:
             return None
         return pod.items[0].spec.containers[0].image
@@ -514,10 +508,13 @@ def get_effective_pod_image(self, pod_name, namespace='default'):
 """
   Inspiriational classes towards easier writing of end to end tests with one cluster per test case
 """
+
+
 class K8sOperator(K8sBase):
     def __init__(self, labels="name=postgres-operator", namespace="default"):
         super().__init__(labels, namespace)
 
+
 class K8sPostgres(K8sBase):
     def __init__(self, labels="cluster-name=acid-minimal-cluster", namespace="default"):
         super().__init__(labels, namespace)