Repository: zalando/patroni
Pull request #2883 (2023-09-25 06:54:10)

Bugfix for GUC's values with units
Despite being validated by `IntValidator` some GUC's couldn't be casted directly to `int` because they include suffix. Example: `128MB`.

Close https://github.com/zalando/patroni/issues/2879

coveralls: ## Pull Request Test Coverage Report for [Build 6295921612](https://coveralls.io/builds/62864933)

* **2** of **2**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **99.863%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/62864933/badge)](https://coveralls.io/builds/62864933) |
| :-- | --: |
| Change from base [Build 6250333541](https://coveralls.io/builds/62784539): |  0.0% |
| Covered Lines: | 13103 |
| Relevant Lines: | 13121 |

---
##### 💛  - [Coveralls](https://coveralls.io)

hughcapet: 👍 

Patch:
diff --git a/patroni/__main__.py b/patroni/__main__.py
index 2b318a67..7d56172b 100644
--- a/patroni/__main__.py
+++ b/patroni/__main__.py
@@ -106,6 +106,8 @@ class Patroni(AbstractPatroniDaemon, Tags):
 
     def ensure_unique_name(self) -> None:
         """A helper method to prevent splitbrain from operator naming error."""
+        from urllib.parse import urlparse
+        from urllib3.connection import HTTPConnection
         from patroni.dcs import Member
 
         cluster = self.dcs.get_cluster()
@@ -115,9 +117,12 @@ class Patroni(AbstractPatroniDaemon, Tags):
         if not isinstance(member, Member):
             return
         try:
-            _ = self.request(member, endpoint="/liveness", timeout=3)
-            logger.fatal("Can't start; there is already a node named '%s' running", self.config['name'])
-            sys.exit(1)
+            parts = urlparse(member.api_url)
+            if isinstance(parts.hostname, str):
+                connection = HTTPConnection(parts.hostname, port=parts.port or 80, timeout=3)
+                connection.connect()
+                logger.fatal("Can't start; there is already a node named '%s' running", self.config['name'])
+                sys.exit(1)
         except Exception:
             return
 
diff --git a/patroni/config.py b/patroni/config.py
index 1650934d..fee2147a 100644
--- a/patroni/config.py
+++ b/patroni/config.py
@@ -490,7 +490,8 @@ class Config(object):
             elif not is_local:
                 validator = ConfigHandler.CMDLINE_OPTIONS[name][1]
                 if validator(value):
-                    pg_params[name] = int(value) if isinstance(validator, IntValidator) else value
+                    int_val = parse_int(value) if isinstance(validator, IntValidator) else None
+                    pg_params[name] = int_val if isinstance(int_val, int) else value
                 else:
                     logger.warning("postgresql parameter %s=%s failed validation, defaulting to %s",
                                    name, value, ConfigHandler.CMDLINE_OPTIONS[name][0])


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index bd8d0a90..f0a780bc 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -155,6 +155,7 @@ class TestConfig(unittest.TestCase):
         expected_params = {
             'f.oo': 'bar',  # not in ConfigHandler.CMDLINE_OPTIONS
             'max_connections': 100,  # IntValidator
+            'wal_keep_size': '128MB',  # IntValidator
             'wal_level': 'hot_standby',  # EnumValidator
         }
         input_params = deepcopy(expected_params)
diff --git a/tests/test_patroni.py b/tests/test_patroni.py
index 0385731c..df59677d 100644
--- a/tests/test_patroni.py
+++ b/tests/test_patroni.py
@@ -40,7 +40,7 @@ class MockFrozenImporter(object):
 @patch('time.sleep', Mock())
 @patch('subprocess.call', Mock(return_value=0))
 @patch('patroni.psycopg.connect', psycopg_connect)
-@patch('urllib3.PoolManager.request', Mock(side_effect=Exception))
+@patch('urllib3.connection.HTTPConnection.connect', Mock(side_effect=Exception))
 @patch.object(ConfigHandler, 'append_pg_hba', Mock())
 @patch.object(ConfigHandler, 'write_postgresql_conf', Mock())
 @patch.object(ConfigHandler, 'write_recovery_conf', Mock())
@@ -64,7 +64,7 @@ class TestPatroni(unittest.TestCase):
             self.assertRaises(SystemExit, _main)
 
     @patch('pkgutil.iter_importers', Mock(return_value=[MockFrozenImporter()]))
-    @patch('urllib3.PoolManager.request', Mock(side_effect=Exception))
+    @patch('urllib3.connection.HTTPConnection.connect', Mock(side_effect=Exception))
     @patch('sys.frozen', Mock(return_value=True), create=True)
     @patch.object(HTTPServer, '__init__', Mock())
     @patch.object(etcd.Client, 'read', etcd_read)
@@ -108,6 +108,7 @@ class TestPatroni(unittest.TestCase):
     @patch('os.getpid')
     @patch('multiprocessing.Process')
     @patch('patroni.__main__.patroni_main', Mock())
+    @patch('sys.argv', ['patroni.py', 'postgres0.yml'])
     def test_patroni_main(self, mock_process, mock_getpid):
         mock_getpid.return_value = 2
         _main()
@@ -233,8 +234,8 @@ class TestPatroni(unittest.TestCase):
         )
         with patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=bad_cluster)):
             # If the api of the running node cannot be reached, this implies unique name
-            with patch.object(self.p, 'request', Mock(side_effect=ConnectionError)):
+            with patch('urllib3.connection.HTTPConnection.connect', Mock(side_effect=ConnectionError)):
                 self.assertIsNone(self.p.ensure_unique_name())
             # Only if the api of the running node is reachable do we throw an error
-            with patch.object(self.p, 'request', Mock()):
+            with patch('urllib3.connection.HTTPConnection.connect', Mock()):
                 self.assertRaises(SystemExit, self.p.ensure_unique_name)