Avoid duplicate ``create-pool`` requests for same pool

Change-Id: I801843f81d5fbe9838142b8fba96d93bbdfc91e4

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,

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()