From 2c7e3f60cc249e8c34f92e85ab3829e433ca5fbc Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 21 Oct 2015 15:34:55 +0200 Subject: [PATCH 1/8] Make possible to override default namespace (/service/) from a config file If the namespace is not specified in a config file /service/ would be used. Also it's possible to use just '/' as a namespace. It means we would have following structure: /scope1 /scope2 ... --- patroni/dcs.py | 4 ++-- tests/test_etcd.py | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index a8afda06..bb56fecc 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -122,8 +122,8 @@ class AbstractDCS: i.e.: `zookeeper` for zookeeper, `etcd` for etcd, etc... """ self._name = name - self._scope = config['scope'] - self._base_path = '/service/' + self._scope + self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/')) + self._base_path = '/'.join([self._namespace, config['scope']]) self._cluster = None self._cluster_thread_lock = Lock() diff --git a/tests/test_etcd.py b/tests/test_etcd.py index d0d01d71..6ffa59af 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -80,7 +80,7 @@ def etcd_watch(key, index=None, timeout=None, recursive=None): def etcd_write(key, value, **kwargs): if key == '/service/exists/leader': raise etcd.EtcdAlreadyExist - if key == '/service/test/leader': + if key == '/service/test/leader' or key == '/patroni/test/leader': if kwargs.get('prevValue', None) == 'foo' or not kwargs.get('prevExist', True): return True raise etcd.EtcdException @@ -204,11 +204,14 @@ class TestEtcd(unittest.TestCase): def setUp(self): with patch.object(Client, 'machines') as mock_machines: mock_machines.__get__ = Mock(return_value=['http://localhost:2379', 'http://localhost:4001']) - self.etcd = Etcd('foo', {'ttl': 30, 'host': 'localhost:2379', 'scope': 'test'}) + self.etcd = Etcd('foo', {'namespace': '/patroni/', 'ttl': 30, 'host': 'localhost:2379', 'scope': 'test'}) self.etcd.client.write = etcd_write self.etcd.client.read = etcd_read self.etcd.client.delete = Mock(side_effect=etcd.EtcdException()) + def test_base_path(self): + self.assertEquals(self.etcd._base_path, '/patroni/test') + @patch('dns.resolver.query', dns_query) def test_get_etcd_client(self): with patch.object(etcd.Client, 'machines') as mock_machines: From deaaf8ad1aa1ecae2c5bef9768c3e647a447b647 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 21 Oct 2015 15:38:51 +0200 Subject: [PATCH 2/8] Fix unit-test for Postgresql.controldata() --- tests/test_postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 83e93ec0..70446533 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -346,7 +346,7 @@ class TestPostgresql(unittest.TestCase): self.p.remove_data_directory() @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string)) - @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError) + @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError(1, '')) @patch('subprocess.check_output', side_effect=Exception('Failed')) def test_controldata(self, check_output_call_error, check_output_generic_exception): data = self.p.controldata() From e0e4789b8a1d96e94627a050b17f57729fe64b13 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 21 Oct 2015 15:49:20 +0200 Subject: [PATCH 3/8] Explicitly cast scope to string. Fixes issue #74 --- patroni/dcs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index bb56fecc..cd1a6512 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -123,7 +123,7 @@ class AbstractDCS: """ self._name = name self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/')) - self._base_path = '/'.join([self._namespace, config['scope']]) + self._base_path = '/'.join([self._namespace, str(config['scope']])) self._cluster = None self._cluster_thread_lock = Lock() From c751dfdebfe3c044c3b22790c98eaa04722a16e2 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 22 Oct 2015 08:50:53 +0200 Subject: [PATCH 4/8] Typo in joining namespace to scope --- patroni/dcs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index cd1a6512..f406456d 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -123,7 +123,7 @@ class AbstractDCS: """ self._name = name self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/')) - self._base_path = '/'.join([self._namespace, str(config['scope']])) + self._base_path = '/'.join([self._namespace, str(config['scope'])]) self._cluster = None self._cluster_thread_lock = Lock() From 857caa13977273bc1a6677d14234e8a67b3acc4b Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 22 Oct 2015 09:24:31 +0200 Subject: [PATCH 5/8] Revert casting to string --- patroni/dcs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index f406456d..bb56fecc 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -123,7 +123,7 @@ class AbstractDCS: """ self._name = name self._namespace = '/{}'.format(config.get('namespace', '/service/').strip('/')) - self._base_path = '/'.join([self._namespace, str(config['scope'])]) + self._base_path = '/'.join([self._namespace, config['scope']]) self._cluster = None self._cluster_thread_lock = Lock() From eaf63db886da9c2930b54e546547bfc6a831bc4b Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 22 Oct 2015 09:28:00 +0200 Subject: [PATCH 6/8] Use a different namespace in the Docker container. Also bugfix: Patroni should advertise Docker ip as connect address --- docker/entrypoint.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index d94757b3..7afdf9c5 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -83,10 +83,11 @@ cat > /patroni/postgres.yml <<__EOF__ ttl: &ttl 30 loop_wait: &loop_wait 10 -scope: &scope ${PATRONI_SCOPE} +scope: &scope '${PATRONI_SCOPE}' +namespace: 'patroni' restapi: - listen: 127.0.0.1:8008 - connect_address: 127.0.0.1:8008 + listen: 0.0.0.0:8008 + connect_address: ${DOCKER_IP}:8008 etcd: scope: *scope ttl: *ttl From 0c5a21e57d106f42028f41ea906b5091c17ed37b Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 23 Oct 2015 10:46:55 +0200 Subject: [PATCH 7/8] Fix removal of keys on failed initialization. The initialize key was checked against the value of the node name before removal, but it was changed recently to contain either an empty string, or cluster sysid. To fix this, the check for the previous value was simply removed: we can guarantee that the code path that removes the key is the one that created it. --- patroni/etcd.py | 2 +- patroni/zookeeper.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patroni/etcd.py b/patroni/etcd.py index d84f5a50..4a82f2d7 100644 --- a/patroni/etcd.py +++ b/patroni/etcd.py @@ -249,7 +249,7 @@ class Etcd(AbstractDCS): @catch_etcd_errors def cancel_initialization(self): - return self.retry(self.client.delete, self.initialize_path, prevValue=self._name) + return self.retry(self.client.delete, self.initialize_path) def watch(self, timeout): cluster = self.cluster diff --git a/patroni/zookeeper.py b/patroni/zookeeper.py index 1fa2cca3..bc9b83c4 100644 --- a/patroni/zookeeper.py +++ b/patroni/zookeeper.py @@ -271,7 +271,7 @@ class ZooKeeper(AbstractDCS): def _cancel_initialization(self): node = self.get_node(self.initialize_path) - if node and node[0] == self._name: + if node: self.client.delete(self.initialize_path, version=node[1].version) def cancel_initialization(self): From 553129a981d84a0f96bc83337002e44c8255f7ed Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 23 Oct 2015 15:59:20 +0200 Subject: [PATCH 8/8] Revert "Fix unit-test for Postgresql.controldata()" This reverts commit deaaf8ad1aa1ecae2c5bef9768c3e647a447b647. --- tests/test_postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 9eb7c1d3..0ed04a15 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -354,7 +354,7 @@ class TestPostgresql(unittest.TestCase): self.p.remove_data_directory() @patch('subprocess.check_output', MagicMock(return_value=0, side_effect=pg_controldata_string)) - @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError(1, '')) + @patch('subprocess.check_output', side_effect=subprocess.CalledProcessError) @patch('subprocess.check_output', side_effect=Exception('Failed')) def test_controldata(self, check_output_call_error, check_output_generic_exception): data = self.p.controldata()