From 1741fa7e0f5c0b4d930cea1e56e28bc6744a51bc Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 19 May 2016 10:00:32 +0200 Subject: [PATCH 1/8] Mininize number of references to dcs implementations from tests where it is not necessary (test_ha, test_ctl, etc...) It will simplyfy further refactoring and make it possible to install implementations of AbstractDCS independant of each other. --- tests/test_ctl.py | 164 ++++++++++++++++++++-------------------- tests/test_etcd.py | 5 +- tests/test_ha.py | 6 +- tests/test_patroni.py | 17 ++--- tests/test_zookeeper.py | 2 +- 5 files changed, 97 insertions(+), 97 deletions(-) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index e5c450c1..5d6b33d6 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -8,7 +8,6 @@ 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 -from patroni.etcd import Etcd, Client 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, \ @@ -50,9 +49,9 @@ class TestCtl(unittest.TestCase): @patch('socket.getaddrinfo', socket_getaddrinfo) def setUp(self): self.runner = CliRunner() - with patch.object(Client, 'machines') as mock_machines: + with patch.object(etcd.Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) - self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}) + self.e = get_dcs({'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}}, 'foo') @patch('psycopg2.connect', psycopg2_connect) def test_get_cursor(self): @@ -81,29 +80,30 @@ class TestCtl(unittest.TestCase): self.assertIsNone(output_members(cluster, name='abc', fmt='json')) self.assertIsNone(output_members(cluster, name='abc', fmt='tsv')) - @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)) + @patch('patroni.ctl.get_dcs') @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''') + def test_failover(self, mock_get_dcs): + mock_get_dcs.return_value = self.e + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader + 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''') + 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''') + 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''') + 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''') + 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''') + 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']) @@ -124,26 +124,26 @@ class TestCtl(unittest.TestCase): 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''') + 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''') + result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\ny') assert 'Failover failed' in result.output + # No members available + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_only_leader + result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\ny') + assert result.exit_code == 1 + + # No master available + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_without_leader + result = self.runner.invoke(ctl, ['failover', 'dummy'], input='leader\nother\n\ny') + assert result.exit_code == 1 + def test_get_dcs(self): self.assertRaises(PatroniCtlException, get_dcs, {'dummy': {}}, 'dummy') with patch('patroni.Patroni.get_dcs', Mock(return_value=self.e)): @@ -151,36 +151,37 @@ class TestCtl(unittest.TestCase): @patch('psycopg2.connect', psycopg2_connect) @patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None))) + @patch('patroni.ctl.get_dcs') @patch.object(etcd.Client, 'read', etcd_read) - def test_query(self): - with patch('patroni.ctl.get_dcs', Mock(return_value=self.e)): + def test_query(self, mock_get_dcs): + mock_get_dcs.return_value = self.e + # Mutually exclusive + result = self.runner.invoke(ctl, ['query', 'alpha', '--member', 'abc', '--role', 'master']) + assert result.exit_code == 1 + + with self.runner.isolated_filesystem(): + with open('dummy', 'w') as dummy_file: + dummy_file.write('SELECT 1') + # Mutually exclusive - result = self.runner.invoke(ctl, ['query', 'alpha', '--member', 'abc', '--role', 'master']) + result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy', '--command', 'dummy']) assert result.exit_code == 1 - with self.runner.isolated_filesystem(): - with open('dummy', 'w') as dummy_file: - dummy_file.write('SELECT 1') + result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) + assert result.exit_code == 0 - # Mutually exclusive - result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy', '--command', 'dummy']) - assert result.exit_code == 1 + os.remove('dummy') - result = self.runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) - assert result.exit_code == 0 + result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) + assert 'mock column' in result.output - os.remove('dummy') + # --command or --file is mandatory + result = self.runner.invoke(ctl, ['query', 'alpha']) + assert result.exit_code == 1 - result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) - assert 'mock column' in result.output - - # --command or --file is mandatory - result = self.runner.invoke(ctl, ['query', 'alpha']) - assert result.exit_code == 1 - - result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--username', 'root', - '--password', '--dbname', 'postgres'], input='ab\nab') - assert 'mock column' in result.output + result = self.runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--username', 'root', + '--password', '--dbname', 'postgres'], input='ab\nab') + assert 'mock column' in result.output def test_query_member(self): with patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor())): @@ -203,24 +204,24 @@ class TestCtl(unittest.TestCase): with patch('patroni.ctl.get_cursor', Mock(side_effect=OperationalError('bla'))): rows = query_member(None, None, None, 'replica', 'SELECT pg_is_in_recovery()') - @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']) - assert 'host=127.0.0.1 port=5435' in result.output + @patch('patroni.ctl.get_dcs') + def test_dsn(self, mock_get_dcs): + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader + result = self.runner.invoke(ctl, ['dsn', 'alpha']) + assert 'host=127.0.0.1 port=5435' in result.output - # Mutually exclusive options - result = self.runner.invoke(ctl, ['dsn', 'alpha', '--role', 'master', '--member', 'dummy']) - assert result.exit_code == 1 + # Mutually exclusive options + result = self.runner.invoke(ctl, ['dsn', 'alpha', '--role', 'master', '--member', 'dummy']) + assert result.exit_code == 1 - # Non-existing member - result = self.runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) - assert result.exit_code == 1 + # Non-existing member + result = self.runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) + assert result.exit_code == 1 - @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)) @patch('requests.post', requests_get) - def test_restart_reinit(self): + @patch('patroni.ctl.get_dcs') + def test_restart_reinit(self, mock_get_dcs): + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader result = self.runner.invoke(ctl, ['restart', 'alpha'], input='y') assert 'restart failed for' in result.output assert result.exit_code == 0 @@ -240,29 +241,28 @@ class TestCtl(unittest.TestCase): 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'], 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 + @patch('patroni.ctl.get_dcs') + def test_remove(self, mock_get_dcs): + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader + 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'], input='''alpha\nYes I am aware\nslave''') - assert result.exit_code == 1 + # master specified does not match master of cluster + 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 + # 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\nYes I am aware\nleader''') - assert result.exit_code == 0 + 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)) - @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) + @patch('patroni.dcs.AbstractDCS.watch', Mock(return_value=None)) + @patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) def test_wait_for_leader(self): self.assertRaises(PatroniCtlException, wait_for_leader, self.e, 0) @@ -299,9 +299,9 @@ class TestCtl(unittest.TestCase): self.assertEquals(len(list(get_all_members(get_cluster_initialized_without_leader(), role='replica'))), 2) - @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): + @patch('patroni.ctl.get_dcs') + def test_members(self, mock_get_dcs): + mock_get_dcs.return_value.get_cluster = get_cluster_initialized_with_leader result = self.runner.invoke(members, ['alpha']) assert '127.0.0.1' in result.output assert result.exit_code == 0 diff --git a/tests/test_etcd.py b/tests/test_etcd.py index ef61f413..a6116652 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -240,6 +240,9 @@ class TestEtcd(unittest.TestCase): def test_delete_leader(self): self.assertFalse(self.etcd.delete_leader()) + def test_delete_cluster(self): + self.assertFalse(self.etcd.delete_cluster()) + @patch.object(etcd.Client, 'watch', etcd_watch) def test_watch(self): self.etcd.watch(0) @@ -249,6 +252,6 @@ class TestEtcd(unittest.TestCase): with patch.object(AbstractDCS, 'watch', Mock()): self.etcd.watch(9.5) - @patch('patroni.etcd.Etcd.retry', Mock(side_effect=AttributeError("foo"))) def test_other_exceptions(self): + self.etcd.retry = Mock(side_effect=AttributeError('foo')) self.assertRaises(EtcdError, self.etcd.cancel_initialization) diff --git a/tests/test_ha.py b/tests/test_ha.py index 3071ecc8..9dc512e0 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -5,10 +5,10 @@ import pytz from mock import Mock, MagicMock, patch from patroni.dcs import Cluster, Failover, Leader, Member -from patroni.etcd import Client, Etcd from patroni.exceptions import DCSError, PostgresException from patroni.ha import Ha from patroni.postgresql import Postgresql +from patroni import Patroni from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get @@ -85,7 +85,7 @@ class TestHa(unittest.TestCase): @patch('socket.getaddrinfo', socket_getaddrinfo) @patch.object(etcd.Client, 'read', etcd_read) def setUp(self): - with patch.object(Client, 'machines') as mock_machines: + with patch.object(etcd.Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) self.p = Postgresql({'name': 'postgresql0', 'scope': 'dummy', 'listen': '127.0.0.1:5432', 'data_dir': 'data/postgresql0', 'superuser': {}, 'admin': {}, @@ -94,7 +94,7 @@ class TestHa(unittest.TestCase): self.p.set_role('replica') self.p.check_replication_lag = true self.p.can_create_replica_without_replication_connection = MagicMock(return_value=False) - self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}) + self.e = Patroni.get_dcs('foo', {'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}}) self.ha = Ha(MockPatroni(self.p, self.e)) self.ha._async_executor.run_async = run_async self.ha.old_cluster = self.e.get_cluster() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 6f7e67fe..3f5e913c 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -9,11 +9,10 @@ from mock import Mock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor from patroni.consul import Consul -from patroni.etcd import Etcd 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 +from test_etcd import SleepException, etcd_read, etcd_write from test_postgresql import Postgresql, psycopg2_connect from test_zookeeper import MockKazooClient @@ -30,13 +29,11 @@ from test_zookeeper import MockKazooClient class TestPatroni(unittest.TestCase): def setUp(self): - with patch.object(Client, 'machines') as mock_machines: + RestApiServer._BaseServer__is_shut_down = Mock() + RestApiServer._BaseServer__shutdown_request = True + RestApiServer.socket = 0 + with patch.object(etcd.Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) - self.touched = False - self.init_cancelled = False - RestApiServer._BaseServer__is_shut_down = Mock() - RestApiServer._BaseServer__shutdown_request = True - RestApiServer.socket = 0 with open('postgres0.yml', 'r') as f: config = yaml.load(f) self.p = Patroni(config) @@ -49,8 +46,8 @@ class TestPatroni(unittest.TestCase): self.assertRaises(PatroniException, self.p.get_dcs, '', {}) @patch('time.sleep', Mock(side_effect=SleepException)) - @patch.object(Etcd, 'delete_leader', Mock()) - @patch.object(Client, 'machines') + @patch.object(etcd.Client, 'delete', Mock()) + @patch.object(etcd.Client, 'machines') def test_patroni_main(self, mock_machines): with patch('subprocess.call', Mock(return_value=1)): _main() diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index 5e9ebc82..a784b0eb 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -92,7 +92,7 @@ class MockKazooClient(Mock): @patch('requests.get', requests_get) -@patch('patroni.zookeeper.sleep', Mock(side_effect=SleepException())) +@patch('time.sleep', Mock(side_effect=SleepException)) class TestExhibitorEnsembleProvider(unittest.TestCase): def test_init(self): From 5bfc41d47551b85a3f35d91d03b9b89bf6e3c6b3 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 19 May 2016 10:22:13 +0200 Subject: [PATCH 2/8] Update .zappr.yml --- .zappr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zappr.yml b/.zappr.yml index 4d628636..0058bd69 100644 --- a/.zappr.yml +++ b/.zappr.yml @@ -2,7 +2,7 @@ approvals: # PR needs at least 4 approvals minimum: 1 # approval = comment that matches this regex - pattern: "^:?\\+1:?$" + pattern: "^\s*(:?\\+1:?|👍)\s*$" from: # commenter must be either one of: # a public zalando org member From 0c2aad98a3410da68f9a994523a30582a51de6b1 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 19 May 2016 10:57:18 +0200 Subject: [PATCH 3/8] Move dcs implementations into dcs package --- patroni/__init__.py | 6 +++--- patroni/{dcs.py => dcs/__init__.py} | 0 patroni/{ => dcs}/consul.py | 0 patroni/{ => dcs}/etcd.py | 0 patroni/{ => dcs}/zookeeper.py | 0 tests/test_consul.py | 3 +-- tests/test_etcd.py | 3 +-- tests/test_patroni.py | 6 +++--- tests/test_zookeeper.py | 7 +++---- 9 files changed, 11 insertions(+), 14 deletions(-) rename patroni/{dcs.py => dcs/__init__.py} (100%) rename patroni/{ => dcs}/consul.py (100%) rename patroni/{ => dcs}/etcd.py (100%) rename patroni/{ => dcs}/zookeeper.py (100%) diff --git a/patroni/__init__.py b/patroni/__init__.py index 161270cf..c60bbffe 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -43,13 +43,13 @@ class Patroni(object): @staticmethod def get_dcs(name, config): if 'etcd' in config: - from patroni.etcd import Etcd + from patroni.dcs.etcd import Etcd return Etcd(name, config['etcd']) if 'zookeeper' in config: - from patroni.zookeeper import ZooKeeper + from patroni.dcs.zookeeper import ZooKeeper return ZooKeeper(name, config['zookeeper']) if 'consul' in config: - from patroni.consul import Consul + from patroni.dcs.consul import Consul return Consul(name, config['consul']) raise PatroniException('Can not find suitable configuration of distributed configuration store') diff --git a/patroni/dcs.py b/patroni/dcs/__init__.py similarity index 100% rename from patroni/dcs.py rename to patroni/dcs/__init__.py diff --git a/patroni/consul.py b/patroni/dcs/consul.py similarity index 100% rename from patroni/consul.py rename to patroni/dcs/consul.py diff --git a/patroni/etcd.py b/patroni/dcs/etcd.py similarity index 100% rename from patroni/etcd.py rename to patroni/dcs/etcd.py diff --git a/patroni/zookeeper.py b/patroni/dcs/zookeeper.py similarity index 100% rename from patroni/zookeeper.py rename to patroni/dcs/zookeeper.py diff --git a/tests/test_consul.py b/tests/test_consul.py index 7cd07bbb..22665174 100644 --- a/tests/test_consul.py +++ b/tests/test_consul.py @@ -1,9 +1,8 @@ import consul import unittest -from patroni.dcs import AbstractDCS from mock import Mock, patch -from patroni.consul import Cluster, Consul, ConsulError, ConsulException, HTTPClient, NotFound +from patroni.dcs.consul import AbstractDCS, Cluster, Consul, ConsulError, ConsulException, HTTPClient, NotFound from test_etcd import SleepException diff --git a/tests/test_etcd.py b/tests/test_etcd.py index a6116652..508cd480 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -6,8 +6,7 @@ import unittest from dns.exception import DNSException from mock import Mock, patch -from patroni.dcs import Cluster, AbstractDCS -from patroni.etcd import Client, Etcd, EtcdError +from patroni.dcs.etcd import AbstractDCS, Client, Cluster, Etcd, EtcdError from patroni.exceptions import DCSError from urllib3.exceptions import ReadTimeoutError diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 3f5e913c..caaadf78 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -8,9 +8,9 @@ import yaml from mock import Mock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor -from patroni.consul import Consul +from patroni.dcs.consul import Consul +from patroni.dcs.zookeeper import ZooKeeper from patroni import Patroni, PatroniException, main as _main -from patroni.zookeeper import ZooKeeper from six.moves import BaseHTTPServer from test_etcd import SleepException, etcd_read, etcd_write from test_postgresql import Postgresql, psycopg2_connect @@ -38,7 +38,7 @@ class TestPatroni(unittest.TestCase): config = yaml.load(f) self.p = Patroni(config) - @patch('patroni.zookeeper.KazooClient', MockKazooClient()) + @patch('patroni.dcs.zookeeper.KazooClient', MockKazooClient()) @patch.object(Consul, 'create_or_restore_session', Mock()) def test_get_dcs(self): self.assertIsInstance(self.p.get_dcs('', {'zookeeper': {'scope': '', 'hosts': ''}}), ZooKeeper) diff --git a/tests/test_zookeeper.py b/tests/test_zookeeper.py index a784b0eb..d2a288f0 100644 --- a/tests/test_zookeeper.py +++ b/tests/test_zookeeper.py @@ -1,12 +1,11 @@ import six import unittest -from mock import Mock, patch -from patroni.dcs import Leader -from patroni.zookeeper import ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError from kazoo.client import KazooState from kazoo.exceptions import NoNodeError, NodeExistsError from kazoo.protocol.states import ZnodeStat +from mock import Mock, patch +from patroni.dcs.zookeeper import Leader, ExhibitorEnsembleProvider, ZooKeeper, ZooKeeperError from test_etcd import SleepException, requests_get @@ -102,7 +101,7 @@ class TestExhibitorEnsembleProvider(unittest.TestCase): class TestZooKeeper(unittest.TestCase): @patch('requests.get', requests_get) - @patch('patroni.zookeeper.KazooClient', MockKazooClient) + @patch('patroni.dcs.zookeeper.KazooClient', MockKazooClient) def setUp(self): self.zk = ZooKeeper('foo', {'exhibitor': {'hosts': ['localhost', 'exhibitor'], 'port': 8181}, 'scope': 'test'}) From 6a4793bba879ba0287a50c435558c4ace90a27a1 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 19 May 2016 12:42:19 +0200 Subject: [PATCH 4/8] Find and load dcs class implementation dynamically --- patroni/__init__.py | 17 ++--------------- patroni/ctl.py | 8 ++++---- patroni/dcs/__init__.py | 24 ++++++++++++++++++++++++ tests/test_ctl.py | 2 -- tests/test_ha.py | 5 ++--- tests/test_patroni.py | 12 +----------- 6 files changed, 33 insertions(+), 35 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index c60bbffe..70c7fdfb 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -5,7 +5,7 @@ import time import yaml from patroni.api import RestApiServer -from patroni.exceptions import PatroniException +from patroni.dcs import get_dcs from patroni.ha import Ha from patroni.postgresql import Postgresql from patroni.utils import reap_children, set_ignore_sigterm, setup_signal_handlers @@ -22,7 +22,7 @@ class Patroni(object): self.tags = {tag: value for tag, value in config.get('tags', {}).items() if tag not in ('clonefrom', 'nofailover', 'noloadbalance') or value} self.postgresql = Postgresql(config['postgresql']) - self.dcs = self.get_dcs(self.postgresql.name, config) + self.dcs = get_dcs(self.postgresql.name, config) self.version = __version__ self.api = RestApiServer(self, config['restapi']) self.ha = Ha(self) @@ -40,19 +40,6 @@ class Patroni(object): def replicatefrom(self): return self.tags.get('replicatefrom') - @staticmethod - def get_dcs(name, config): - if 'etcd' in config: - from patroni.dcs.etcd import Etcd - return Etcd(name, config['etcd']) - if 'zookeeper' in config: - from patroni.dcs.zookeeper import ZooKeeper - return ZooKeeper(name, config['zookeeper']) - if 'consul' in config: - from patroni.dcs.consul import Consul - return Consul(name, config['consul']) - raise PatroniException('Can not find suitable configuration of distributed configuration store') - def schedule_next_run(self): self.next_run += self.nap_time current_time = time.time() diff --git a/patroni/ctl.py b/patroni/ctl.py index ee6e5e57..19b48feb 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -16,7 +16,8 @@ import tzlocal import yaml from click import ClickException -from patroni import Patroni, PatroniException +from patroni.dcs import get_dcs as _get_dcs +from patroni.exceptions import PatroniException from patroni.postgresql import parseurl from prettytable import PrettyTable from six.moves.urllib_parse import urlparse @@ -93,10 +94,9 @@ def ctl(ctx): def get_dcs(config, scope): - for k in set(DCS_DEFAULTS.keys()) & set(config.keys()): - config[k].setdefault('scope', scope) + config.setdefault('scope', scope) try: - return Patroni.get_dcs(scope, config) + return _get_dcs(scope, config) except PatroniException as e: raise PatroniCtlException(str(e)) diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index 7ae70269..ceffecbc 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -1,9 +1,13 @@ import abc import dateutil +import importlib +import inspect import json +import os import six from collections import namedtuple +from patroni.exceptions import PatroniException from random import randint from six.moves.urllib_parse import urlparse, urlunparse, parse_qsl from threading import Event, Lock @@ -26,6 +30,26 @@ def parse_connection_string(value): return conn_url, api_url +def get_dcs(node_name, config): + available_implementations = [] + for name in os.listdir(os.path.dirname(__file__)): + if name.endswith('.py') and not name.startswith('__'): # find module + module = importlib.import_module(__package__ + '.' + name[:-3]) + for name in dir(module): # iterate through module content + if not name.startswith('__'): # skip internal stuff + value = getattr(module, name) + name = name.lower() + # try to find implementation of AbstractDCS interface + if inspect.isclass(value) and issubclass(value, AbstractDCS): + available_implementations.append(name) + if name in config: # which has configuration section in the config file + # propagate some parameters + config[name].update({p: config[p] for p in ('namespace', 'scope', 'ttl') if p in config}) + return value(node_name, config[name]) + raise PatroniException("""Can not find suitable configuration of distributed configuration store +Available implementations: """ + ', '.join(available_implementations)) + + class Member(namedtuple('Member', 'index,name,session,data')): """Immutable object (namedtuple) which represents single member of PostgreSQL cluster. diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 5d6b33d6..d7394db3 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -146,8 +146,6 @@ class TestCtl(unittest.TestCase): def test_get_dcs(self): 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))) diff --git a/tests/test_ha.py b/tests/test_ha.py index 9dc512e0..5ceb3392 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -4,11 +4,10 @@ import datetime import pytz from mock import Mock, MagicMock, patch -from patroni.dcs import Cluster, Failover, Leader, Member +from patroni.dcs import Cluster, Failover, Leader, Member, get_dcs from patroni.exceptions import DCSError, PostgresException from patroni.ha import Ha from patroni.postgresql import Postgresql -from patroni import Patroni from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get @@ -94,7 +93,7 @@ class TestHa(unittest.TestCase): self.p.set_role('replica') self.p.check_replication_lag = true self.p.can_create_replica_without_replication_connection = MagicMock(return_value=False) - self.e = Patroni.get_dcs('foo', {'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}}) + self.e = get_dcs('foo', {'etcd': {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}}) self.ha = Ha(MockPatroni(self.p, self.e)) self.ha._async_executor.run_async = run_async self.ha.old_cluster = self.e.get_cluster() diff --git a/tests/test_patroni.py b/tests/test_patroni.py index caaadf78..d9daaedc 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -8,13 +8,10 @@ import yaml from mock import Mock, patch from patroni.api import RestApiServer from patroni.async_executor import AsyncExecutor -from patroni.dcs.consul import Consul -from patroni.dcs.zookeeper import ZooKeeper -from patroni import Patroni, PatroniException, main as _main +from patroni import Patroni, main as _main from six.moves import BaseHTTPServer from test_etcd import SleepException, etcd_read, etcd_write from test_postgresql import Postgresql, psycopg2_connect -from test_zookeeper import MockKazooClient @patch('time.sleep', Mock()) @@ -38,13 +35,6 @@ class TestPatroni(unittest.TestCase): config = yaml.load(f) self.p = Patroni(config) - @patch('patroni.dcs.zookeeper.KazooClient', MockKazooClient()) - @patch.object(Consul, 'create_or_restore_session', Mock()) - def test_get_dcs(self): - self.assertIsInstance(self.p.get_dcs('', {'zookeeper': {'scope': '', 'hosts': ''}}), ZooKeeper) - self.assertIsInstance(self.p.get_dcs('', {'consul': {'scope': '', 'hosts': '127.0.0.1:1'}}), Consul) - self.assertRaises(PatroniException, self.p.get_dcs, '', {}) - @patch('time.sleep', Mock(side_effect=SleepException)) @patch.object(etcd.Client, 'delete', Mock()) @patch.object(etcd.Client, 'machines') From b43b670195a8a08e6381f2c4b7e73ede26e2a018 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 19 May 2016 14:17:55 +0200 Subject: [PATCH 5/8] Update .zappr.yml --- .zappr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zappr.yml b/.zappr.yml index 0058bd69..2aeb5a7f 100644 --- a/.zappr.yml +++ b/.zappr.yml @@ -2,7 +2,7 @@ approvals: # PR needs at least 4 approvals minimum: 1 # approval = comment that matches this regex - pattern: "^\s*(:?\\+1:?|👍)\s*$" + pattern: "^\s*(:?\\+1:?|\u{1F44D})\s*$" from: # commenter must be either one of: # a public zalando org member From 79ecfd994a29784b6c41ee6abd04ba62d1f646fc Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 19 May 2016 14:18:56 +0200 Subject: [PATCH 6/8] Update .zappr.yml --- .zappr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zappr.yml b/.zappr.yml index 2aeb5a7f..5c9153d1 100644 --- a/.zappr.yml +++ b/.zappr.yml @@ -2,7 +2,7 @@ approvals: # PR needs at least 4 approvals minimum: 1 # approval = comment that matches this regex - pattern: "^\s*(:?\\+1:?|\u{1F44D})\s*$" + pattern: "^\s*:?\\+1:?\s*$" from: # commenter must be either one of: # a public zalando org member From 4186e73c139944d4884bb66e7f22c8edd860fd5b Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 19 May 2016 14:21:33 +0200 Subject: [PATCH 7/8] Update .zappr.yml --- .zappr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zappr.yml b/.zappr.yml index 5c9153d1..baadb57c 100644 --- a/.zappr.yml +++ b/.zappr.yml @@ -2,7 +2,7 @@ approvals: # PR needs at least 4 approvals minimum: 1 # approval = comment that matches this regex - pattern: "^\s*:?\\+1:?\s*$" + pattern: "^\\s*(:?\\+1:?|\\u{1F44D})\\s*$" from: # commenter must be either one of: # a public zalando org member From dcfbdc7d2965109135046144edaa24a03af47c8d Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 19 May 2016 14:22:58 +0200 Subject: [PATCH 8/8] Update .zappr.yml --- .zappr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zappr.yml b/.zappr.yml index baadb57c..68a8cdbb 100644 --- a/.zappr.yml +++ b/.zappr.yml @@ -2,7 +2,7 @@ approvals: # PR needs at least 4 approvals minimum: 1 # approval = comment that matches this regex - pattern: "^\\s*(:?\\+1:?|\\u{1F44D})\\s*$" + pattern: "^\\s*(:?\\+1:?|👍)\\s*$" from: # commenter must be either one of: # a public zalando org member