diff --git a/patroni/ctl.py b/patroni/ctl.py index d32440dc..5fc14336 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -561,9 +561,10 @@ def get_cursor(obj: Dict[str, Any], cluster: Cluster, group: Optional[int], conn from . import psycopg conn = psycopg.connect(**params) cursor = conn.cursor() - # If we want ``any`` node we are fine to return the cursor + # If we want ``any`` node we are fine to return the cursor. ``None`` is similar to ``any`` at this point, as it's + # been dealt with through :func:`get_any_member`. # If we want the Patroni leader node, :func:`get_any_member` already checks that for us - if role in ('any', 'leader'): + if role in (None, 'any', 'leader'): return cursor # If we want something other than ``any`` or ``leader``, then we do not rely only on the DCS information about @@ -857,9 +858,11 @@ def query_member(obj: Dict[str, Any], cluster: Cluster, group: Optional[int], if cursor is None: if member is not None: - message = 'No connection to member {0} is available'.format(member) + message = f'No connection to member {member} is available' + elif role is not None: + message = f'No connection to role {role} is available' else: - message = 'No connection to role={0} is available'.format(role) + message = 'No connection is available' logging.debug(message) return [[timestamp(0), message]], None diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 7cf542b5..85691812 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -75,6 +75,21 @@ class TestCtl(unittest.TestCase): self.assertIsNotNone(get_cursor({}, get_cluster_initialized_with_leader(), None, {'dbname': 'foo'}, role='any')) + # Mutually exclusive options + with self.assertRaises(PatroniCtlException) as e: + get_cursor({}, get_cluster_initialized_with_leader(), None, {'dbname': 'foo'}, member_name='other', + role='replica') + + self.assertEqual(str(e.exception), '--role and --member are mutually exclusive options') + + # Invalid member provided + self.assertIsNone(get_cursor({}, get_cluster_initialized_with_leader(), None, {'dbname': 'foo'}, + member_name='invalid')) + + # Valid member provided + self.assertIsNotNone(get_cursor({}, get_cluster_initialized_with_leader(), None, {'dbname': 'foo'}, + member_name='other')) + def test_parse_dcs(self): assert parse_dcs(None) is None assert parse_dcs('localhost') == {'etcd': {'host': 'localhost:2379'}} @@ -278,11 +293,17 @@ class TestCtl(unittest.TestCase): rows = query_member({}, None, None, None, None, 'replica', 'SELECT pg_catalog.pg_is_in_recovery()', {}) with patch('patroni.ctl.get_cursor', Mock(return_value=None)): + # No role nor member given -- generic message rows = query_member({}, None, None, None, None, None, 'SELECT pg_catalog.pg_is_in_recovery()', {}) - self.assertTrue('No connection to' in str(rows)) + self.assertTrue('No connection is available' in str(rows)) - rows = query_member({}, None, None, None, 'foo', 'replica', 'SELECT pg_catalog.pg_is_in_recovery()', {}) - self.assertTrue('No connection to' in str(rows)) + # Member given -- message pointing to member + rows = query_member({}, None, None, None, 'foo', None, 'SELECT pg_catalog.pg_is_in_recovery()', {}) + self.assertTrue('No connection to member foo' in str(rows)) + + # Role given -- message pointing to role + rows = query_member({}, None, None, None, None, 'replica', 'SELECT pg_catalog.pg_is_in_recovery()', {}) + self.assertTrue('No connection to role replica' in str(rows)) with patch('patroni.ctl.get_cursor', Mock(side_effect=OperationalError('bla'))): rows = query_member({}, None, None, None, None, 'replica', 'SELECT pg_catalog.pg_is_in_recovery()', {})