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

@@ -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))))
 
 

patroni/exceptions.py

@@ -13,6 +13,10 @@ class PatroniException(Exception):
         return repr(self.value)
 
 
+class PatroniFatalException(PatroniException):
+    pass
+
+
 class PostgresException(PatroniException):
     pass
 

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'

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

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)