Repository: zalando/patroni
Pull request #2878 (2023-09-22 09:52:14)

Silence annoying warnings when checking for node uniqueness
WARNING messages are produced by `urllib3` if Patroni is quickly restarted.
Instead we will check that the node is listen on a given port. This fact is actually enough to detect names clashes, while HTTP request could raise an exception is a few other cases, what might case false negatives.

Close https://github.com/zalando/patroni/issues/2881

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

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

---



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

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

CyberDem0n: @barthisrael actually, after thinking a bit more about it I decided to change the check so it fails as soon as it detects that the port is open.
rishiraj88: :+1: 

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
 


Test patch:
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)