From 65415824d01299f2f5cd233a154a913eb6dee01b Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Wed, 6 Mar 2019 15:20:37 +0100 Subject: [PATCH] Avoid duplicate ``create-pool`` requests for same pool Change-Id: I801843f81d5fbe9838142b8fba96d93bbdfc91e4 --- requires.py | 33 +++++++++------------ unit_tests/test_requires.py | 57 +++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/requires.py b/requires.py index e519be3..e2c48da 100644 --- a/requires.py +++ b/requires.py @@ -13,7 +13,6 @@ # limitations under the License. import ipaddress -import json import socket # the reactive framework unfortunately does not grok `import as` in conjunction @@ -75,22 +74,6 @@ class CephRBDMirrorRequires(Endpoint): for relation in self.relations: relation.to_publish['unique_id'] = self.unique_id - def get_current_request(self): - """ - Retrieve the current Ceph broker request. - - If no request has been created yet then create a new one. - """ - json_rq = self.all_joined_units.received['broker_req'] - current_request = ch_ceph.CephBrokerRq() - if json_rq: - try: - j = json.loads(json_rq) - current_request.set_ops(j['ops']) - except (KeyError, json.decoder.JSONDecodeError): - raise - return current_request - def create_replicated_pool(self, name, replicas=3, weight=None, pg_num=None, group=None, namespace=None, app_name=None, max_bytes=None, @@ -104,7 +87,13 @@ class CephRBDMirrorRequires(Endpoint): max_bytes = int(max_bytes) if max_bytes else None max_objects = int(max_objects) if max_objects else None - current_request = self.get_current_request() + current_request = ch_ceph.get_previous_request( + self.relations[0].relation_id) or ch_ceph.CephBrokerRq() + for req in current_request.ops: + if 'op' in req and 'name' in req: + if req['op'] == 'create-pool' and req['name'] == name: + # request already exists, don't create a new one + return current_request.add_op_create_replicated_pool( name="{}".format(name), replica_count=replicas, @@ -128,7 +117,13 @@ class CephRBDMirrorRequires(Endpoint): max_bytes = int(max_bytes) if max_bytes else None max_objects = int(max_objects) if max_objects else None - current_request = self.get_current_request() + current_request = ch_ceph.get_previous_request( + self.relations[0].relation_id) or ch_ceph.CephBrokerRq() + for req in current_request.ops: + if 'op' in req and 'name' in req: + if req['op'] == 'create-pool' and req['name'] == name: + # request already exists, don't create a new one + return current_request.add_op_create_erasure_pool( name="{}".format(name), erasure_profile=erasure_profile, diff --git a/unit_tests/test_requires.py b/unit_tests/test_requires.py index 2976eb9..c6348cd 100644 --- a/unit_tests/test_requires.py +++ b/unit_tests/test_requires.py @@ -109,6 +109,63 @@ class TestCephRBDMirrorRequires(test_utils.PatchHelper): self.requires_class.request_key() to_publish.__setitem__.assert_called_with('unique_id', 'some-hostname') + def test_create_replicated_pool(self): + self.patch_requires_class('_relations') + relation = mock.MagicMock() + relation.relation_id = 'some-endpoint:42' + self._relations.__iter__.return_value = [relation] + self.patch_object(requires.ch_ceph, 'get_previous_request') + broker_req = mock.MagicMock() + broker_req.ops = [{'op': 'create-pool', 'name': 'rbd'}] + self.get_previous_request.return_value = broker_req + self.requires_class.create_replicated_pool('rbd') + self.assertFalse(broker_req.add_op_create_replicated_pool.called) + self.get_previous_request.return_value = None + self.patch_object(requires.ch_ceph, 'CephBrokerRq') + self.CephBrokerRq.return_value = broker_req + self.requires_class.create_replicated_pool('rbd') + self.CephBrokerRq.assert_called_with() + self.assertFalse(broker_req.add_op_create_replicated_pool.called) + broker_req = mock.MagicMock() + self.CephBrokerRq.return_value = broker_req + self.patch_object(requires.ch_ceph, 'send_request_if_needed') + self.requires_class.create_replicated_pool('rbd') + broker_req.add_op_create_replicated_pool.assert_called_once_with( + app_name=None, group=None, max_bytes=None, max_objects=None, + name='rbd', namespace=None, pg_num=None, replica_count=3, + weight=None) + self.send_request_if_needed.assert_called_once_with( + broker_req, + relation='some-endpoint') + + def test_create_erasure_pool(self): + self.patch_requires_class('_relations') + relation = mock.MagicMock() + relation.relation_id = 'some-endpoint:42' + self._relations.__iter__.return_value = [relation] + self.patch_object(requires.ch_ceph, 'get_previous_request') + broker_req = mock.MagicMock() + broker_req.ops = [{'op': 'create-pool', 'name': 'rbd'}] + self.get_previous_request.return_value = broker_req + self.requires_class.create_erasure_pool('rbd') + self.assertFalse(broker_req.add_op_create_erasure_pool.called) + self.get_previous_request.return_value = None + self.patch_object(requires.ch_ceph, 'CephBrokerRq') + self.CephBrokerRq.return_value = broker_req + self.requires_class.create_erasure_pool('rbd') + self.CephBrokerRq.assert_called_with() + self.assertFalse(broker_req.add_op_create_erasure_pool.called) + broker_req = mock.MagicMock() + self.CephBrokerRq.return_value = broker_req + self.patch_object(requires.ch_ceph, 'send_request_if_needed') + self.requires_class.create_erasure_pool('rbd') + broker_req.add_op_create_erasure_pool.assert_called_once_with( + app_name=None, erasure_profile=None, group=None, max_bytes=None, + max_objects=None, name='rbd', weight=None) + self.send_request_if_needed.assert_called_once_with( + broker_req, + relation='some-endpoint') + def test_mon_hosts(self): self.patch_requires_class('_relations') relation = mock.MagicMock()