Repository: zalando/patroni
Pull request #1848 (2021-02-20 02:13:44)

Quick attempt at Prometheus
Let's talk about this for #318, I had a little bit of downtime this evening to start something.

Here's hitting my leader:

```bash
❯ curl -s xxx:8008/metrics
# HELP patroni_running Value is 1 if Patroni/Postgres is up, 0 otherwise.
# TYPE patroni_running gauge
patroni_running 1
# HELP patroni_postmaster_start_time Epoch seconds since Patroni/Postgres started.
# TYPE patroni_postmaster_start_time counter
patroni_postmaster_start_time 1613786855
# HELP patroni_master Value is 1 if this node is the leader, 0 otherwise.
# TYPE patroni_master gauge
patroni_master 1
# HELP patroni_xlog_location Current location of the Postgres transaction log, 0 if this node is not the leader.
# TYPE patroni_xlog_location counter
patroni_xlog_location 50337208
# HELP patroni_replica Value is 1 if this node is a replica, 0 otherwise.
# TYPE patroni_replica gauge
patroni_replica 0
# HELP patroni_xlog_received_location Current location of the received Postgres transaction log, 0 if this node is not a replica.
# TYPE patroni_xlog_received_location counter
patroni_xlog_received_location 0
# HELP patroni_server_version Version of Patroni.
# TYPE patroni_server_version gauge
patroni_server_version 100015
# HELP patroni_cluster_unlocked Value is 1 if the cluster is unlocked, 0 if locked.
# TYPE patroni_cluster_unlocked gauge
patroni_cluster_unlocked 0
# HELP patroni_timeline Current Postgres timeline number of this node.
# TYPE patroni_timeline counter
patroni_timeline 14
```

Here's hitting my replica:

```bash
❯ curl -s yyy:8008/metrics
# HELP patroni_running Value is 1 if Patroni/Postgres is up, 0 otherwise.
# TYPE patroni_running gauge
patroni_running 1
# HELP patroni_postmaster_start_time Epoch seconds since Patroni/Postgres started.
# TYPE patroni_postmaster_start_time counter
patroni_postmaster_start_time 1613786867
# HELP patroni_master Value is 1 if this node is the leader, 0 otherwise.
# TYPE patroni_master gauge
patroni_master 0
# HELP patroni_xlog_location Current location of the Postgres transaction log, 0 if this node is not the leader.
# TYPE patroni_xlog_location counter
patroni_xlog_location 0
# HELP patroni_replica Value is 1 if this node is a replica, 0 otherwise.
# TYPE patroni_replica gauge
patroni_replica 1
# HELP patroni_xlog_received_location Current location of the received Postgres transaction log, 0 if this node is not a replica.
# TYPE patroni_xlog_received_location counter
patroni_xlog_received_location 50337208
# HELP patroni_server_version Version of Patroni.
# TYPE patroni_server_version gauge
patroni_server_version 100015
# HELP patroni_cluster_unlocked Value is 1 if the cluster is unlocked, 0 if locked.
# TYPE patroni_cluster_unlocked gauge
patroni_cluster_unlocked 0
# HELP patroni_timeline Current Postgres timeline number of this node.
# TYPE patroni_timeline counter
patroni_timeline 14
```

Here's my cluster topology:

```bash
(patroni) postgres@db1:~$ patronictl -c /etc/patroni/config.yml topology
+ Cluster: postgres (6931151304255215270) ---+----+-----------+
| Member | Host          | Role    | State   | TL | Lag in MB |
+--------+---------------+---------+---------+----+-----------+
| db1    | xxx | Leader  | running | 14 |           |
| + db2  | yyy | Replica | running | 14 |         0 |
+--------+---------------+---------+---------+----+-----------+
```

mamercad: I'll work on the coverage when I get some time.
mamercad: Fixes #318 maybe :)
mamercad: Thanks for helping me @CyberDem0n !
mamercad: > Quite a lot of duplicated code.
> 
> Also, the role could be the `standby_leader`.
> Last, but not least, not all values are always available. For example if postgres is not in the running state there will be no xlog metrics.

I'll tackle these in a little while, thank you for helping me.

Patch:
diff --git a/docs/releases.rst b/docs/releases.rst
index 03ed170b..6d484c4f 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -3,6 +3,102 @@
 Release notes
 =============
 
+Version 2.0.2
+-------------
+
+**New features**
+
+- Ability to ignore externally managed replication slots (James Coleman)
+
+  Patroni is trying to remove any replication slot which is unknown to it, but there are certainly cases when replication slots should be managed externally. From now on it is possible to configure slots that should not be removed.
+
+- Added support for cipher suite limitation for REST API (Gunnar "Nick" Bluth)
+
+  It could be configured via ``restapi.ciphers`` or the ``PATRONI_RESTAPI_CIPHERS`` environment variable.
+
+- Added support for encrypted TLS keys for REST API (Jonathan S. Katz)
+
+  It could be configured via ``restapi.keyfile_password`` or the ``PATRONI_RESTAPI_KEYFILE_PASSWORD`` environment variable.
+
+- Constant time comparison of REST API authentication credentials (Alex Brasetvik)
+
+  Use ``hmac.compare_digest()`` instead of ``==``, which is vulnerable to timing attack.
+
+- Choose synchronous nodes based on replication lag (Krishna Sarabu)
+
+  If the replication lag on the synchronous node starts exceeding the configured threshold it could be demoted to asynchronous and/or replaced by the other node. Behaviour is controlled with ``maximum_lag_on_syncnode``.
+
+
+**Stability improvements**
+
+- Start postgres with ``hot_standby = off`` when doing custom bootstrap (Igor Yanchenko)
+
+  During custom bootstrap Patroni is restoring the basebackup, starting Postgres up, and waiting until recovery finishes. Some PostgreSQL parameters on the standby can't be smaller than on the primary and if the new value (restored from WAL) is higher than the configured one, Postgres panics and stops. In order to avoid such behavior we will do custom bootstrap without ``hot_standby`` mode.
+
+- Warn the user if the required watchdog is not healthy (Nicolas Thauvin)
+
+  When the watchdog device is not writable or missing in required mode, the member cannot be promoted. Added a warning to show the user where to search for this misconfiguration.
+
+- Better verbosity for single-user mode recovery (Alexander Kukushkin)
+
+  If Patroni notices that PostgreSQL wasn't shutdown clearly, in certain cases the crash-recovery is executed by starting Postgres in single-user mode. It could happen that the recovery failed (for example due to the lack of space on disk) but errors were swallowed.
+
+- Added compatibility with ``python-consul2`` module (Alexander, Wilfried Roset)
+
+  The good old ``python-consul`` is not maintained since a few years, therefore someone created a fork with new features and bug-fixes.
+
+- Don't use ``bypass_api_service`` when running ``patronictl`` (Alexander)
+
+  When a K8s pod is running in a non-``default`` namespace it does not necessarily have enough permissions to query the ``kubernetes`` endpoint. In this case Patroni shows the warning and ignores the ``bypass_api_service`` setting. In case of ``patronictl`` the warning was a bit annoying.
+
+- Create ``raft.data_dir`` if it doesn't exists or make sure that it is writable (Mark Mercado)
+
+  Improves user-friendliness and usability.
+
+
+**Bugfixes**
+
+- Don't interrupt restart or promote if lost leader lock in pause (Alexander)
+
+  In pause it is allowed to run postgres as primary without lock.
+
+- Fixed issue with ``shutdown_request()`` in the REST API (Nicolas Limage)
+
+  In order to improve handling of SSL connections and delay the handshake until thread is started Patroni overrides a few methods in the ``HTTPServer``. The ``shutdown_request()`` method was forgotten.
+
+- Fixed issue with sleep time when using Zookeeper (Alexander)
+
+  There were chances that Patroni was sleeping up to twice longer between running HA code.
+
+- Fixed invalid ``os.symlink()`` calls when moving data directory after failed bootstrap (Andrew L'Ecuyer)
+
+  If the bootstrap failed Patroni is renaming data directory, pg_wal, and all tablespaces. After that it updates symlinks so filesystem remains consistent. The symlink creation was failing due to the ``src`` and ``dst`` arguments being swapped.
+
+- Fixed bug in the post_bootstrap() method (Alexander)
+
+  If the superuser password wasn't configured Patroni was failing to call the ``post_init`` script and therefore the whole bootstrap was failing.
+
+- Fixed an issues with pg_rewind in the standby cluster (Alexander)
+
+  If the superuser name is different from Postgres, the ``pg_rewind`` in the standby cluster was failing because the connection string didn't contain the database name.
+
+- Exit only if authentication with Etcd v3 explicitly failed (Alexander)
+
+  On start Patroni performs discovery of Etcd cluster topology and authenticates if it is necessarily. It could happen that one of etcd servers is not accessible, Patroni was trying to perform authentication on this server and failing instead of retrying with the next node.
+
+- Handle case with psutil cmdline() returning empty list (Alexander)
+
+  Zombie processes are still postmasters children, but they don't have cmdline()
+
+- Treat ``PATRONI_KUBERNETES_USE_ENDPOINTS`` environment variable as boolean (Alexander)
+
+  Not doing so was making impossible disabling ``kubernetes.use_endpoints`` via environment.
+
+- Improve handling of concurrent endpoint update errors (Alexander)
+
+  Patroni will explicitly query the current endpoint object, verify that the current pod still holds the leader lock and repeat the update.
+
+
 Version 2.0.1
 -------------
 
diff --git a/patroni/api.py b/patroni/api.py
index 06be43ef..e02d5acf 100644
--- a/patroni/api.py
+++ b/patroni/api.py
@@ -45,7 +45,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
         self.wfile.write(body.encode('utf-8'))
 
     def _write_json_response(self, status_code, response):
-        self._write_response(status_code, json.dumps(response), content_type='application/json')
+        self._write_response(status_code, json.dumps(response, default=str), content_type='application/json')
 
     def check_auth(func):
         """Decorator function to check authorization header or client certificates
@@ -180,6 +180,83 @@ class RestApiHandler(BaseHTTPRequestHandler):
         else:
             self.send_error(502)
 
+    def do_GET_metrics(self):
+        postgres = self.get_postgresql_status(True)
+        patroni = self.server.patroni
+        epoch = datetime.datetime(1970, 1, 1, tzinfo=tzutc)
+
+        metrics = []
+
+        metrics.append("# HELP patroni_version Patroni semver without periods.")
+        metrics.append("# TYPE patroni_version gauge")
+        padded_semver = ''.join([x.zfill(2) for x in patroni.version.split('.')])  # 2.0.2 => 020002
+        metrics.append("patroni_version {0}".format(padded_semver))
+
+        metrics.append("# HELP patroni_postgres_running Value is 1 if Postgres is running, 0 otherwise.")
+        metrics.append("# TYPE patroni_postgres_running gauge")
+        metrics.append("patroni_postgres_running {0}".format(int(postgres['state'] == 'running')))
+
+        metrics.append("# HELP patroni_postmaster_start_time Epoch seconds since Postgres started.")
+        metrics.append("# TYPE patroni_postmaster_start_time gauge")
+        postmaster_start_time = postgres.get('postmaster_start_time')
+        postmaster_start_time = (postmaster_start_time - epoch).total_seconds() if postmaster_start_time else 0
+        metrics.append("patroni_postmaster_start_time {0}".format(postmaster_start_time))
+
+        metrics.append("# HELP patroni_master Value is 1 if this node is the leader, 0 otherwise.")
+        metrics.append("# TYPE patroni_master gauge")
+        metrics.append("patroni_master {0}".format(int(postgres['role'] == 'master')))
+
+        metrics.append("# HELP patroni_xlog_location Current location of the Postgres"
+                       " transaction log, 0 if this node is not the leader.")
+        metrics.append("# TYPE patroni_xlog_location counter")
+        metrics.append("patroni_xlog_location {0}".format(postgres.get('xlog', {}).get('location', 0)))
+
+        metrics.append("# HELP patroni_standby_leader Value is 1 if this node is the standby_leader, 0 otherwise.")
+        metrics.append("# TYPE patroni_standby_leader gauge")
+        metrics.append("patroni_standby_leader {0}".format(int(postgres['role'] == 'standby_leader')))
+
+        metrics.append("# HELP patroni_replica Value is 1 if this node is a replica, 0 otherwise.")
+        metrics.append("# TYPE patroni_replica gauge")
+        metrics.append("patroni_replica {0}".format(int(postgres['role'] == 'replica')))
+
+        metrics.append("# HELP patroni_xlog_received_location Current location of the received"
+                       " Postgres transaction log, 0 if this node is not a replica.")
+        metrics.append("# TYPE patroni_xlog_received_location counter")
+        metrics.append("patroni_xlog_received_location {0}".format(
+                        postgres.get('xlog', {}).get('received_location', 0)))
+
+        metrics.append("# HELP patroni_xlog_replayed_location Current location of the replayed"
+                       " Postgres transaction log, 0 if this node is not a replica.")
+        metrics.append("# TYPE patroni_xlog_replayed_location counter")
+        metrics.append("patroni_xlog_replayed_location {0}".format(
+                        postgres.get('xlog', {}).get('replayed_location', 0)))
+
+        metrics.append("# HELP patroni_xlog_replayed_timestamp Current timestamp of the replayed"
+                       " Postgres transaction log, 0 if null.")
+        metrics.append("# TYPE patroni_xlog_replayed_timestamp gauge")
+        replayed_timestamp = postgres.get('xlog', {}).get('replayed_timestamp')
+        replayed_timestamp = (replayed_timestamp - epoch).total_seconds() if replayed_timestamp else 0
+        metrics.append("patroni_xlog_replayed_timestamp {0}".format(replayed_timestamp))
+
+        metrics.append("# HELP patroni_xlog_paused Value is 1 if the Postgres xlog is paused, 0 otherwise.")
+        metrics.append("# TYPE patroni_xlog_paused gauge")
+        metrics.append("patroni_xlog_paused {0}".format(
+                        int(postgres.get('xlog', {}).get('paused', False) is True)))
+
+        metrics.append("# HELP patroni_postgres_server_version Version of Postgres (if running), 0 otherwise.")
+        metrics.append("# TYPE patroni_postgres_server_version gauge")
+        metrics.append("patroni_postgres_server_version {0}".format(postgres.get('server_version', 0)))
+
+        metrics.append("# HELP patroni_cluster_unlocked Value is 1 if the cluster is unlocked, 0 if locked.")
+        metrics.append("# TYPE patroni_cluster_unlocked gauge")
+        metrics.append("patroni_cluster_unlocked {0}".format(int(postgres['cluster_unlocked'])))
+
+        metrics.append("# HELP patroni_postgres_timeline Postgres timeline of this node (if running), 0 otherwise.")
+        metrics.append("# TYPE patroni_postgres_timeline counter")
+        metrics.append("patroni_postgres_timeline {0}".format(postgres.get('timeline', 0)))
+
+        self._write_response(200, '\n'.join(metrics)+'\n', content_type='text/plain')
+
     def _read_json_content(self, body_is_optional=False):
         if 'content-length' not in self.headers:
             return self.send_error(411) if not body_is_optional else {}
@@ -476,7 +553,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
             if postgresql.state not in ('running', 'restarting', 'starting'):
                 raise RetryFailedError('')
             stmt = ("SELECT " + postgresql.POSTMASTER_START_TIME + ", " + postgresql.TL_LSN + ","
-                    " pg_catalog.to_char(pg_catalog.pg_last_xact_replay_timestamp(), 'YYYY-MM-DD HH24:MI:SS.MS TZ'),"
+                    " pg_catalog.pg_last_xact_replay_timestamp(),"
                     " pg_catalog.array_to_json(pg_catalog.array_agg(pg_catalog.row_to_json(ri))) "
                     "FROM (SELECT (SELECT rolname FROM pg_authid WHERE oid = usesysid) AS usename,"
                     " application_name, client_addr, w.state, sync_state, sync_priority"
diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py
index 7c5eb1f8..aced2d7c 100644
--- a/patroni/postgresql/__init__.py
+++ b/patroni/postgresql/__init__.py
@@ -48,7 +48,7 @@ def null_context():
 
 class Postgresql(object):
 
-    POSTMASTER_START_TIME = "pg_catalog.to_char(pg_catalog.pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ')"
+    POSTMASTER_START_TIME = "pg_catalog.pg_postmaster_start_time()"
     TL_LSN = ("CASE WHEN pg_catalog.pg_is_in_recovery() THEN 0 "
               "ELSE ('x' || pg_catalog.substr(pg_catalog.pg_{0}file_name("
               "pg_catalog.pg_current_{0}_{1}()), 1, 8))::bit(32)::int END, "  # master timeline
@@ -865,10 +865,10 @@ class Postgresql(object):
         try:
             query = "SELECT " + self.POSTMASTER_START_TIME
             if current_thread().ident == self.__thread_ident:
-                return self.query(query).fetchone()[0]
+                return self.query(query).fetchone()[0].isoformat(sep=' ')
             with self.connection().cursor() as cursor:
                 cursor.execute(query)
-                return cursor.fetchone()[0]
+                return cursor.fetchone()[0].isoformat(sep=' ')
         except psycopg2.Error:
             return None
 
diff --git a/patroni/version.py b/patroni/version.py
index 3f390799..668c3446 100644
--- a/patroni/version.py
+++ b/patroni/version.py
@@ -1,1 +1,1 @@
-__version__ = '2.0.1'
+__version__ = '2.0.2'


Test patch:
diff --git a/tests/__init__.py b/tests/__init__.py
index 32c65cc4..23861881 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -1,3 +1,4 @@
+import datetime
 import os
 import shutil
 import unittest
@@ -10,7 +11,7 @@ import urllib3
 from patroni.dcs import Leader, Member
 from patroni.postgresql import Postgresql
 from patroni.postgresql.config import ConfigHandler
-from patroni.utils import RetryFailedError
+from patroni.utils import RetryFailedError, tzutc
 
 
 class SleepException(Exception):
@@ -95,10 +96,11 @@ class MockCursor(object):
             self.results = [(1, 2, 1, 0, False, 1, 1, None, None)]
         elif sql.startswith('SELECT pg_catalog.pg_is_in_recovery()'):
             self.results = [(False, 2)]
-        elif sql.startswith('SELECT pg_catalog.to_char'):
+        elif sql.startswith('SELECT pg_catalog.pg_postmaster_start_time'):
             replication_info = '[{"application_name":"walreceiver","client_addr":"1.2.3.4",' +\
                                '"state":"streaming","sync_state":"async","sync_priority":0}]'
-            self.results = [('', 0, '', 0, '', '', False, replication_info)]
+            now = datetime.datetime.now(tzutc)
+            self.results = [(now, 0, '', 0, '', False, now, replication_info)]
         elif sql.startswith('SELECT name, setting'):
             self.results = [('wal_segment_size', '2048', '8kB', 'integer', 'internal'),
                             ('wal_block_size', '8192', None, 'integer', 'internal'),
diff --git a/tests/test_api.py b/tests/test_api.py
index a382fa6f..ec91267e 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -30,7 +30,7 @@ class MockPostgresql(object):
     pending_restart = True
     wal_name = 'wal'
     lsn_name = 'lsn'
-    POSTMASTER_START_TIME = 'pg_catalog.to_char(pg_catalog.pg_postmaster_start_time'
+    POSTMASTER_START_TIME = 'pg_catalog.pg_postmaster_start_time()'
     TL_LSN = 'CASE WHEN pg_catalog.pg_is_in_recovery()'
 
     @staticmethod
@@ -39,7 +39,7 @@ class MockPostgresql(object):
 
     @staticmethod
     def postmaster_start_time():
-        return str(postmaster_start_time)
+        return postmaster_start_time
 
     @staticmethod
     def replica_cached_timeline(_):
@@ -236,6 +236,10 @@ class TestRestApiHandler(unittest.TestCase):
         mock_dcs.cluster.config = None
         self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /config'))
 
+    @patch.object(MockPatroni, 'dcs')
+    def test_do_GET_metrics(self, mock_dcs):
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /metrics'))
+
     @patch.object(MockPatroni, 'dcs')
     def test_do_PATCH_config(self, mock_dcs):
         config = {'postgresql': {'use_slots': False, 'use_pg_rewind': True, 'parameters': {'wal_level': 'logical'}}}
@@ -446,10 +450,12 @@ class TestRestApiHandler(unittest.TestCase):
 class TestRestApiServer(unittest.TestCase):
 
     @patch('ssl.SSLContext.load_cert_chain', Mock())
+    @patch('ssl.SSLContext.set_ciphers', Mock())
     @patch('ssl.SSLContext.wrap_socket', Mock(return_value=0))
     @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock())
     def setUp(self):
-        self.srv = MockRestApiServer(Mock(), '', {'listen': '*:8008', 'certfile': 'a', 'verify_client': 'required'})
+        self.srv = MockRestApiServer(Mock(), '', {'listen': '*:8008', 'certfile': 'a', 'verify_client': 'required',
+                                                  'ciphers': '!SSLv1:!SSLv2:!SSLv3:!TLSv1:!TLSv1.1'})
 
     @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock())
     def test_reload_config(self):
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index b22d1a03..3a3937d4 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -1,3 +1,4 @@
+import datetime
 import mock  # for the mock.call method, importing it without a namespace breaks python3
 import os
 import psutil
@@ -522,8 +523,9 @@ class TestPostgresql(BaseTestPostgresql):
             self.assertEqual(self.p.get_major_version(), 0)
 
     def test_postmaster_start_time(self):
-        with patch.object(MockCursor, "fetchone", Mock(return_value=('foo', True, '', '', '', '', False))):
-            self.assertEqual(self.p.postmaster_start_time(), 'foo')
+        now = datetime.datetime.now()
+        with patch.object(MockCursor, "fetchone", Mock(return_value=(now, True, '', '', '', '', False))):
+            self.assertEqual(self.p.postmaster_start_time(), now.isoformat(sep=' '))
             t = Thread(target=self.p.postmaster_start_time)
             t.start()
             t.join()
diff --git a/tests/test_raft.py b/tests/test_raft.py
index d1e245bc..4b51ebe0 100644
--- a/tests/test_raft.py
+++ b/tests/test_raft.py
@@ -1,5 +1,6 @@
 import os
 import unittest
+import tempfile
 import time
 
 from mock import Mock, patch
@@ -121,8 +122,11 @@ class TestKVStoreTTL(unittest.TestCase):
 
 class TestRaft(unittest.TestCase):
 
+    _TMP = tempfile.gettempdir()
+
     def test_raft(self):
-        raft = Raft({'ttl': 30, 'scope': 'test', 'name': 'pg', 'self_addr': '127.0.0.1:1234', 'retry_timeout': 10})
+        raft = Raft({'ttl': 30, 'scope': 'test', 'name': 'pg', 'self_addr': '127.0.0.1:1234',
+                     'retry_timeout': 10, 'data_dir': self._TMP})
         raft.set_retry_timeout(20)
         raft.set_ttl(60)
         self.assertTrue(raft.touch_member(''))
@@ -144,7 +148,7 @@ class TestRaft(unittest.TestCase):
         raft._sync_obj._SyncObj__thread.join()
 
     def tearDown(self):
-        remove_files('127.0.0.1:1234.')
+        remove_files(os.path.join(self._TMP, '127.0.0.1:1234.'))
 
     def setUp(self):
         self.tearDown()
@@ -154,4 +158,5 @@ class TestRaft(unittest.TestCase):
     def test_init(self, mock_event, mock_kvstore):
         mock_kvstore.return_value.applied_local_log = False
         mock_event.return_value.isSet.side_effect = [False, True]
-        self.assertIsNotNone(Raft({'ttl': 30, 'scope': 'test', 'name': 'pg', 'patronictl': True, 'self_addr': '1'}))
+        self.assertIsNotNone(Raft({'ttl': 30, 'scope': 'test', 'name': 'pg', 'patronictl': True,
+                                   'self_addr': '1', 'data_dir': self._TMP}))