Unify Patroni and Patronictl configuration

A Patroni configuration should be enough for Patronictl
The previous dcs_api url style is still supported.

To remove duplicate magic variables a DCS_DEFAULTS was introduced.
Some behaviour has changed: If you do not specify a DCS at all
(not in configuration, not on commandline, not in environment), it will
not default to etcd://localhost:4001

More test coverage for patronictl

patroni/ctl.py

@@ -24,6 +24,10 @@ from six.moves.urllib_parse import urlparse
 CONFIG_DIR_PATH = click.get_app_dir('patroni')
 CONFIG_FILE_PATH = os.path.join(CONFIG_DIR_PATH, 'patronictl.yaml')
 LOGLEVEL = 'WARNING'
+DCS_DEFAULTS = {'zookeeper': {'port': 2181, 'template': "zookeeper:\n hosts: ['{host}:{port}']"},
+                'exhibitor': {'port': 8181, 'template': "zookeeper:\n exhibitor:\n  hosts: [{host}]\n  port: {port}"},
+                'consul': {'port': 8500, 'template': "consul:\n host: '{host}:{port}'"},
+                'etcd': {'port': 4001, 'template': "etcd:\n host: '{host}:{port}'"}}
 
 
 class PatroniCtlException(ClickException):
@@ -31,42 +35,22 @@ class PatroniCtlException(ClickException):
 
 
 def parse_dcs(dcs):
-    """
+    if dcs is None:
-    Break up the provided dcs string
+        return None
-    >>> parse_dcs('localhost') == {'etcd': {'host': 'localhost:4001'}}
-    True
-    >>> parse_dcs('localhost:8500') == {'consul': {'host': 'localhost:8500'}}
-    True
-    >>> parse_dcs('zookeeper://localhost') == {'zookeeper': {'hosts': ['localhost:2181']}}
-    True
-    >>> parse_dcs('exhibitor://localhost') == {'zookeeper': {'exhibitor': {'hosts': ['localhost'], 'port': 8181}}}
-    True
-    """
-
-    if not dcs:
-        return {}
 
     parsed = urlparse(dcs)
     scheme = parsed.scheme
     if scheme == '' and parsed.netloc == '':
         parsed = urlparse('//' + dcs)
+    port = int(parsed.port) if parsed.port else None
 
     if scheme == '':
-        default_schemes = {'2181': 'zookeeper', '8181': 'exhibitor', '8500': 'consul'}
+        scheme = ([k for k, v in DCS_DEFAULTS.items() if v['port'] == port] or ['etcd'])[0]
-        scheme = default_schemes.get(str(parsed.port), 'etcd')
+    elif scheme not in DCS_DEFAULTS:
+        raise PatroniCtlException('Unknown dcs scheme: {}'.format(scheme))
 
-    port = parsed.port
+    dcs_info = DCS_DEFAULTS[scheme]
-    if port is None:
+    return yaml.load(dcs_info['template'].format(host=parsed.hostname or 'localhost', port=port or dcs_info['port']))
-        default_ports = {'consul': 8500, 'zookeeper': 2181, 'exhibitor': 8181}
-        port = default_ports.get(str(scheme), 4001)
-
-    config = {'host': '{0}:{1}'.format(parsed.hostname, port)}
-    if scheme == 'exhibitor':
-        config = {scheme: {'port': int(port), 'hosts': [str(parsed.hostname)]}}
-        scheme = 'zookeeper'
-    elif scheme == 'zookeeper':
-        config['hosts'] = [config.pop('host')]
-    return {scheme: config}
 
 
 def load_config(path, dcs):
@@ -78,10 +62,7 @@ def load_config(path, dcs):
     except (IOError, yaml.YAMLError):
         logging.exception('Could not load configuration file')
 
-    if dcs:
+    config.update(parse_dcs(dcs) or parse_dcs(config.get('dcs_api')) or {})
-        config['dcs'] = parse_dcs(dcs)
-    else:
-        config['dcs'] = parse_dcs(config.get('dcs_api'))
 
     return config
 
@@ -112,10 +93,10 @@ def ctl(ctx):
 
 
 def get_dcs(config, scope):
-    dcs_config = config.get('dcs', {})
+    for k in set(DCS_DEFAULTS.keys()) & set(config.keys()):
-    dcs_config[list(dcs_config.keys())[0]]['scope'] = scope
+        config[k].setdefault('scope', scope)
     try:
-        return Patroni.get_dcs(scope, dcs_config)
+        return Patroni.get_dcs(scope, config)
     except PatroniException as e:
         raise PatroniCtlException(str(e))
 

tests/test_ctl.py

@@ -6,7 +6,7 @@ import unittest
 
 from click.testing import CliRunner
 from mock import patch, Mock
-from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, \
+from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, parse_dcs, \
     wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
 from patroni.etcd import Etcd, Client
 from psycopg2 import OperationalError
@@ -40,6 +40,8 @@ def test_rw_config():
     load_config(CONFIG_FILE_PATH, None)
     load_config(CONFIG_FILE_PATH, '0.0.0.0')
 
+    store_config({'dcs_api': None}, CONFIG_FILE_PATH)
+    load_config(CONFIG_FILE_PATH, None)
 
 @patch('patroni.ctl.load_config', Mock(return_value={'dcs': {'etcd': {'host': 'localhost:4001'}}}))
 class TestCtl(unittest.TestCase):
@@ -62,6 +64,17 @@ class TestCtl(unittest.TestCase):
 
         self.assertIsNotNone(get_cursor(get_cluster_initialized_with_leader(), role='any'))
 
+    def test_parse_dcs(self):
+        assert parse_dcs(None) is None
+        assert parse_dcs('localhost') == {'etcd': {'host': 'localhost:4001'}}
+        assert parse_dcs('') == {'etcd': {'host': 'localhost:4001'}}
+        assert parse_dcs('localhost:8500') == {'consul': {'host': 'localhost:8500'}}
+        assert parse_dcs('zookeeper://localhost') == {'zookeeper': {'hosts': ['localhost:2181']}}
+        assert parse_dcs('exhibitor://dummy') == {'zookeeper': {'exhibitor': {'hosts': ['dummy'], 'port': 8181}}}
+        assert parse_dcs('consul://localhost') == {'consul': {'host': 'localhost:8500'}}
+        self.assertRaises(PatroniCtlException, parse_dcs, 'invalid://test')
+
+
     def test_output_members(self):
         cluster = get_cluster_initialized_with_leader()
         self.assertIsNone(output_members(cluster, name='abc', fmt='pretty'))
@@ -72,67 +85,70 @@ class TestCtl(unittest.TestCase):
     @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
     @patch('patroni.ctl.post_patroni', Mock(return_value=MockResponse()))
     def test_failover(self):
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+        with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)):
-        assert 'leader' in result.output
-
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n2100-01-01T12:23:00\ny''')
-        assert result.exit_code == 0
-
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n2030-01-01T12:23:00\ny''')
-        assert result.exit_code == 0
-
-        # Aborting failover,as we anser NO to the confirmation
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\nN''')
-        assert result.exit_code == 1
-
-        # Target and source are equal
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nleader\n\ny''')
-        assert result.exit_code == 1
-
-        # Reality is not part of this cluster
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nReality\n\ny''')
-        assert result.exit_code == 1
-
-        result = self.runner.invoke(ctl, ['failover', 'dummy', '--force'])
-        assert 'Member' in result.output
-
-        result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00+01:00'])
-        assert result.exit_code == 0
-
-        # Invalid timestamp
-        result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', 'invalid'])
-        assert result.exit_code != 0
-
-        # Invalid timestamp
-        result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2115-02-30T12:00:00+01:00'])
-        assert result.exit_code != 0
-
-        # Specifying wrong leader
-        result = self.runner.invoke(ctl, ['failover', 'dummy'], input='dummy')
-        assert result.exit_code == 1
-
-        with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())):
-            # No members available
             result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+            assert 'leader' in result.output
+
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n2100-01-01T12:23:00\ny''')
+            assert result.exit_code == 0
+
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n2030-01-01T12:23:00\ny''')
+            assert result.exit_code == 0
+
+            # Aborting failover,as we anser NO to the confirmation
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\nN''')
             assert result.exit_code == 1
 
-        with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())):
+            # Target and source are equal
-            # No master available
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nleader\n\ny''')
-            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
             assert result.exit_code == 1
 
-        with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception)):
+            # Reality is not part of this cluster
-            # Non-responding patroni
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nReality\n\ny''')
-            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+            assert result.exit_code == 1
-            assert 'falling back to DCS' in result.output
 
-        with patch('patroni.ctl.post_patroni') as mocked:
+            result = self.runner.invoke(ctl, ['failover', 'dummy', '--force'])
-            mocked.return_value.status_code = 500
+            assert 'Member' in result.output
-            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+
-            assert 'Failover failed' in result.output
+            result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2015-01-01T12:00:00+01:00'])
+            assert result.exit_code == 0
+
+            # Invalid timestamp
+            result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', 'invalid'])
+            assert result.exit_code != 0
+
+            # Invalid timestamp
+            result = self.runner.invoke(ctl, ['failover', 'dummy', '--force', '--scheduled', '2115-02-30T12:00:00+01:00'])
+            assert result.exit_code != 0
+
+            # Specifying wrong leader
+            result = self.runner.invoke(ctl, ['failover', 'dummy'], input='dummy')
+            assert result.exit_code == 1
+
+            with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())):
+                # No members available
+                result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+                assert result.exit_code == 1
+
+            with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())):
+                # No master available
+                result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+                assert result.exit_code == 1
+
+            with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception)):
+                # Non-responding patroni
+                result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+                assert 'falling back to DCS' in result.output
+
+            with patch('patroni.ctl.post_patroni') as mocked:
+                mocked.return_value.status_code = 500
+                result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''')
+                assert 'Failover failed' in result.output
 
     def test_get_dcs(self):
-        self.assertRaises(PatroniCtlException, get_dcs, {'dcs': {'dummy': {}}}, 'dummy')
+        self.assertRaises(PatroniCtlException, get_dcs, {'dummy': {}}, 'dummy')
+        with patch('patroni.Patroni.get_dcs', Mock(return_value=self.e)):
+            assert get_dcs({'etcd': {'host':'none'}}, 'dummy').client_path('') == '/service/test/'
 
     @patch('psycopg2.connect', psycopg2_connect)
     @patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None)))
@@ -206,24 +222,26 @@ class TestCtl(unittest.TestCase):
     @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
     @patch('requests.post', requests_get)
     def test_restart_reinit(self):
-        result = self.runner.invoke(ctl, ['restart', 'alpha'], input='y')
+        with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)):
-        assert result.exit_code == 0
-
-        result = self.runner.invoke(ctl, ['reinit', 'alpha'], input='y')
-        assert result.exit_code == 1
-
-        # Aborted restart
-        result = self.runner.invoke(ctl, ['restart', 'alpha'], input='N')
-        assert result.exit_code == 1
-
-        # Not a member
-        result = self.runner.invoke(ctl, ['restart', 'alpha', 'dummy', '--any'], input='y')
-        assert result.exit_code == 1
-
-        with patch('requests.post', Mock(return_value=MockResponse())):
             result = self.runner.invoke(ctl, ['restart', 'alpha'], input='y')
+            assert 'restart failed for' in result.output
             assert result.exit_code == 0
 
+            result = self.runner.invoke(ctl, ['reinit', 'alpha'], input='y')
+            assert result.exit_code == 1
+
+            # Aborted restart
+            result = self.runner.invoke(ctl, ['restart', 'alpha'], input='N')
+            assert result.exit_code == 1
+
+            # Not a member
+            result = self.runner.invoke(ctl, ['restart', 'alpha', 'dummy', '--any'], input='y')
+            assert result.exit_code == 1
+
+            with patch('requests.post', Mock(return_value=MockResponse())):
+                result = self.runner.invoke(ctl, ['restart', 'alpha'], input='y')
+                assert result.exit_code == 0
+
     @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
     @patch.object(etcd.Client, 'delete', Mock(side_effect=etcd.EtcdException))
     def test_remove(self):
@@ -286,8 +304,10 @@ class TestCtl(unittest.TestCase):
     @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
     @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
     def test_members(self):
-        result = self.runner.invoke(members, ['alpha'])
+        with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)):
-        assert result.exit_code == 0
+            result = self.runner.invoke(members, ['alpha'])
+            assert '127.0.0.1' in result.output
+            assert result.exit_code == 0
 
     def test_configure(self):
         result = self.runner.invoke(configure, ['--dcs', 'abc', '-c', 'dummy', '-n', 'bla'])