Release 1.4.2 (#619)

* Bump version to 1.4.2
* Update release notes

README.rst

@@ -12,7 +12,7 @@ Patroni is a template for you to create your own customized, high-availability s
 
 We call Patroni a "template" because it is far from being a one-size-fits-all or plug-and-play replication system. It will have its own caveats. Use wisely.
 
-**Note to Kubernetes users**: Patroni can run natively on top of Kubernetes. Take a look at the `Kubernetes <https://github.com/zalando/patroni/blog/master/docs/kubernetes.rst>`__ chapter of the Patroni documentation.
+**Note to Kubernetes users**: Patroni can run natively on top of Kubernetes. Take a look at the `Kubernetes <https://github.com/zalando/patroni/blob/master/docs/kubernetes.rst>`__ chapter of the Patroni documentation.
 
 .. contents::
     :local:

docs/ENVIRONMENT.rst

@@ -55,7 +55,7 @@ Exhibitor
 
 Kubernetes
 ----------
--  **PATRONI\_KUBERNETES\_NAMESPACE**: (optional) Kubernetes namespace where the operator is running. Default value is `default`.
+-  **PATRONI\_KUBERNETES\_NAMESPACE**: (optional) Kubernetes namespace where the Patroni pod is running. Default value is `default`.
 -  **PATRONI\_KUBERNETES\_LABELS**: Labels in format ``{label1: value1, label2: value2}``. These labels will be used to find existing objects (Pods and either Endpoints or ConfigMaps) associated with the current cluster. Also Patroni will set them on every object (Endpoint or ConfigMap) it creates.
 -  **PATRONI\_KUBERNETES\_SCOPE\_LABEL**: (optional) name of the label containing cluster name. Default value is `cluster-name`.
 -  **PATRONI\_KUBERNETES\_ROLE\_LABEL**: (optional) name of the label containing Postgres role (`master` or `replica`). Patroni will set this label on the pod it is running in. Default value is `role`.

docs/SETTINGS.rst

@@ -87,7 +87,7 @@ Exhibitor
 
 Kubernetes
 ----------
--  **namespace**: (optional) Kubernetes namespace where we are running. Default value is `default`.
+-  **namespace**: (optional) Kubernetes namespace where Patroni pod is running. Default value is `default`.
 -  **labels**: Labels in format ``{label1: value1, label2: value2}``. These labels will be used to find existing objects (Pods and either Endpoints or ConfigMaps) associated with the current cluster. Also Patroni will set them on every object (Endpoint or ConfigMap) it creates.
 -  **scope\_label**: (optional) name of the label containing cluster name. Default value is `cluster-name`.
 -  **role\_label**: (optional) name of the label containing role (master or replica). Patroni will set this label on the pod it runs in. Default value is ``role``.

docs/releases.rst

@@ -3,6 +3,63 @@
 Release notes
 =============
 
+Version 1.4.2
+-------------
+
+**Improvements in patronictl**
+
+- Rename scheduled failover to scheduled switchover (Alexander Kukushkin)
+
+  Failover and switchover functions were separated in version 1.4, but `patronictl list` was still reporting `Scheduled failover` instead of `Scheduled switchover`.
+
+- Show information about pending restarts (Alexander)
+
+  In order to apply some configuration changes sometimes it is necessary to restart postgres. Patroni was already giving a hint about that in the REST API and when writing node status into DCS, but there were no easy way to display it.
+
+- Make show-config to work with cluster_name from config file (Alexander)
+
+  It works similar to the `patronictl edit-config`
+
+**Stability improvements**
+
+- Avoid calling pg_controldata during bootstrap (Alexander)
+
+  During initdb or custom bootstrap there is a time window when pgdata is not empty but pg_controldata has not been written yet. In such case pg_controldata call was failing with error messages.
+
+- Handle exceptions raised from psutil (Alexander)
+
+  cmdline is read and parsed every time when `cmdline()` method is called. It could happen that the process being examined
+  has already disappeared, in that case `NoSuchProcess` is raised.
+
+**Kubernetes support improvements**
+
+- Don't swallow errors from k8s API (Alexander)
+
+  A call to Kubernetes API could fail for a different number of reasons. In some cases such call should be retried, in some other cases we should log the error message and the exception stack trace. The change here will help debug Kubernetes permission issues.
+
+- Update Kubernetes example Dockerfile to install Patroni from the master branch (Maciej Szulik)
+
+  Before that it was using `feature/k8s`, which became outdated.
+
+- Add proper RBAC to run patroni on k8s (Maciej)
+
+  Add the Service account that is assigned to the pods of the cluster, the role that holds only the necessary permissions, and the rolebinding that connects the Service account and the Role.
+
+
+Version 1.4.1
+-------------
+
+**Fixes in patronictl**
+
+- Don't show current leader in suggested list of members to failover to. (Alexander Kukushkin)
+
+  patronictl failover could still work when there is leader in the cluster and it should be excluded from the list of member where it is possible to failover to.
+
+- Make patronictl switchover compatible with the old Patroni api (Alexander)
+
+  In case if POST /switchover REST API call has failed with status code 501 it will do it once again, but to /failover endpoint.
+
+
 Version 1.4
 -----------
 

extras/confd/templates/haproxy.tmpl

@@ -10,19 +10,23 @@ defaults
 	timeout server 30m
 	timeout check 5s
 
-frontend master_postgresql
+listen stats
+	mode http
+	bind *:7000
+	stats enable
+	stats uri /
+
+listen master
 	bind *:5000
-	default_backend backend_master
-
-frontend replicas_postgresql
-	bind *:5001
-	default_backend backend_replicas
-
-backend backend_master
 	option httpchk OPTIONS /master
+	http-check expect status 200
+	default-server inter 3s fall 3 rise 2 on-marked-down shutdown-sessions
 {{range gets "/members/*"}}	server {{base .Key}} {{$data := json .Value}}{{base (replace (index (split $data.conn_url "/") 2) "@" "/" -1)}} maxconn 100 check port {{index (split (index (split $data.api_url "/") 2) ":") 1}}
 {{end}}
-backend backend_replicas
+listen replicas
+	bind *:5001
 	option httpchk OPTIONS /replica
+	http-check expect status 200
+	default-server inter 3s fall 3 rise 2 on-marked-down shutdown-sessions
 {{range gets "/members/*"}}	server {{base .Key}} {{$data := json .Value}}{{base (replace (index (split $data.conn_url "/") 2) "@" "/" -1)}} maxconn 100 check port {{index (split (index (split $data.api_url "/") 2) ":") 1}}
 {{end}}

kubernetes/Dockerfile

@@ -12,7 +12,7 @@ RUN export DEBIAN_FRONTEND=noninteractive \
     && localedef -i en_US -c -f UTF-8 -A /usr/share/locale/locale.alias en_US.UTF-8 \
 
     && pip install setuptools pip --upgrade \
-    && pip install 'git+https://github.com/zalando/patroni.git@feature/k8s#egg=patroni[kubernetes]' \
+    && pip install 'git+https://github.com/zalando/patroni.git#egg=patroni[kubernetes]' \
 
     && mkdir -p /home/postgres \
     && chown postgres:postgres /home/postgres \

kubernetes/patroni_k8s.yaml

@@ -14,6 +14,7 @@ spec:
         application: patroni
         cluster-name: *cluster_name
     spec:
+      serviceAccountName: patronidemo
       containers:
       - name: *cluster_name
         image: patroni  # docker build -t patroni .
@@ -120,3 +121,62 @@ type: Opaque
 data:
   superuser-password: emFsYW5kbw==
   replication-password: cmVwLXBhc3M=
+
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: patronidemo
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: patronidemo
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - create
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - endpoints
+  verbs:
+  - get
+  - patch
+  - update
+  # the following three privileges are necessary only when using endpoints
+  - create
+  - list
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - get
+  - list
+  - patch
+  - update
+  - watch
+
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: patronidemo
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: patronidemo
+subjects:
+- kind: ServiceAccount
+  name: patronidemo

patroni/ctl.py

@@ -546,15 +546,14 @@ def _do_failover_or_switchover(obj, action, cluster_name, master, candidate, for
     dcs = get_dcs(obj, cluster_name)
     cluster = dcs.get_cluster()
 
-    if action == 'switchover':
-        if cluster.leader is None:
-            raise PatroniCtlException('This cluster has no master')
+    if action == 'switchover' and cluster.leader is None:
+        raise PatroniCtlException('This cluster has no master')
 
-        if master is None:
-            if force:
-                master = cluster.leader.member.name
-            else:
-                master = click.prompt('Master', type=str, default=cluster.leader.member.name)
+    if master is None:
+        if force or action == 'failover':
+            master = cluster.leader and cluster.leader.name
+        else:
+            master = click.prompt('Master', type=str, default=cluster.leader.member.name)
 
     if master is not None and cluster.leader and cluster.leader.member.name != master:
         raise PatroniCtlException('Member {0} is not the leader of cluster {1}'.format(master, cluster_name))
@@ -610,6 +609,11 @@ def _do_failover_or_switchover(obj, action, cluster_name, master, candidate, for
         member = cluster.leader.member if cluster.leader else cluster.get_member(candidate, False)
 
         r = request_patroni(member, 'post', action, failover_value, auth_header(obj))
+
+        # probably old patroni, which doesn't support switchover yet
+        if r.status_code == 501 and action == 'switchover' and 'Server does not support this operation' in r.text:
+            r = request_patroni(member, 'post', 'failover', failover_value, auth_header(obj))
+
         if r.status_code in (200, 202):
             logging.debug(r)
             cluster = dcs.get_cluster()
@@ -662,7 +666,8 @@ def output_members(cluster, name, extended=False, fmt='pretty'):
     # Mainly for consistent pretty printing and watching we sort the output
     cluster.members.sort(key=lambda x: x.name)
 
-    extended = extended or any(m.data.get('scheduled_restart') for m in cluster.members)
+    has_scheduled_restarts = any(m.data.get('scheduled_restart') for m in cluster.members)
+    has_pending_restarts = any(m.data.get('pending_restart') for m in cluster.members)
 
     for m in cluster.members:
         logging.debug(m)
@@ -682,7 +687,10 @@ def output_members(cluster, name, extended=False, fmt='pretty'):
 
         row = [name, m.name, m.conn_kwargs()['host'], role, m.data.get('state', ''), lag]
 
-        if extended:
+        if extended or has_pending_restarts:
+            row.append('*' if m.data.get('pending_restart') else '')
+
+        if extended or has_scheduled_restarts:
             value = ''
             scheduled_restart = m.data.get('scheduled_restart')
             if scheduled_restart:
@@ -695,11 +703,13 @@ def output_members(cluster, name, extended=False, fmt='pretty'):
         rows.append(row)
 
     columns = ['Cluster', 'Member', 'Host', 'Role', 'State', 'Lag in MB']
-    alignment = {'Cluster': 'l', 'Member': 'l', 'Host': 'l', 'Lag in MB': 'r'}
+    alignment = {'Lag in MB': 'r'}
 
-    if extended:
+    if extended or has_pending_restarts:
+        columns.append('Pending restart')
+
+    if extended or has_scheduled_restarts:
         columns.append('Scheduled restart')
-        alignment['Scheduled restart'] = 'l'
 
     print_output(columns, rows, alignment, fmt)
 
@@ -708,11 +718,11 @@ def output_members(cluster, name, extended=False, fmt='pretty'):
         service_info.append('Maintenance mode: on')
 
     if cluster.failover and cluster.failover.scheduled_at:
-        info = 'Failover scheduled at: ' + cluster.failover.scheduled_at.isoformat()
+        info = 'Switchover scheduled at: ' + cluster.failover.scheduled_at.isoformat()
         if cluster.failover.leader:
-            info += '\n                  from: ' + cluster.failover.leader
+            info += '\n                    from: ' + cluster.failover.leader
         if cluster.failover.candidate:
-            info += '\n                    to: ' + cluster.failover.candidate
+            info += '\n                      to: ' + cluster.failover.candidate
         service_info.append(info)
 
     if service_info:
@@ -1091,7 +1101,7 @@ def edit_config(obj, cluster_name, force, quiet, kvpairs, pgkvpairs, apply_filen
 
 
 @ctl.command('show-config', help="Show cluster configuration")
-@click.argument('cluster_name')
+@arg_cluster_name
 @click.pass_obj
 def show_config(obj, cluster_name):
     cluster = get_dcs(obj, cluster_name).get_cluster()

patroni/dcs/kubernetes.py

@@ -29,7 +29,7 @@ class KubernetesRetriableException(k8s_client.rest.ApiException):
         self.headers = orig.headers
 
 
-class CoreV1Api(object):
+class CoreV1ApiProxy(object):
 
     def __init__(self, use_endpoints=False):
         self._api = k8s_client.CoreV1Api()
@@ -55,6 +55,21 @@ class CoreV1Api(object):
         return wrapper
 
 
+def catch_kubernetes_errors(func):
+    def wrapper(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except k8s_client.rest.ApiException as e:
+            if e.status == 403:
+                logger.exception('Permission denied')
+            elif e.status != 409:  # Object exists or conflict in resource_version
+                logger.exception('Unexpected error from Kubernetes API')
+            return False
+        except (RetryFailedError, HTTPException, HTTPError, socket.error, socket.timeout):
+            return False
+    return wrapper
+
+
 class Kubernetes(AbstractDCS):
 
     def __init__(self, config):
@@ -84,7 +99,7 @@ class Kubernetes(AbstractDCS):
                 port.update({n: p[n] for n in ('name', 'protocol') if p.get(n)})
                 ports.append(k8s_client.V1EndpointPort(**port))
             self.__subsets = [k8s_client.V1EndpointSubset(addresses=addresses, ports=ports)]
-        self._api = CoreV1Api(use_endpoints)
+        self._api = CoreV1ApiProxy(use_endpoints)
         self.set_retry_timeout(config['retry_timeout'])
         self.set_ttl(config.get('ttl') or 30)
         self._leader_observed_record = {}
@@ -96,16 +111,6 @@ class Kubernetes(AbstractDCS):
     def retry(self, *args, **kwargs):
         return self._retry.copy()(*args, **kwargs)
 
-    def catch_kubernetes_errors(func):
-        @functools.wraps(func)
-        def wrapper(*args, **kwargs):
-            try:
-                return func(*args, **kwargs)
-            except (RetryFailedError, k8s_client.rest.ApiException,
-                    HTTPException, HTTPError, socket.error, socket.timeout):
-                return False
-        return wrapper
-
     def client_path(self, path):
         return super(Kubernetes, self).client_path(path)[1:].replace('/', '-')
 
@@ -362,7 +367,6 @@ class Kubernetes(AbstractDCS):
     def delete_cluster(self):
         self.retry(self._api.delete_collection_namespaced_kind, self._namespace, label_selector=self._label_selector)
 
-    @catch_kubernetes_errors
     def set_history_value(self, value):
         patch = bool(self.cluster and self.cluster.config and self.cluster.config.index)
         return self.patch_or_create(self.config_path, {self._HISTORY: value}, None, patch, False)

patroni/postgresql.py

@@ -547,6 +547,7 @@ class Postgresql(object):
         return ret
 
     def _custom_bootstrap(self, config):
+        self.set_state('running custom bootstrap script')
         params = ['--scope=' + self.scope, '--datadir=' + self._data_dir]
         try:
             logger.info('Running custom bootstrap script: %s', config['command'])
@@ -1161,7 +1162,7 @@ class Postgresql(object):
         """ return the contents of pg_controldata, or non-True value if pg_controldata call failed """
         result = {}
         # Don't try to call pg_controldata during backup restore
-        if self._version_file_exists() and self.state != 'creating replica':
+        if not self.bootstrapping and self._version_file_exists() and self.state != 'creating replica':
             try:
                 data = subprocess.check_output([self._pgcommand('pg_controldata'), self._data_dir],
                                                env={'LANG': 'C', 'LC_ALL': 'C', 'PATH': os.environ['PATH']})

patroni/postmaster.py

@@ -82,11 +82,23 @@ class PostmasterProcess(psutil.Process):
                                  "autovacuum launcher|autovacuum worker|stats collector|wal receiver|archiver|"
                                  "wal sender) process|bgworker: )")
 
-        user_backends = [p for p in self.children() if not aux_proc_re.match(p.cmdline()[0])]
-        logger.debug("Waiting for user backends {0} to close".format(
-            ",".join(p.cmdline()[0] for p in user_backends)))
-        psutil.wait_procs(user_backends)
-        logger.debug("Backends closed")
+        try:
+            user_backends = []
+            user_backends_cmdlines = []
+            for child in self.children():
+                try:
+                    cmdline = child.cmdline()[0]
+                    if not aux_proc_re.match(cmdline):
+                        user_backends.append(child)
+                        user_backends_cmdlines.append(cmdline)
+                except psutil.NoSuchProcess:
+                    pass
+            if user_backends:
+                logger.debug('Waiting for user backends %s to close', ', '.join(user_backends_cmdlines))
+                psutil.wait_procs(user_backends)
+            logger.debug("Backends closed")
+        except psutil.Error:
+            logger.exception('wait_for_user_backends_to_close')
 
     @classmethod
     def start(cls, pgcommand, data_dir, conf, options):

patroni/version.py

@@ -1 +1 @@
-__version__ = '1.4'
+__version__ = '1.4.2'

tests/test_ctl.py

@@ -129,6 +129,11 @@ class TestCtl(unittest.TestCase):
             result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny')
             assert 'Switchover failed' in result.output
 
+            mocked.return_value.status_code = 501
+            mocked.return_value.text = 'Server does not support this operation'
+            result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny')
+            assert 'Switchover failed' in result.output
+
         # No members available
         mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_only_leader
         result = self.runner.invoke(ctl, ['switchover', 'dummy'], input='leader\nother\n\ny')

tests/test_kubernetes.py

@@ -1,7 +1,7 @@
 import unittest
 
 from mock import Mock, patch
-from patroni.dcs.kubernetes import Kubernetes, KubernetesError, k8s_client, k8s_watch
+from patroni.dcs.kubernetes import Kubernetes, KubernetesError, k8s_client, k8s_watch, RetryFailedError
 
 
 def mock_list_namespaced_config_map(self, *args, **kwargs):
@@ -57,7 +57,8 @@ class TestKubernetes(unittest.TestCase):
         self.k.take_leader()
 
     def test_manual_failover(self):
-        self.k.manual_failover('foo', 'bar')
+        with patch.object(k8s_client.CoreV1Api, 'patch_namespaced_config_map', Mock(side_effect=RetryFailedError(''))):
+            self.k.manual_failover('foo', 'bar')
 
     def test_set_config_value(self):
         self.k.set_config_value('{}')
@@ -79,7 +80,7 @@ class TestKubernetes(unittest.TestCase):
         self.k.cancel_initialization()
 
     @patch.object(k8s_client.CoreV1Api, 'delete_collection_namespaced_config_map',
-                  Mock(side_effect=k8s_client.rest.ApiException(500, '')))
+                  Mock(side_effect=k8s_client.rest.ApiException(403, '')))
     def test_delete_cluster(self):
         self.k.delete_cluster()
 

tests/test_postmaster.py

@@ -61,11 +61,18 @@ class TestPostmasterProcess(unittest.TestCase):
         c1.cmdline = Mock(return_value=["postgres: startup process"])
         c2 = Mock()
         c2.cmdline = Mock(return_value=["postgres: postgres postgres [local] idle"])
-        with patch('psutil.Process.children', Mock(return_value=[c1, c2])):
+        c3 = Mock()
+        c3.cmdline = Mock(side_effect=psutil.NoSuchProcess(123))
+        with patch('psutil.Process.children', Mock(return_value=[c1, c2, c3])):
             proc = PostmasterProcess(123)
             self.assertIsNone(proc.wait_for_user_backends_to_close())
             mock_wait.assert_called_with([c2])
 
+        c3.cmdline = Mock(side_effect=psutil.AccessDenied(123))
+        with patch('psutil.Process.children', Mock(return_value=[c3])):
+            proc = PostmasterProcess(123)
+            self.assertIsNone(proc.wait_for_user_backends_to_close())
+
     @patch('subprocess.Popen')
     @patch.object(PostmasterProcess, 'from_pid')
     def test_start(self, mock_frompid, mock_popen):