From 59dae9b1bb045fb9154663f06358308363e506f6 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 28 Jul 2020 08:37:04 +0200 Subject: [PATCH] A few little bug-fixes (#1623) 1. Put the `*` character into pgpass if the actual value is empty 2. Re-raise fatal exception from the HA loop (we need to exit if for example cluster initialization failed) Close https://github.com/zalando/patroni/issues/1617 --- patroni/dcs/__init__.py | 4 ++-- patroni/exceptions.py | 4 ++++ patroni/ha.py | 6 ++++-- patroni/postgresql/config.py | 8 ++++---- tests/test_ha.py | 8 +++++--- 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index cda9d43d..16f79e99 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -13,7 +13,7 @@ import time from collections import defaultdict, namedtuple from copy import deepcopy -from patroni.exceptions import PatroniException +from patroni.exceptions import PatroniFatalException from patroni.utils import parse_bool, uri from random import randint from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl @@ -102,7 +102,7 @@ def get_dcs(config): and inspect.isclass(item) and issubclass(item, AbstractDCS)) except ImportError: logger.info('Failed to import %s', module_name) - raise PatroniException("""Can not find suitable configuration of distributed configuration store + raise PatroniFatalException("""Can not find suitable configuration of distributed configuration store Available implementations: """ + ', '.join(sorted(set(available_implementations)))) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 43980721..21df1f7b 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -13,6 +13,10 @@ class PatroniException(Exception): return repr(self.value) +class PatroniFatalException(PatroniException): + pass + + class PostgresException(PatroniException): pass diff --git a/patroni/ha.py b/patroni/ha.py index 1759c93f..19b15f5c 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -10,7 +10,7 @@ import uuid from collections import namedtuple from multiprocessing.pool import ThreadPool from patroni.async_executor import AsyncExecutor, CriticalTask -from patroni.exceptions import DCSError, PostgresConnectionException, PatroniException +from patroni.exceptions import DCSError, PostgresConnectionException, PatroniFatalException from patroni.postgresql import ACTION_ON_START, ACTION_ON_ROLE_CHANGE from patroni.postgresql.misc import postgres_version_to_int from patroni.postgresql.rewind import Rewind @@ -1176,7 +1176,7 @@ class Ha(object): self.dcs.cancel_initialization() self.state_handler.stop('immediate', stop_timeout=self.patroni.config['retry_timeout']) self.state_handler.move_data_directory() - raise PatroniException('Failed to bootstrap cluster') + raise PatroniFatalException('Failed to bootstrap cluster') def post_bootstrap(self): # bootstrap has failed if postgres is not running @@ -1384,6 +1384,8 @@ class Ha(object): try: info = self._run_cycle() return (self.is_paused() and 'PAUSE: ' or '') + info + except PatroniFatalException: + raise except Exception: logger.exception('Unexpected exception') return 'Unexpected exception raised, please report it as a BUG' diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 0225a9bd..156294f3 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -6,7 +6,7 @@ import socket import stat import time -from patroni.exceptions import PatroniException +from patroni.exceptions import PatroniFatalException from six.moves.urllib_parse import urlparse, parse_qsl, unquote from urllib3.response import HTTPHeaderDict @@ -339,8 +339,8 @@ class ConfigHandler(object): self._auto_conf_mtime = None self._pgpass = os.path.abspath(config.get('pgpass') or os.path.join(os.path.expanduser('~'), 'pgpass')) if os.path.exists(self._pgpass) and not os.path.isfile(self._pgpass): - raise PatroniException("'{}' exists and it's not a file, check your `postgresql.pgpass` configuration" - .format(self._pgpass)) + raise PatroniFatalException("'{0}' exists and it's not a file, check your `postgresql.pgpass` configuration" + .format(self._pgpass)) self._passfile = None self._passfile_mtime = None self._synchronous_standby_names = None @@ -748,7 +748,7 @@ class ConfigHandler(object): def escape(value): return re.sub(r'([:\\])', r'\\\1', str(value)) - record = {n: escape(record.get(n, '*')) for n in ('host', 'port', 'user', 'password')} + record = {n: escape(record.get(n) or '*') for n in ('host', 'port', 'user', 'password')} return '{host}:{port}:*:{user}:{password}'.format(**record) def write_pgpass(self, record): diff --git a/tests/test_ha.py b/tests/test_ha.py index 86a3c6b6..dbd33f7f 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -7,7 +7,7 @@ from mock import Mock, MagicMock, PropertyMock, patch, mock_open from patroni.config import Config from patroni.dcs import Cluster, ClusterConfig, Failover, Leader, Member, get_dcs, SyncState, TimelineHistory from patroni.dcs.etcd import Client -from patroni.exceptions import DCSError, PostgresConnectionException, PatroniException +from patroni.exceptions import DCSError, PostgresConnectionException, PatroniFatalException from patroni.ha import Ha, _MemberStatus from patroni.postgresql import Postgresql from patroni.postgresql.bootstrap import Bootstrap @@ -422,7 +422,7 @@ class TestHa(PostgresInit): self.e.initialize = true self.ha.bootstrap() self.p.is_running = false - self.assertRaises(PatroniException, self.ha.post_bootstrap) + self.assertRaises(PatroniFatalException, self.ha.post_bootstrap) def test_bootstrap_release_initialize_key_on_watchdog_failure(self): self.ha.cluster = get_cluster_not_initialized_without_leader() @@ -432,7 +432,7 @@ class TestHa(PostgresInit): self.p.is_leader = true with patch.object(Watchdog, 'activate', Mock(return_value=False)): self.assertEqual(self.ha.post_bootstrap(), 'running post_bootstrap') - self.assertRaises(PatroniException, self.ha.post_bootstrap) + self.assertRaises(PatroniFatalException, self.ha.post_bootstrap) @patch('psycopg2.connect', psycopg2_connect) def test_reinitialize(self): @@ -1088,3 +1088,5 @@ class TestHa(PostgresInit): def test_run_cycle(self): self.ha.dcs.touch_member = Mock(side_effect=DCSError('foo')) self.assertEqual(self.ha.run_cycle(), 'Unexpected exception raised, please report it as a BUG') + self.ha.dcs.touch_member = Mock(side_effect=PatroniFatalException('foo')) + self.assertRaises(PatroniFatalException, self.ha.run_cycle)