Repository: zalando/patroni
Pull request #2349 (2022-06-22 12:43:05)

Always return checkpoint location as integer
before it was also returned as str in some cases

sdudoladov: :+1: 

Patch:
diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst
index 9712f445..205f80dd 100644
--- a/docs/SETTINGS.rst
+++ b/docs/SETTINGS.rst
@@ -131,7 +131,7 @@ Most of the parameters are optional, but you have to specify one of the **host**
 -  **checks**: (optional) list of Consul health checks used for the session. By default an empty list is used.
 -  **register\_service**: (optional) whether or not to register a service with the name defined by the scope parameter and the tag master, replica or standby-leader depending on the node's role. Defaults to **false**.
 -  **service\_tags**: (optional) additional static tags to add to the Consul service apart from the role (``master``/``replica``/``standby-leader``).  By default an empty list is used.
--  **service\_check\_interval**: (optional) how often to perform health check against registered url.
+-  **service\_check\_interval**: (optional) how often to perform health check against registered url. Defaults to '5s'.
 -  **service\_check\_tls\_server\_name**: (optional) overide SNI host when connecting via TLS, see also `consul agent check API reference <https://www.consul.io/api-docs/agent/check#tlsservername>`__.
 
 The ``token`` needs to have the following ACL permissions:
diff --git a/docs/replica_bootstrap.rst b/docs/replica_bootstrap.rst
index a9ed7848..1524f7f2 100644
--- a/docs/replica_bootstrap.rst
+++ b/docs/replica_bootstrap.rst
@@ -212,4 +212,9 @@ in a patroni configuration:
 Note, that these options will be applied only once during cluster bootstrap,
 and the only way to change them afterwards is through DCS.
 
+Patroni expects to find `postgresql.conf` or `postgresql.conf.backup` in PGDATA
+of the remote master and will not start if it does not find it after a
+basebackup. If the remote master keeps its `postgresql.conf` elsewhere, it is
+your responsibility to copy it to PGDATA.
+
 If you use replication slots on the standby cluster, you must also create the corresponding replication slot on the primary cluster.  It will not be done automatically by the standby cluster implementation.  You can use Patroni's permanent replication slots feature on the primary cluster to maintain a replication slot with the same name as ``primary_slot_name``, or its default value if ``primary_slot_name`` is not provided.
diff --git a/patroni/config.py b/patroni/config.py
index 5996f68e..0ae9726a 100644
--- a/patroni/config.py
+++ b/patroni/config.py
@@ -102,9 +102,9 @@ class Config(object):
             config_env = os.environ.pop(self.PATRONI_CONFIG_VARIABLE, None)
             self._local_configuration = config_env and yaml.safe_load(config_env) or self.__environment_configuration
         if validator:
-            error = validator(self._local_configuration)
-            if error:
-                raise ConfigParseError(error)
+            errors = validator(self._local_configuration)
+            if errors:
+                raise ConfigParseError("\n".join(errors))
 
         self.__effective_configuration = self._build_effective_configuration({}, self._local_configuration)
         self._data_dir = self.__effective_configuration.get('postgresql', {}).get('data_dir', "")
diff --git a/patroni/daemon.py b/patroni/daemon.py
index 5bb99990..ca118acf 100644
--- a/patroni/daemon.py
+++ b/patroni/daemon.py
@@ -1,3 +1,5 @@
+from __future__ import print_function
+
 import abc
 import os
 import signal
@@ -83,16 +85,18 @@ def abstract_main(cls, validator=None):
                         help='Patroni may also read the configuration from the {0} environment variable'
                         .format(Config.PATRONI_CONFIG_VARIABLE))
     args = parser.parse_args()
+    validate_config = validator and args.validate_config
     try:
-        if validator and args.validate_config:
+        if validate_config:
             Config(args.configfile, validator=validator)
             sys.exit()
 
         config = Config(args.configfile)
     except ConfigParseError as e:
         if e.value:
-            print(e.value)
-        parser.print_help()
+            print(e.value, file=sys.stderr)
+        if not validate_config:
+            parser.print_help()
         sys.exit(1)
 
     controller = cls(config)
diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py
index c52c397b..a5b3d592 100644
--- a/patroni/postgresql/__init__.py
+++ b/patroni/postgresql/__init__.py
@@ -428,11 +428,11 @@ class Postgresql(object):
                     # If the cluster is shutdown with archive_mode=on, WAL is switched before writing the checkpoint.
                     # In this case we want to take the LSN of previous record (switch) as the last known WAL location.
                     if parse_lsn(lsn) == prev and desc.strip() in ('xlog switch', 'SWITCH'):
-                        return str(prev)
+                        return prev
             except Exception as e:
                 logger.error('Exception when parsing WAL pg_%sdump output: %r', self.wal_name, e)
             if isinstance(checkpoint_lsn, six.integer_types):
-                return str(checkpoint_lsn)
+                return checkpoint_lsn
 
     def is_running(self):
         """Returns PostmasterProcess if one is running on the data directory or None. If most recently seen process
@@ -668,7 +668,7 @@ class Postgresql(object):
             while postmaster.is_running():
                 data = self.controldata()
                 if data.get('Database cluster state', '') == 'shut down':
-                    on_shutdown(int(self.latest_checkpoint_location()))
+                    on_shutdown(self.latest_checkpoint_location())
                     break
                 elif data.get('Database cluster state', '').startswith('shut down'):  # shut down in recovery
                     break
diff --git a/patroni/validator.py b/patroni/validator.py
index 6d2a060c..595d4355 100644
--- a/patroni/validator.py
+++ b/patroni/validator.py
@@ -178,9 +178,11 @@ class Schema(object):
         self.validator = validator
 
     def __call__(self, data):
+        errors = []
         for i in self.validate(data):
             if not i.status:
-                print(i)
+                errors.append(str(i))
+        return errors
 
     def validate(self, data):
         self.data = data


Test patch:
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
index 3f9bd738..055ee2e0 100644
--- a/tests/test_postgresql.py
+++ b/tests/test_postgresql.py
@@ -347,7 +347,7 @@ class TestPostgresql(BaseTestPostgresql):
     @patch('subprocess.Popen')
     def test_latest_checkpoint_location(self, mock_popen):
         mock_popen.return_value.communicate.return_value = (None, None)
-        self.assertEqual(self.p.latest_checkpoint_location(), '28163096')
+        self.assertEqual(self.p.latest_checkpoint_location(), 28163096)
         # 9.3 and 9.4 format
         mock_popen.return_value.communicate.side_effect = [
             (b'rmgr: XLOG        len (rec/tot):     72/   104, tx:          0, lsn: 0/01ADBC18, prev 0/01ADBBB8, ' +
@@ -355,14 +355,14 @@ class TestPostgresql(BaseTestPostgresql):
              b' 1; offset 0; oldest xid 715 in DB 1; oldest multi 1 in DB 1; oldest running xid 0; shutdown', None),
             (b'rmgr: Transaction len (rec/tot):     64/    96, tx:        726, lsn: 0/01ADBBB8, prev 0/01ADBB70, ' +
              b'bkp: 0000, desc: commit: 2021-02-26 11:19:37.900918 CET; inval msgs: catcache 11 catcache 10', None)]
-        self.assertEqual(self.p.latest_checkpoint_location(), '28163096')
+        self.assertEqual(self.p.latest_checkpoint_location(), 28163096)
         mock_popen.return_value.communicate.side_effect = [
             (b'rmgr: XLOG        len (rec/tot):     72/   104, tx:          0, lsn: 0/01ADBC18, prev 0/01ADBBB8, ' +
              b'bkp: 0000, desc: checkpoint: redo 0/1ADBC18; tli 1; prev tli 1; fpw true; xid 0/727; oid 16386; multi' +
              b' 1; offset 0; oldest xid 715 in DB 1; oldest multi 1 in DB 1; oldest running xid 0; shutdown', None),
             (b'rmgr: XLOG        len (rec/tot):      0/    32, tx:          0, lsn: 0/01ADBBB8, prev 0/01ADBBA0, ' +
              b'bkp: 0000, desc: xlog switch ', None)]
-        self.assertEqual(self.p.latest_checkpoint_location(), '28163000')
+        self.assertEqual(self.p.latest_checkpoint_location(), 28163000)
         # 9.5+ format
         mock_popen.return_value.communicate.side_effect = [
             (b'rmgr: XLOG        len (rec/tot):    114/   114, tx:          0, lsn: 0/01ADBC18, prev 0/018260F8, ' +
@@ -371,7 +371,7 @@ class TestPostgresql(BaseTestPostgresql):
              b' oldest running xid 0; shutdown', None),
             (b'rmgr: XLOG        len (rec/tot):     24/    24, tx:          0, lsn: 0/018260F8, prev 0/01826080, ' +
              b'desc: SWITCH ', None)]
-        self.assertEqual(self.p.latest_checkpoint_location(), '25321720')
+        self.assertEqual(self.p.latest_checkpoint_location(), 25321720)
 
     def test_reload(self):
         self.assertTrue(self.p.reload())
diff --git a/tests/test_validator.py b/tests/test_validator.py
index a04403ce..830b415e 100644
--- a/tests/test_validator.py
+++ b/tests/test_validator.py
@@ -141,14 +141,14 @@ class TestValidator(unittest.TestCase):
         del directories[:]
 
     def test_empty_config(self, mock_out, mock_err):
-        schema({})
-        output = mock_out.getvalue()
+        errors = schema({})
+        output = "\n".join(errors)
         expected = list(sorted(['name', 'postgresql', 'restapi', 'scope'] + available_dcs))
         self.assertEqual(expected, parse_output(output))
 
     def test_complete_config(self, mock_out, mock_err):
-        schema(config)
-        output = mock_out.getvalue()
+        errors = schema(config)
+        output = "\n".join(errors)
         self.assertEqual(['postgresql.bin_dir', 'raft.bind_addr', 'raft.self_addr'], parse_output(output))
 
     def test_bin_dir_is_file(self, mock_out, mock_err):
@@ -158,8 +158,8 @@ class TestValidator(unittest.TestCase):
         c["restapi"]["connect_address"] = 'False:blabla'
         c["etcd"]["hosts"] = ["127.0.0.1:2379", "1244.0.0.1:2379", "127.0.0.1:invalidport"]
         c["kubernetes"]["pod_ip"] = "127.0.0.1111"
-        schema(c)
-        output = mock_out.getvalue()
+        errors = schema(c)
+        output = "\n".join(errors)
         self.assertEqual(['etcd.hosts.1', 'etcd.hosts.2', 'kubernetes.pod_ip', 'postgresql.bin_dir',
                           'postgresql.data_dir', 'raft.bind_addr', 'raft.self_addr',
                           'restapi.connect_address'], parse_output(output))
@@ -176,8 +176,8 @@ class TestValidator(unittest.TestCase):
         c["etcd"]["host"] = "127.0.0.1:237"
         c["postgresql"]["listen"] = "127.0.0.1:5432"
         with patch('patroni.validator.open', mock_open(read_data='9')):
-            schema(c)
-        output = mock_out.getvalue()
+            errors = schema(c)
+        output = "\n".join(errors)
         self.assertEqual(['consul.host', 'etcd.host', 'postgresql.bin_dir', 'postgresql.data_dir', 'postgresql.listen',
                           'raft.bind_addr', 'raft.self_addr', 'restapi.connect_address'], parse_output(output))
 
@@ -195,8 +195,8 @@ class TestValidator(unittest.TestCase):
         files.append(os.path.join(config["postgresql"]["bin_dir"], "postgres"))
         files.append(os.path.join(config["postgresql"]["bin_dir"], "pg_isready"))
         with patch('patroni.validator.open', mock_open(read_data='12')):
-            schema(config)
-        output = mock_out.getvalue()
+            errors = schema(config)
+        output = "\n".join(errors)
         self.assertEqual(['raft.bind_addr', 'raft.self_addr'], parse_output(output))
 
     @patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 12.1"))
@@ -210,8 +210,8 @@ class TestValidator(unittest.TestCase):
         c["etcd"]["hosts"] = []
         del c["postgresql"]["bin_dir"]
         with patch('patroni.validator.open', mock_open(read_data='11')):
-            schema(c)
-        output = mock_out.getvalue()
+            errors = schema(c)
+        output = "\n".join(errors)
         self.assertEqual(['etcd.hosts', 'postgresql.data_dir',
                           'raft.bind_addr', 'raft.self_addr'], parse_output(output))
 
@@ -224,8 +224,8 @@ class TestValidator(unittest.TestCase):
         c = copy.deepcopy(config)
         del c["postgresql"]["bin_dir"]
         with patch('patroni.validator.open', mock_open(read_data='11')):
-            schema(c)
-        output = mock_out.getvalue()
+            errors = schema(c)
+        output = "\n".join(errors)
         self.assertEqual(['postgresql.data_dir', 'raft.bind_addr', 'raft.self_addr'], parse_output(output))
 
     def test_data_dir_is_empty_string(self, mock_out, mock_err):
@@ -236,7 +236,7 @@ class TestValidator(unittest.TestCase):
         c["postgresql"]["pg_hba"] = ""
         c["postgresql"]["data_dir"] = ""
         c["postgresql"]["bin_dir"] = ""
-        schema(c)
-        output = mock_out.getvalue()
+        errors = schema(c)
+        output = "\n".join(errors)
         self.assertEqual(['kubernetes', 'postgresql.bin_dir', 'postgresql.data_dir',
                           'postgresql.pg_hba', 'raft.bind_addr', 'raft.self_addr'], parse_output(output))