Release v3.1.0 (#2801)

- bump pyright and resolve reported issues
- bump Patroni version
- update release notes

.github/workflows/tests.yaml

@@ -173,4 +173,4 @@ jobs:
 
     - uses: jakebailey/pyright-action@v1
       with:
-        version: 1.1.317
+        version: 1.1.320

docs/kubernetes.rst

@@ -32,8 +32,11 @@ Configuration
 
 Patroni Kubernetes :ref:`settings <kubernetes_settings>` and :ref:`environment variables <kubernetes_environment>` are described in the general chapters of the documentation.
 
+.. _kubernetes_role_values:
+
 Customize role label
 ^^^^^^^^^^^^^^^^^^^^
+
 By default, Patroni will set corresponding labels on the pod it runs in based on node's role, such as ``role=master``.
 The key and value of label can be customized by `kubernetes.role_label`, `kubernetes.leader_label_value`, `kubernetes.follower_label_value` and `kubernetes.standby_leader_label_value`.
 

docs/releases.rst

@@ -3,6 +3,84 @@
 Release notes
 =============
 
+Version 3.1.0
+-------------
+
+**Breaking changes**
+
+- Changed semantic of ``restapi.keyfile`` and ``restapi.certfile`` (Alexander Kukushkin)
+
+  Previously Patroni was using ``restapi.keyfile`` and ``restapi.certfile`` as client certificates as a fallback if there were no respective configuration parameters in the ``ctl`` section.
+
+.. warning::
+    If you enabled client certificates validation (``restapi.verify_client`` is set to ``required``), you also **must** provide **valid client certificates** in the ``ctl.certfile``, ``ctl.keyfile``, ``ctl.keyfile_password``. If not provided, Patroni will not work correctly.
+
+
+**New features**
+
+- Make Pod role label configurable (Waynerv)
+
+  Values could be customized using ``kubernetes.leader_label_value``, ``kubernetes.follower_label_value`` and ``kubernetes.standby_leader_label_value`` parameters. This feature will be very useful when we change the ``master`` role to the ``primary``. You can read more about the feature and migration steps :ref:`here <kubernetes_role_values>`.
+
+
+**Improvements**
+
+- Various improvements of ``patroni --validate-config`` (Alexander Kukushkin)
+
+  Improved parameter validation for different DCS, ``bootstrap.dcs`` , ``ctl``, ``restapi``, and ``watchdog`` sections.
+
+- Start Postgres not in recovery if it crashed during recovery while Patroni is running (Alexander Kukushkin)
+
+  It may reduce recovery time and will help to prevent unnecessary timeline increments.
+
+- Avoid unnecessary updates of ``/status`` key (Alexander Kukushkin)
+
+  When there are no permanent logical slots Patroni was updating the ``/status`` on every heartbeat loop even when LSN on the primary didn't move forward.
+
+- Don't allow stale primary to win the leader race (Alexander Kukushkin)
+
+  If Patroni was hanging during a significant time due to lack of resources it will additionally check that no other nodes promoted Postgres before acquiring the leader lock.
+
+- Implemented visibility of certain PostgreSQL parameters validation (Alexander Kukushkin, Feike Steenbergen)
+
+  If validation of ``max_connections``, ``max_wal_senders``, ``max_prepared_transactions``, ``max_locks_per_transaction``, ``max_replication_slots``, or ``max_worker_processes`` failed Patroni was using some sane default value. Now in addition to that it will also show a warning.
+
+- Set permissions for files and directories created in ``PGDATA`` (Alexander Kukushkin)
+
+  All files created by Patroni had only owner read/write permissions. This behaviour was breaking backup tools that run under a different user and relying on group read permissions. Now Patroni honors permissions on ``PGDATA`` and correctly sets permissions on all directories and files it creates inside ``PGDATA``.
+
+
+**Bugfixes**
+
+- Run ``archive_command`` through shell (Waynerv)
+
+  Patroni might archive some WAL segments before doing crash recovery in a single-user mode or before ``pg_rewind``. If the archive_command contains some shell operators, like ``&&`` it didn't work with Patroni.
+
+- Fixed "on switchover" shutdown checks (Polina Bungina)
+
+  It was possible that specified candidate is still streaming and didn't received shut down checking but the leader key was removed because some other nodes were healthy.
+
+- Fixed "is primary" check (Alexander Kukushkin)
+
+  During the leader race replicas were not able to recognize that Postgres on the old leader is still running as a primary.
+
+- Fixed ``patronictl list`` (Alexander Kukushkin)
+
+  The Cluster name field was missing in ``tsv``, ``json``, and ``yaml`` output formats.
+
+- Fixed ``pg_rewind`` behaviour after pause (Alexander Kukushkin)
+
+  Under certain conditions, Patroni wasn't able to join the false primary back to the cluster with ``pg_rewind`` after coming out of maintenance mode.
+
+- Fixed bug in Etcd v3 implementation (Alexander Kukushkin)
+
+  Invalidate internal KV cache if key update performed using ``create_revision``/``mod_revision`` field due to revision mismatch.
+
+- Fixed behaviour of replicas in standby cluster in pause (Alexander Kukushkin)
+
+  When the leader key expires replicas in standby cluster will not follow the remote node but keep ``primary_conninfo`` as it is.
+
+
 Version 3.0.4
 -------------
 

patroni/dcs/__init__.py

@@ -170,22 +170,32 @@ _Version = Union[int, str]
 _Session = Union[int, float, str, None]
 
 
-class Member(NamedTuple):
+class Member(NamedTuple('Member',
+                        [('version', _Version),
+                         ('name', str),
+                         ('session', _Session),
+                         ('data', Dict[str, Any])])):
     """Immutable object (namedtuple) which represents single member of PostgreSQL cluster.
-    Consists of the following fields:
-    :param version: modification version of a given member key in a Configuration Store
-    :param name: name of PostgreSQL cluster member
-    :param session: either session id or just ttl in seconds
-    :param data: arbitrary data i.e. conn_url, api_url, xlog location, state, role, tags, etc...
 
-    There are two mandatory keys in a data:
-    conn_url: connection string containing host, user and password which could be used to access this member.
-    api_url: REST API url of patroni instance
+    .. note::
+        We are using an old-style attribute declaration here because otherwise it is not possible to override
+        ``__new__`` method in the :class:`RemoteMember` class.
+
+    .. note::
+        These two keys in data are always written to the DCS, but care is taken to maintain consistency and resilience
+        from data that is read:
+
+        ``conn_url``: connection string containing host, user and password which could be used to access this member.
+        ``api_url``: REST API url of patroni instance
+
+    Consists of the following fields:
+
+    :ivar version: modification version of a given member key in a Configuration Store.
+    :ivar name: name of PostgreSQL cluster member.
+    :ivar session: either session id or just ttl in seconds.
+    :ivar data: dictionary containing arbitrary data i.e. ``conn_url``, ``api_url``, ``xlog_location``, ``state``,
+                ``role``, ``tags``, etc...
     """
-    version: _Version
-    name: str
-    session: _Session
-    data: Dict[str, Any]
 
     @staticmethod
     def from_node(version: _Version, name: str, session: _Session, value: str) -> 'Member':
@@ -300,8 +310,14 @@ class RemoteMember(Member):
         'no_replication_slot'
     )
 
-    @classmethod
-    def from_name_and_data(cls, name: str, data: Dict[str, Any]) -> 'RemoteMember':
+    def __new__(cls, name: str, data: Dict[str, Any]) -> 'RemoteMember':
+        """Factory method to construct instance from given *name* and *data*.
+
+        :param name: name of the remote member.
+        :param data: dictionary of member information.
+
+        :returns: constructed instance using supplied parameters.
+        """
         return super(RemoteMember, cls).__new__(cls, -1, name, None, data)
 
     def __getattr__(self, name: str) -> Any:

patroni/dcs/etcd3.py

@@ -228,7 +228,7 @@ class Etcd3Client(AbstractEtcdClientWithFailover):
         return self.http.urlopen
 
     def _handle_server_response(self, response: urllib3.response.HTTPResponse) -> Dict[str, Any]:
-        data: Union[bytes, str] = response.data
+        data = response.data
         try:
             data = data.decode('utf-8')
             ret: Dict[str, Any] = json.loads(data)

patroni/dcs/kubernetes.py

@@ -134,6 +134,8 @@ class K8sConfig(object):
             config: Dict[str, Any] = yaml.safe_load(f)
 
         context = context or config['current-context']
+        if TYPE_CHECKING:  # pragma: no cover
+            assert isinstance(context, str)
         context_value = self._get_by_name(config, 'context', context)
         if TYPE_CHECKING:  # pragma: no cover
             assert isinstance(context_value, dict)

patroni/ha.py

@@ -101,9 +101,9 @@ class Failsafe(object):
     def leader(self) -> Optional[Leader]:
         with self._lock:
             if self._last_update + self._dcs.ttl > time.time() and self._name:
-                return Leader('', '', RemoteMember.from_name_and_data(self._name, {'api_url': self._api_url,
-                                                                                   'conn_url': self._conn_url,
-                                                                                   'slots': self._slots}))
+                return Leader('', '', RemoteMember(self._name, {'api_url': self._api_url,
+                                                                'conn_url': self._conn_url,
+                                                                'slots': self._slots}))
 
     def update_cluster(self, cluster: Cluster) -> Cluster:
         # Enreach cluster with the real leader if there was a ping from it
@@ -839,7 +839,7 @@ class Ha(object):
             data['slots'] = self.state_handler.slots()
         except Exception:
             logger.exception('Exception when called state_handler.slots()')
-        members = [RemoteMember.from_name_and_data(name, {'api_url': url})
+        members = [RemoteMember(name, {'api_url': url})
                    for name, url in failsafe.items() if name != self.state_handler.name]
         if not members:  # A sinlge node cluster
             return True
@@ -1046,8 +1046,7 @@ class Ha(object):
                 if failsafe_members and self.state_handler.name not in failsafe_members:
                     return False
                 # Race among not only existing cluster members, but also all known members from the failsafe config
-                all_known_members += [RemoteMember.from_name_and_data(name, {'api_url': url})
-                                      for name, url in failsafe_members.items()]
+                all_known_members += [RemoteMember(name, {'api_url': url}) for name, url in failsafe_members.items()]
         all_known_members += self.cluster.members
 
         # When in sync mode, only last known primary and sync standby are allowed to promote automatically.
@@ -1929,7 +1928,7 @@ class Ha(object):
                 data['conn_kwargs'] = conn_kwargs
 
         name = member.name if member else 'remote_member:{}'.format(uuid.uuid1())
-        return RemoteMember.from_name_and_data(name, data)
+        return RemoteMember(name, data)
 
     def get_failover_candidates(self, check_sync: bool = False) -> List[Member]:
         """Return list of candidates for either manual or automatic failover.

patroni/version.py

@@ -2,4 +2,4 @@
 
 :var __version__: the current Patroni version.
 """
-__version__ = '3.0.4'
+__version__ = '3.1.0'

tests/test_kubernetes.py

@@ -306,19 +306,19 @@ class TestKubernetesConfigMaps(BaseTestKubernetes):
 
         self.k.touch_member({'state': 'running', 'role': 'replica'})
         mock_patch_namespaced_pod.assert_called()
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['isMaster'], 'false')
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['tmp_role'], 'replica')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'false')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'replica')
 
         self.k.touch_member({'state': 'running', 'role': 'standby-leader'})
         mock_patch_namespaced_pod.assert_called()
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['isMaster'], 'false')
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['tmp_role'], 'standby-leader')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'false')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'standby-leader')
 
         self.k._name = 'p-0'
         self.k.touch_member({'role': 'primary'})
         mock_patch_namespaced_pod.assert_called()
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['isMaster'], 'true')
-        self.assertEqual(mock_patch_namespaced_pod.call_args.args[2].metadata.labels['tmp_role'], 'master')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['isMaster'], 'true')
+        self.assertEqual(mock_patch_namespaced_pod.call_args[0][2].metadata.labels['tmp_role'], 'master')
 
     def test_initialize(self):
         self.k.initialize()

tests/test_postgresql.py

@@ -346,8 +346,7 @@ class TestPostgresql(BaseTestPostgresql):
     @patch.object(Postgresql, 'start', Mock())
     def test_follow(self):
         self.p.call_nowait(CallbackAction.ON_START)
-        m = RemoteMember.from_name_and_data('1', {'restore_command': '2', 'primary_slot_name': 'foo',
-                                                  'conn_kwargs': {'host': 'bar'}})
+        m = RemoteMember('1', {'restore_command': '2', 'primary_slot_name': 'foo', 'conn_kwargs': {'host': 'bar'}})
         self.p.follow(m)
         with patch.object(Postgresql, 'ensure_major_version_is_known', Mock(return_value=False)):
             self.assertIsNone(self.p.follow(m))

tests/test_rewind.py

@@ -253,8 +253,8 @@ class TestRewind(BaseTestPostgresql):
                                          mock_logger_info.call_args[0])
                         mock_logger_info.reset_mock()
                         mock_subprocess_call.assert_called_once()
-                        self.assertEqual(mock_subprocess_call.call_args.args[0], ['command 000000000000000000000000'])
-                        self.assertEqual(mock_subprocess_call.call_args.kwargs['shell'], True)
+                        self.assertEqual(mock_subprocess_call.call_args[0][0], ['command 000000000000000000000000'])
+                        self.assertEqual(mock_subprocess_call.call_args[1]['shell'], True)
                         mock_subprocess_call.reset_mock()
 
             # failed archive_command call