From 7659ccd50bb3f07a65d3b65dd5b9af864353ed70 Mon Sep 17 00:00:00 2001 From: Waynerv Date: Thu, 15 Aug 2024 22:39:56 +0800 Subject: [PATCH] Fix request URL in failsafe handling logs (#3126) --- patroni/dcs/__init__.py | 13 +++++++++++++ patroni/ha.py | 8 +++++--- patroni/request.py | 6 +----- tests/test_ha.py | 17 +++++++++++++++++ 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index c41b7904..3fd71fce 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -260,6 +260,19 @@ class Member(Tags, NamedTuple('Member', ret['user'] = ret.pop('username') return ret + def get_endpoint_url(self, endpoint: Optional[str] = None) -> str: + """Get URL from member :attr:`~Member.api_url` and endpoint. + + :param endpoint: URL path of REST API. + + :returns: full URL for this REST API. + """ + url = self.api_url or '' + if endpoint: + scheme, netloc, _, _, _, _ = urlparse(url) + url = urlunparse((scheme, netloc, endpoint, '', '', '')) + return url + @property def api_url(self) -> Optional[str]: """The ``api_url`` value from :attr:`~Member.data` if defined.""" diff --git a/patroni/ha.py b/patroni/ha.py index 72d52b32..f3d46509 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -1174,15 +1174,17 @@ class Ha(object): :returns: a :class:`_FailsafeResponse` object. """ + endpoint = 'failsafe' + url = member.get_endpoint_url(endpoint) try: - response = self.patroni.request(member, 'post', 'failsafe', data, timeout=2, retries=1) + response = self.patroni.request(member, 'post', endpoint, data, timeout=2, retries=1) response_data = response.data.decode('utf-8') - logger.info('Got response from %s %s: %s', member.name, member.api_url, response_data) + logger.info('Got response from %s %s: %s', member.name, url, response_data) accepted = response.status == 200 and response_data == 'Accepted' # member may return its current received/replayed LSN in the "lsn" header. return _FailsafeResponse(member.name, accepted, parse_int(response.headers.get('lsn'))) except Exception as e: - logger.warning("Request failed to %s: POST %s (%s)", member.name, member.api_url, e) + logger.warning("Request failed to %s: POST %s (%s)", member.name, url, e) return _FailsafeResponse(member.name, False, None) def check_failsafe_topology(self) -> bool: diff --git a/patroni/request.py b/patroni/request.py index 2bba134b..be087315 100644 --- a/patroni/request.py +++ b/patroni/request.py @@ -2,7 +2,6 @@ import json from typing import Any, Dict, Optional, Union -from urllib.parse import urlparse, urlunparse import urllib3 @@ -164,10 +163,7 @@ class PatroniRequest(object): :returns: the response returned upon request. """ - url = member.api_url or '' - if endpoint: - scheme, netloc, _, _, _, _ = urlparse(url) - url = urlunparse((scheme, netloc, endpoint, '', '', '')) + url = member.get_endpoint_url(endpoint) return self.request(method, url, data, **kwargs) diff --git a/tests/test_ha.py b/tests/test_ha.py index d6f04358..0d218c38 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -577,6 +577,23 @@ class TestHa(PostgresInit): self.p.set_role('primary') self.assertEqual(self.ha.update_failsafe({}), 'Running as a leader') + def test_call_failsafe_member(self): + member = Member(0, 'test', 1, {'api_url': 'http://localhost:8011/patroni'}) + self.ha.patroni.request = Mock() + self.ha.patroni.request.return_value.data = b'Accepted' + self.ha.patroni.request.return_value.status = 200 + with patch('patroni.ha.logger.info') as mock_logger: + ret = self.ha.call_failsafe_member({}, member) + self.assertEqual(mock_logger.call_args_list[0][0], ('Got response from %s %s: %s', 'test', 'http://localhost:8011/failsafe', 'Accepted')) + self.assertTrue(ret.accepted) + + e = Exception('request failed') + self.ha.patroni.request.side_effect = e + with patch('patroni.ha.logger.warning') as mock_logger: + ret = self.ha.call_failsafe_member({}, member) + self.assertEqual(mock_logger.call_args_list[0][0], ('Request failed to %s: POST %s (%s)', 'test', 'http://localhost:8011/failsafe', e)) + self.assertFalse(ret.accepted) + @patch('time.sleep', Mock()) def test_bootstrap_from_another_member(self): self.ha.cluster = get_cluster_initialized_with_leader()