Further address code reviews.

- Fix the issue in ctl that would result in setting the  listen_address to True.
- Minor stylistic issues.
- Add unit-tests.

patroni/ctl.py

@@ -649,7 +649,7 @@ def configure(config_file, dcs, namespace):
 
 
 def touch_member(config, dcs):
-    ''' Rip of the ha.touch_member without inter-class dependencies '''
+    ''' Rip-off of the ha.touch_member without inter-class dependencies '''
     p = Postgresql(config['postgresql'])
     p.set_state('running')
     p.set_role('master')
@@ -674,7 +674,8 @@ def set_defaults(config, cluster_name):
     config['postgresql'].setdefault('scope', cluster_name)
     config['postgresql'].setdefault('listen', '127.0.0.1')
     config['postgresql']['authentication'] = {'replication': None}
-    config['restapi']['listen'] = ':' in config['restapi'].get('listen', ".") or '127.0.0.1:5432'
+    config['restapi']['listen'] = (config['restapi']['listen']
+                                   if ':' in config['restapi'].get('listen', ".") else '127.0.0.1:5432')
 
 
 @ctl.command('scaffold', help='Create a structure for the cluster in DCS')
@@ -683,9 +684,8 @@ def set_defaults(config, cluster_name):
 @option_config_file
 @option_dcs
 def scaffold(cluster_name, config_file, dcs, sysid):
-    logging.debug("config_file = %s, cluster_name = %s, dcs = %s, sysid = %s", config_file, cluster_name, dcs, sysid)
     config, dcs, cluster = ctl_load_config(cluster_name, config_file, dcs)
-    if cluster and cluster.initialize:
+    if cluster and cluster.initialize is not None:
         raise PatroniCtlException("This cluster is already initialized")
 
     if not dcs.initialize(create_new=True, sysid=sysid):
@@ -700,6 +700,5 @@ def scaffold(cluster_name, config_file, dcs, sysid):
             # we did initialize this cluster, but failed to write the leader or member keys, wipe it down completely.
             raise PatroniCtlException("Unable to install permanent leader for cluster {0}".format(cluster_name))
     except:
-        dcs.delete_cluster()
         raise
     click.echo("Cluster {0} has been created successfully".format(cluster_name))

patroni/dcs/consul.py

@@ -191,7 +191,7 @@ class Consul(AbstractDCS):
             return True
 
         try:
-            args = {} if kwargs.get('permanent') else {'acquire': self._session}
+            args = {} if kwargs.get('permanent', False) else {'acquire': self._session}
             self._client.kv.put(self.member_path, data, **args)
             self._my_member_data = data
             return True

tests/test_ctl.py

@@ -4,14 +4,20 @@ import requests
 import sys
 import unittest
 
+from patroni.api import RestApiServer
+from six.moves import BaseHTTPServer
+
 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, parse_dcs, \
-    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException
+    wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, touch_member, set_defaults,\
+    PatroniCtlException
+
+from patroni.exceptions import DCSError
 from psycopg2 import OperationalError
 from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse
 from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \
-    get_cluster_initialized_with_only_leader
+    get_cluster_initialized_with_only_leader, get_cluster_not_initialized_without_leader
 from test_postgresql import MockConnect, psycopg2_connect
 
 CONFIG_FILE_PATH = './test-ctl.yaml'
@@ -28,7 +34,9 @@ def test_rw_config():
         os.rmdir(CONFIG_FILE_PATH)
 
 
-@patch('patroni.ctl.load_config', Mock(return_value={'restapi': {'auth': 'u:p'}, 'etcd': {'host': 'localhost:4001'}}))
+@patch('patroni.ctl.load_config', Mock(return_value={'postgresql': {'data_dir': '.', 'parameters': {}, 'retry_timeout': 5},
+                                                     'restapi': {'auth': 'u:p', 'listen': ''},
+                                                     'etcd': {'host': 'localhost:4001'}}))
 class TestCtl(unittest.TestCase):
 
     @patch('socket.getaddrinfo', socket_getaddrinfo)
@@ -292,3 +300,31 @@ class TestCtl(unittest.TestCase):
     def test_configure(self):
         result = self.runner.invoke(configure, ['--dcs', 'abc', '-c', 'dummy', '-n', 'bla'])
         assert result.exit_code == 0
+
+    @patch('patroni.ctl.get_dcs')
+    @patch.object(BaseHTTPServer.HTTPServer, '__init__', Mock())
+    def test_scaffold(self, mock_get_dcs):
+        mock_get_dcs.return_value = self.e
+        mock_get_dcs.return_value.get_cluster = get_cluster_not_initialized_without_leader
+        mock_get_dcs.return_value.initialize = Mock(return_value=True)
+        mock_get_dcs.return_value.touch_member = Mock(return_value=True)
+        mock_get_dcs.return_value.attempt_to_acquire_leader = Mock(return_value=True)
+
+        RestApiServer._BaseServer__is_shut_down = Mock()
+        RestApiServer._BaseServer__shutdown_request = True
+        RestApiServer.socket = 0
+
+        with patch.object(self.e, 'initialize', return_value=False):
+            result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+            assert result.exit_code == 1
+
+        with patch.object(mock_get_dcs.return_value, 'touch_member', Mock(side_effect=DCSError("foo"))):
+            result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+            assert result.exception
+
+        result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+        assert result.exit_code == 0
+
+        mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader
+        result = self.runner.invoke(ctl, ['scaffold', 'alpha'])
+        assert result.exit_code == 1