From eeb8f1b694c2bec08550ab488977baa60d941bfb Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 8 Aug 2016 12:21:01 +0200 Subject: [PATCH] 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 | 9 ++++----- patroni/dcs/consul.py | 2 +- tests/test_ctl.py | 42 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index ba8eca01..cc0bca7e 100644 --- a/patroni/ctl.py +++ b/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)) diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py index b2e15f76..36a93536 100644 --- a/patroni/dcs/consul.py +++ b/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 diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 42929a43..de07ff44 100644 --- a/tests/test_ctl.py +++ b/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