From cd6537c4d377f1c64157bd6f92ecef3d366e1048 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 18 Apr 2016 17:36:30 +0200 Subject: [PATCH 1/3] Refactor patroni/ctl.py Until now get_dcs method was doing absolutely the same job as method from Patroni class. In addition to that I did small refactoring and clean up of unit tests --- patroni/__init__.py | 9 ++--- patroni/ctl.py | 69 ++++++++++++++++++----------------- patroni/exceptions.py | 7 ---- tests/test_ctl.py | 84 +++++++++++++------------------------------ tests/test_patroni.py | 4 +-- 5 files changed, 68 insertions(+), 105 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 88b5e312..916c0d3d 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -5,12 +5,11 @@ import time import yaml from patroni.api import RestApiServer -from patroni.etcd import Etcd +from patroni.exceptions import PatroniException from patroni.ha import Ha from patroni.postgresql import Postgresql from patroni.utils import reap_children, set_ignore_sigterm, setup_signal_handlers -from patroni.zookeeper import ZooKeeper -from .version import __version__ +from patroni.version import __version__ logger = logging.getLogger(__name__) @@ -40,10 +39,12 @@ class Patroni(object): @staticmethod def get_dcs(name, config): if 'etcd' in config: + from patroni.etcd import Etcd return Etcd(name, config['etcd']) if 'zookeeper' in config: + from patroni.zookeeper import ZooKeeper return ZooKeeper(name, config['zookeeper']) - raise Exception('Can not find suitable configuration of distributed configuration store') + raise PatroniException('Can not find suitable configuration of distributed configuration store') def schedule_next_run(self): self.next_run += self.nap_time diff --git a/patroni/ctl.py b/patroni/ctl.py index cba03a48..b6f7eb0a 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -3,39 +3,44 @@ Patroni Control ''' import click -import os -import yaml +import datetime +import dateutil import json -import time +import logging +import os import psycopg2 import random import requests -import datetime +import time +import tzlocal +import yaml + +from click import ClickException +from patroni import Patroni, PatroniException +from patroni.postgresql import parseurl from prettytable import PrettyTable from six.moves.urllib_parse import urlparse -import logging -import dateutil -import tzlocal - -from .etcd import Etcd -from .zookeeper import ZooKeeper -from .exceptions import PatroniCtlException -from .postgresql import parseurl CONFIG_DIR_PATH = click.get_app_dir('patroni') CONFIG_FILE_PATH = os.path.join(CONFIG_DIR_PATH, 'patronictl.yaml') LOGLEVEL = 'WARNING' +class PatroniCtlException(ClickException): + pass + + def parse_dcs(dcs): """ Break up the provided dcs string - >>> parse_dcs('localhost') == {'scheme': 'etcd', 'hostname': 'localhost', 'port': 4001} - True - >>> parse_dcs('localhost:8500') == {'scheme': 'consul', 'hostname': 'localhost', 'port': 8500} - True - >>> parse_dcs('zookeeper://localhost') == {'scheme': 'zookeeper', 'hostname': 'localhost', 'port': 2181} - True + >>> parse_dcs('localhost') + {'etcd': {'host': 'localhost:4001'}} + >>> parse_dcs('localhost:8500') + {'consul': {'host': 'localhost:8500'}} + >>> parse_dcs('zookeeper://localhost') + {'zookeeper': {'hosts': ['localhost:2181']}} + >>> parse_dcs('exhibitor://localhost') + {'zookeeper': {'exhibitor': {'hosts': ['localhost'], 'port': 8181}}} """ if not dcs: @@ -55,7 +60,13 @@ def parse_dcs(dcs): default_ports = {'consul': 8500, 'zookeeper': 2181, 'exhibitor': 8181} port = default_ports.get(str(scheme), 4001) - return {'scheme': str(scheme), 'hostname': str(parsed.hostname), 'port': int(port)} + 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): @@ -101,18 +112,12 @@ def ctl(ctx): def get_dcs(config, scope): - scheme, hostname, port = map(config.get('dcs', {}).get, ('scheme', 'hostname', 'port')) - - if scheme == 'etcd': - return Etcd(name=scope, config={'scope': scope, 'host': '{0}:{1}'.format(hostname, port)}) - - if scheme == 'zookeeper': - return ZooKeeper(name=scope, config={'scope': scope, 'hosts': [hostname], 'port': port}) - - if scheme == 'exhibitor': - return ZooKeeper(name=scope, config={'scope': scope, 'exhibitor': {'hosts': [hostname], 'port': port}}) - - raise PatroniCtlException('Can not find suitable configuration of distributed configuration store') + dcs_config = config.get('dcs', {}) + dcs_config[dcs_config.keys()[0]]['scope'] = scope + try: + return Patroni.get_dcs(scope, dcs_config) + except PatroniException as e: + raise PatroniCtlException(str(e)) def post_patroni(member, endpoint, content, headers=None): @@ -526,7 +531,7 @@ def failover(config_file, cluster_name, master, candidate, force, dcs, scheduled if scheduled_at.tzinfo is None: scheduled_at = tzlocal.get_localzone().localize(scheduled_at) except (ValueError, TypeError): - message = 'Unable to parse scheduled timestamp ({}). It should be in an unambiguous format (e.g. ISO 8601)' + message = 'Unable to parse scheduled timestamp ({0}). It should be in an unambiguous format (e.g. ISO 8601)' raise PatroniCtlException(message.format(scheduled)) scheduled_at = scheduled_at.isoformat() diff --git a/patroni/exceptions.py b/patroni/exceptions.py index d07e6426..97985696 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,6 +1,3 @@ -from click import ClickException - - class PatroniException(Exception): """Parent class for all kind of exceptions related to selected distributed configuration store""" @@ -16,10 +13,6 @@ class PatroniException(Exception): return repr(self.value) -class PatroniCtlException(ClickException): - pass - - class PostgresException(PatroniException): pass diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 95dd5a07..864be7a9 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -7,12 +7,10 @@ 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, \ - wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure + wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure, PatroniCtlException from patroni.etcd import Etcd, Client -from patroni.exceptions import PatroniCtlException from psycopg2 import OperationalError from test_etcd import etcd_read, requests_get, socket_getaddrinfo, MockResponse -from test_zookeeper import MockKazooClient from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \ get_cluster_initialized_with_only_leader from test_postgresql import MockConnect, psycopg2_connect @@ -43,7 +41,7 @@ def test_rw_config(): load_config(CONFIG_FILE_PATH, '0.0.0.0') -@patch('patroni.ctl.load_config', Mock(return_value={'dcs': {'scheme': 'etcd', 'hostname': 'localhost', 'port': 4001}})) +@patch('patroni.ctl.load_config', Mock(return_value={'dcs': {'etcd': {'host': 'localhost:4001'}}})) class TestCtl(unittest.TestCase): @patch('socket.getaddrinfo', socket_getaddrinfo) @@ -74,43 +72,25 @@ 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', '--dcs', '8.8.8.8'], input='''leader -other - -y''') + result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''') assert 'leader' in result.output - result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader -other -2100-01-01T12:23:00 -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -other -2030-01-01T12:23:00 -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -other - -N''') + 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', '--dcs', '8.8.8.8'], input='''leader -leader - -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -Reality - -y''') + 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']) @@ -128,47 +108,31 @@ y''') assert result.exit_code != 0 # Specifying wrong leader - result = self.runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy') + 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', '--dcs', '8.8.8.8'], input='''leader -other - -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -other - -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -other - -y''') + 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', '--dcs', '8.8.8.8'], input='''leader -other - -y''') + result = self.runner.invoke(ctl, ['failover', 'dummy'], input='''leader\nother\n\ny''') assert 'Failover failed' in result.output - @patch('patroni.zookeeper.KazooClient', MockKazooClient) - @patch('requests.get', requests_get) def test_get_dcs(self): - self.assertIsNotNone(get_dcs({'dcs': {'scheme': 'zookeeper', 'hostname': 'foo', 'port': 2181}}, 'dummy')) - self.assertIsNotNone(get_dcs({'dcs': {'scheme': 'exhibitor', 'hostname': 'exhibitor', 'port': 8181}}, 'dummy')) - self.assertRaises(PatroniCtlException, get_dcs, {'scheme': 'dummy'}, 'dummy') + self.assertRaises(PatroniCtlException, get_dcs, {'dcs': {'dummy': {}}}, 'dummy') @patch('psycopg2.connect', psycopg2_connect) @patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None))) @@ -227,7 +191,7 @@ y''') @patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) def test_dsn(self): with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)): - result = self.runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8']) + result = self.runner.invoke(ctl, ['dsn', 'alpha']) assert 'host=127.0.0.1 port=5435' in result.output # Mutually exclusive options @@ -242,45 +206,45 @@ y''') @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', '--dcs', '8.8.8.8'], input='y') + result = self.runner.invoke(ctl, ['restart', 'alpha'], input='y') assert result.exit_code == 0 - result = self.runner.invoke(ctl, ['reinit', 'alpha', '--dcs', '8.8.8.8'], input='y') + result = self.runner.invoke(ctl, ['reinit', 'alpha'], input='y') assert result.exit_code == 1 # Aborted restart - result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='N') + result = self.runner.invoke(ctl, ['restart', 'alpha'], input='N') assert result.exit_code == 1 # Not a member - result = self.runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8', 'dummy', '--any'], input='y') + 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', '--dcs', '8.8.8.8'], input='y') + 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): with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)): - result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave') + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='alpha\nslave') assert 'Please confirm' in result.output assert 'You are about to remove all' in result.output # Not typing an exact confirmation assert result.exit_code == 1 # master specified does not match master of cluster - result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha Yes I am aware slave''') assert result.exit_code == 1 # cluster specified on cmdline does not match verification prompt - result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader') + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='beta\nleader') assert result.exit_code == 1 - result = self.runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha Yes I am aware leader''') assert result.exit_code == 0 diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 501d7cd6..8c6a9718 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -9,7 +9,7 @@ from mock import Mock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor from patroni.etcd import Etcd -from patroni import Patroni, main as _main +from patroni import Patroni, PatroniException, main as _main from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer from test_etcd import Client, SleepException, etcd_read, etcd_write @@ -43,7 +43,7 @@ class TestPatroni(unittest.TestCase): @patch('patroni.zookeeper.KazooClient', MockKazooClient()) def test_get_dcs(self): self.assertIsInstance(self.p.get_dcs('', {'zookeeper': {'scope': '', 'hosts': ''}}), ZooKeeper) - self.assertRaises(Exception, self.p.get_dcs, '', {}) + self.assertRaises(PatroniException, self.p.get_dcs, '', {}) @patch('time.sleep', Mock(side_effect=SleepException)) @patch.object(Etcd, 'delete_leader', Mock()) From fee07efabbe3cde80113ce9cf65329b6c4c94859 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 18 Apr 2016 18:09:36 +0200 Subject: [PATCH 2/3] Fix python3 bugs --- patroni/ctl.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index b6f7eb0a..3e382ce0 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -33,14 +33,14 @@ class PatroniCtlException(ClickException): def parse_dcs(dcs): """ Break up the provided dcs string - >>> parse_dcs('localhost') - {'etcd': {'host': 'localhost:4001'}} - >>> parse_dcs('localhost:8500') - {'consul': {'host': 'localhost:8500'}} - >>> parse_dcs('zookeeper://localhost') - {'zookeeper': {'hosts': ['localhost:2181']}} - >>> parse_dcs('exhibitor://localhost') - {'zookeeper': {'exhibitor': {'hosts': ['localhost'], 'port': 8181}}} + >>> 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: @@ -113,7 +113,7 @@ def ctl(ctx): def get_dcs(config, scope): dcs_config = config.get('dcs', {}) - dcs_config[dcs_config.keys()[0]]['scope'] = scope + dcs_config[list(dcs_config.keys())[0]]['scope'] = scope try: return Patroni.get_dcs(scope, dcs_config) except PatroniException as e: From e4005ac76c7cbc1a3872ad791ea6790c6f6333e9 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 19 Apr 2016 10:30:04 +0200 Subject: [PATCH 3/3] Join multiline string with \n --- tests/test_ctl.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 864be7a9..8586c7d1 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -235,18 +235,14 @@ class TestCtl(unittest.TestCase): assert result.exit_code == 1 # master specified does not match master of cluster - result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha -Yes I am aware -slave''') + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha\nYes I am aware\nslave''') assert result.exit_code == 1 # cluster specified on cmdline does not match verification prompt result = self.runner.invoke(ctl, ['remove', 'alpha'], input='beta\nleader') assert result.exit_code == 1 - result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha -Yes I am aware -leader''') + result = self.runner.invoke(ctl, ['remove', 'alpha'], input='''alpha\nYes I am aware\nleader''') assert result.exit_code == 0 @patch('patroni.etcd.Etcd.watch', Mock(return_value=None))