From 64e09f7ca780aa2128309c9b63c7228b4a02eae9 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:29:05 +0100 Subject: [PATCH 1/8] Patronictl: Prettier error messages by inheriting from ClickException --- patroni/exceptions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 43f54e7f..97b88238 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,3 +1,5 @@ +from click import ClickException + class PatroniException(Exception): """Parent class for all kind of exceptions related to selected distributed configuration store""" @@ -13,7 +15,7 @@ class PatroniException(Exception): return repr(self.value) -class PatroniCtlException(Exception): +class PatroniCtlException(ClickException): pass From 47007c333159c245537673294d24926d355fd1c9 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:30:11 +0100 Subject: [PATCH 2/8] Dockerfile: Ensure all python packages are available and patronictl is configured --- Dockerfile | 9 +++++++-- docker/entrypoint.sh | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 84b9cdf6..b9c3b9ff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,14 +14,19 @@ RUN apt-get upgrade -y ENV PGVERSION 9.4 RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} -y -RUN pip install python-etcd psycopg2 +RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} python-prettytable -y +ADD requirements-py2.txt /tmp/ +RUN pip install -r /tmp/requirements-py2.txt ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH ADD patroni.py /patroni.py +ADD patronictl.py /patronictl.py ADD patroni/ /patroni +RUN ln -s /patroni.py /usr/local/bin/patroni +RUN ln -s /patronictl.py /usr/local/bin/patronictl + ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 9edb2120..31d76e8b 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -79,6 +79,11 @@ then ETCD_CLUSTER="127.0.0.1:4001" fi +mkdir -p ~postgres/.config/patroni +cat > ~postgres/.config/patroni/patronictl.yaml <<__EOF__ +{dcs_api: 'etcd://${ETCD_CLUSTER}', namespace: /service/} +__EOF__ + cat > /patroni/postgres.yml <<__EOF__ ttl: &ttl 30 From 42e07148017f2751b65c2213ddfbe006766b657e Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:30:49 +0100 Subject: [PATCH 3/8] Patronictl: Allow specification of dbname and user, as well as password prompting. --- patroni/ctl.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 3d457c9f..5da78388 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -252,6 +252,8 @@ def dsn(cluster_name, config_file, dcs, role, member): @option_format @click.option('--format', help='Output format (pretty, json)', default='tsv') @click.option('--file', '-f', help='Execute the SQL commands from this file', type=click.File('rb')) +@click.option('--password', help='force password prompt', is_flag=True) +@click.option('-U', '--username', help='database user name', type=str) @option_dcs @option_watch @option_watchrefresh @@ -260,6 +262,7 @@ def dsn(cluster_name, config_file, dcs, role, member): @click.option('--member', '-m', help='Query a specific member', type=str) @click.option('--delimiter', help='The column delimiter', default='\t') @click.option('--command', '-c', help='The SQL commands to execute') +@click.option('-d', '--dbname', help='database name to connect to', type=str) def query( cluster_name, config_file, @@ -271,6 +274,9 @@ def query( delimiter, command, file, + password, + username, + dbname, format='tsv', ): if role is not None and member is not None: @@ -281,6 +287,17 @@ def query( if file is not None and command is not None: raise PatroniCtlException('--file and --command are mutually exclusive options') + if file is None and command is None: + raise PatroniCtlException('You need to specify either --command or --file') + + connect_parameters = dict() + if username: + connect_parameters['user'] = username + if password: + connect_parameters['password'] = click.prompt('Password', hide_input=True, type=str) + if dbname: + connect_parameters['database'] = dbname + if file is not None: command = file.read() @@ -289,17 +306,17 @@ def query( cursor = None for _ in watching(w, watch, clear=False): - output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command) + output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, connect_parameters=connect_parameters) print_output(None, output, format=format, delimiter=delimiter) if cursor is None: cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command): +def query_member(cluster, cursor, member, role, command, connect_parameters): try: if cursor is None: - cursor = get_cursor(cluster, role=role, member=member) + cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) if cursor is None: if role is None: From 6568c56c8585bd36e11ac599550f787bdce79302 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 16:17:18 +0100 Subject: [PATCH 4/8] Patronictl: Add tests to increase coverage, fix regression issue. --- patroni/ctl.py | 5 +++-- patroni/exceptions.py | 1 + tests/test_ctl.py | 45 ++++++++++++++++++++++++++----------------- tests/test_ha.py | 2 +- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 5da78388..1ddb1625 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -306,14 +306,15 @@ def query( cursor = None for _ in watching(w, watch, clear=False): - output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, connect_parameters=connect_parameters) + output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, + connect_parameters=connect_parameters) print_output(None, output, format=format, delimiter=delimiter) if cursor is None: cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command, connect_parameters): +def query_member(cluster, cursor, member, role, command, connect_parameters=dict()): try: if cursor is None: cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 97b88238..d07e6426 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,5 +1,6 @@ from click import ClickException + class PatroniException(Exception): """Parent class for all kind of exceptions related to selected distributed configuration store""" diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 0d65a05c..a2226b7a 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -102,35 +102,35 @@ y''') result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other N''') - assert 'Aborting failover' in str(result.exception) + assert 'Aborting failover' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader leader y''') - assert 'target and source are the same' in str(result.exception) + assert 'target and source are the same' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader Reality y''') - assert 'Reality does not exist' in str(result.exception) + assert 'Reality does not exist' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--force']) assert 'Failing over to new leader' in result.output result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy') - assert 'is not the leader of cluster' in str(result.exception) + assert 'is not the leader of cluster' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') - assert 'No candidates found to failover to' in str(result.exception) + assert 'No candidates found to failover to' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') - assert 'This cluster has no master' in str(result.exception) + assert 'This cluster has no master' in str(result.output) with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader @@ -149,13 +149,13 @@ y''') # with patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())): # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense') -# assert 'is not the leader of cluster' in str(result.exception) +# assert 'is not the leader of cluster' in str(result.output) # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense']) - # assert 'is not the leader of cluster' in str(result.exception) + # assert 'is not the leader of cluster' in str(result.output) # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn') - # assert 'Aborting failover' in str(result.exception) + # assert 'Aborting failover' in str(result.output) # with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())): # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') @@ -181,13 +181,19 @@ y''') '--role', 'master', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) with runner.isolated_filesystem(): dummy_file = open('dummy', 'w') dummy_file.write('SELECT 1') dummy_file.close() + result = runner.invoke(ctl, [ + 'query', + 'alpha' + ]) + assert 'You need to specify' in str(result.output) + result = runner.invoke(ctl, [ 'query', 'alpha', @@ -196,7 +202,7 @@ y''') '--command', 'dummy', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) result = runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) @@ -205,6 +211,9 @@ y''') result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) assert 'mock column' in result.output + result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy', '--password', '--username', 'dummy'], input='password\n') + assert 'mock column' in result.output + @patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor())) def test_query_member(self): rows = query_member(None, None, None, 'master', 'SELECT pg_is_in_recovery()') @@ -242,10 +251,10 @@ y''') '--member', 'dummy', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) result = runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) - assert 'Can not find' in str(result.exception) + assert 'Can not find' in str(result.output) # result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica']) # assert 'host=127.0.0.1 port=5436' in result.output @@ -269,7 +278,7 @@ y''') 'dummy', '--any', ], input='y') - assert 'not a member' in str(result.exception) + assert 'not a member' in str(result.output) with patch('requests.post', Mock(return_value=MockResponse())): result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') @@ -282,15 +291,15 @@ y''') result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave') assert 'Please confirm' in result.output assert 'You are about to remove all' in result.output - assert 'You did not exactly type' in str(result.exception) + assert 'You did not exactly type' in str(result.output) result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha Yes I am aware slave''') - assert 'You did not specify the current master of the cluster' in str(result.exception) + assert 'You did not specify the current master of the cluster' in str(result.output) result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader') - assert 'Cluster names specified do not match' in str(result.exception) + assert 'Cluster names specified do not match' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', get_cluster_initialized_with_leader): result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], @@ -304,7 +313,7 @@ leader''') input='''alpha Yes I am aware leader''') - assert 'We have not implemented this for DCS of type' in str(result.exception) + assert 'We have not implemented this for DCS of type' in str(result.output) @patch('patroni.etcd.Etcd.watch', Mock(return_value=None)) @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) diff --git a/tests/test_ha.py b/tests/test_ha.py index d9a408a4..5a3f5f9a 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -18,7 +18,7 @@ def false(*args, **kwargs): def get_cluster(initialize, leader, members, failover): - return Cluster(initialize, leader, None, members, failover) + return Cluster(initialize, leader, 10, members, failover) def get_cluster_not_initialized_without_leader(): From feac841aadef08edf7e5dfd6d1b6e91f0411fc69 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 19:48:35 +0100 Subject: [PATCH 5/8] Docker: Install python packages via pip only, yaml consistency --- Dockerfile | 4 ++-- docker/entrypoint.sh | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index b9c3b9ff..97995128 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,8 +13,8 @@ RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} python-prettytable -y +RUN apt-get install postgresql-server-dev-${PGVERSION} -y +RUN apt-get install python-pip python-dev -y ADD requirements-py2.txt /tmp/ RUN pip install -r /tmp/requirements-py2.txt diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 31d76e8b..b549cc46 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -84,7 +84,7 @@ cat > ~postgres/.config/patroni/patronictl.yaml <<__EOF__ {dcs_api: 'etcd://${ETCD_CLUSTER}', namespace: /service/} __EOF__ -cat > /patroni/postgres.yml <<__EOF__ +cat > /patroni/postgres.yaml <<__EOF__ ttl: &ttl 30 loop_wait: &loop_wait 10 @@ -131,7 +131,7 @@ postgresql: hot_standby: "on" __EOF__ -cat /patroni/postgres.yml +cat /patroni/postgres.yaml if [ ! -z $CHEAT ] then @@ -140,5 +140,5 @@ then sleep 60 done else - exec python /patroni.py /patroni/postgres.yml + exec python /patroni.py /patroni/postgres.yaml fi From a64c7abdcc07c5cd78501a0a7a19072d8c74bcac Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 23 Dec 2015 09:13:23 +0100 Subject: [PATCH 6/8] Bugfix: Fixing python-etcd version, as behaviour has changed in newer version. Our current master branch doesn't pass the code coverage test, due to behaviour changes in upstream python-etcd. As a bandaid, fix the version for now. Reference build fail: https://travis-ci.org/zalando/patroni/jobs/98470121 --- requirements-py2.txt | 2 +- requirements-py3.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-py2.txt b/requirements-py2.txt index f8eacc12..23193254 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -6,6 +6,6 @@ PyYAML requests six >= 1.7 kazoo>=2.2.1 -python-etcd>=0.4.1 +python-etcd==0.4.1 click>=4.1 prettytable>=0.7 diff --git a/requirements-py3.txt b/requirements-py3.txt index 30b5ce96..3e2df5f1 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -6,6 +6,6 @@ PyYAML requests six kazoo>=2.2.1 -python-etcd>=0.4.1 +python-etcd==0.4.1 click>=4.1 prettytable>=0.7 From 2d457ae26af4e94f1c10919ce255751c5aaa9352 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 1 Feb 2016 11:37:59 +0100 Subject: [PATCH 7/8] Fix a problem with mutable default arguments. Also bump up the version of python-etcd in requirements to the latest one that that does not have https://github.com/jplana/python-etcd/issues/152 --- patroni/ctl.py | 12 +++++++++--- requirements-py2.txt | 2 +- requirements-py3.txt | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 1ddb1625..635b0a89 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -168,7 +168,9 @@ def watching(w, watch, max_count=None, clear=True): yield 0 -def build_connect_parameters(conn_url, connect_parameters={}): +def build_connect_parameters(conn_url, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} params = connect_parameters.copy() parsed = parseurl(conn_url) params['host'] = parsed['host'] @@ -200,7 +202,9 @@ def get_any_member(cluster, role='master', member=None): return None -def get_cursor(cluster, role='master', member=None, connect_parameters={}): +def get_cursor(cluster, role='master', member=None, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} member = get_any_member(cluster=cluster, role=role, member=member) if member is None: return None @@ -314,7 +318,9 @@ def query( cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command, connect_parameters=dict()): +def query_member(cluster, cursor, member, role, command, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} try: if cursor is None: cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) diff --git a/requirements-py2.txt b/requirements-py2.txt index 23193254..1e194bc0 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -6,6 +6,6 @@ PyYAML requests six >= 1.7 kazoo>=2.2.1 -python-etcd==0.4.1 +python-etcd==0.4.2 click>=4.1 prettytable>=0.7 diff --git a/requirements-py3.txt b/requirements-py3.txt index 3e2df5f1..13c3010c 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -6,6 +6,6 @@ PyYAML requests six kazoo>=2.2.1 -python-etcd==0.4.1 +python-etcd==0.4.2 click>=4.1 prettytable>=0.7 From 7db5ec1269f459b3f16c69c1949f4cf5bd2bfb44 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 3 Feb 2016 16:46:30 +0100 Subject: [PATCH 8/8] Revert global Docker changes --- Dockerfile | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 97995128..362c9bf4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,10 +13,9 @@ RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install postgresql-server-dev-${PGVERSION} -y -RUN apt-get install python-pip python-dev -y -ADD requirements-py2.txt /tmp/ -RUN pip install -r /tmp/requirements-py2.txt +RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y +RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} -y +RUN pip install python-etcd psycopg2 ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH @@ -24,9 +23,6 @@ ADD patroni.py /patroni.py ADD patronictl.py /patronictl.py ADD patroni/ /patroni -RUN ln -s /patroni.py /usr/local/bin/patroni -RUN ln -s /patronictl.py /usr/local/bin/patronictl - ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl