Revert "Refactor create_pool." and "Add request_access_to_group method"

This reverts commit e4997e5ab8 and
288bab66dd.

This is necessary as this change does not verify that there is not an
existing request to do this in the queue, and will end up creating
infinite create_pool requests on the ceph-mon

Change-Id: I76305181ddba83eca414d9e08bbd58408d2b01ce

.gitignore

@@ -2,6 +2,3 @@
 *.swp
 .testrepository
 .tox
-.stestr
-.unit-state.db
-__pycache__

requires.py

@@ -67,24 +67,6 @@ class CephClientRequires(RelationBase):
         self.remove_state('{relation_name}.connected')
         self.remove_state('{relation_name}.pools.available')
 
-    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.get_local(key='broker_req')
-        current_request = CephBrokerRq()
-        if json_rq:
-            try:
-                j = json.loads(json_rq)
-                current_request.set_ops(j['ops'])
-            except (KeyError, json.decoder.JSONDecodeError) as err:
-                log("Unable to decode broker_req: {}.  Error: {}".format(
-                    json_rq, err))
-                raise
-        return current_request
-
     def create_pool(self, name, replicas=3, weight=None, pg_num=None,
                     group=None, namespace=None):
         """
@@ -102,16 +84,29 @@ class CephClientRequires(RelationBase):
                           will be used to further restrict pool access.
         """
 
-        current_request = self.get_current_request()
-        current_request.add_op_create_pool(
-            name="{}".format(name),
-            replica_count=replicas,
-            pg_num=pg_num,
-            weight=weight,
-            group=group,
-            namespace=namespace)
-        self.set_local(key='broker_req', value=current_request.request)
-        send_request_if_needed(current_request, relation=self.relation_name)
+        # json.dumps of the CephBrokerRq()
+        json_rq = self.get_local(key='broker_req')
+
+        if not json_rq:
+            rq = CephBrokerRq()
+            rq.add_op_create_pool(name="{}".format(name),
+                                  replica_count=replicas,
+                                  pg_num=pg_num,
+                                  weight=weight,
+                                  group=group,
+                                  namespace=namespace)
+            self.set_local(key='broker_req', value=rq.request)
+            send_request_if_needed(rq, relation=self.relation_name)
+        else:
+            rq = CephBrokerRq()
+            try:
+                j = json.loads(json_rq)
+                log("Json request: {}".format(json_rq))
+                rq.ops = j['ops']
+                send_request_if_needed(rq, relation=self.relation_name)
+            except ValueError as err:
+                log("Unable to decode broker_req: {}.  Error: {}".format(
+                    json_rq, err))
 
     def get_remote_all(self, key, default=None):
         """Return a list of all values presented by remote units for key"""

unit_tests/test_requires.py

@@ -11,9 +11,8 @@
 # limitations under the License.
 
 
-import json
-import mock
 import unittest
+import mock
 
 
 with mock.patch('charmhelpers.core.hookenv.metadata') as _meta:
@@ -172,41 +171,6 @@ class TestCephClientRequires(unittest.TestCase):
             mock.call('{relation_name}.connected'),
             mock.call('{relation_name}.pools.available')])
 
-    def test_get_current_request_new(self):
-        self.patch_kr('get_local', None)
-        req = self.cr.get_current_request()
-        self.assertEqual(req.ops, [])
-
-    def test_get_current_request_existing(self):
-        req = (
-            '{"api-version": 1, '
-            '"ops": [{"op": "create-pool", "name": "bob", "replicas": 3, '
-            '"pg_num": null, "weight": null, "group": null, '
-            '"group-namespace": null}], '
-            '"request-id": "9e34123e-fa0c-11e8-ad9c-fa163ed1cc55"}')
-        self.patch_kr('get_local', req)
-        req = self.cr.get_current_request()
-        self.assertEqual(
-            req.ops,
-            [{
-                'op': 'create-pool',
-                'name': 'bob',
-                'replicas': 3,
-                'group': None,
-                'group-namespace': None,
-                'pg_num': None,
-                'weight': None}])
-
-    def test_get_current_request_not_json(self):
-        self.patch_kr('get_local', '{I am not json')
-        with self.assertRaises(json.decoder.JSONDecodeError):
-            self.cr.get_current_request()
-
-    def test_get_current_request_missing_ops(self):
-        self.patch_kr('get_local', '{}')
-        with self.assertRaises(KeyError):
-            self.cr.get_current_request()
-
     @mock.patch.object(charmhelpers.contrib.storage.linux.ceph.uuid, 'uuid1')
     def test_create_pool_new_request(self, _uuid1):
         _uuid1.return_value = '9e34123e-fa0c-11e8-ad9c-fa163ed1cc55'
@@ -214,7 +178,8 @@ class TestCephClientRequires(unittest.TestCase):
             '{"api-version": 1, '
             '"ops": [{"op": "create-pool", "name": "bob", "replicas": 3, '
             '"pg_num": null, "weight": null, "group": null, '
-            '"group-namespace": null}], '
+            '"group-namespace": null, "app-name": null, "max-bytes": null, '
+            '"max-objects": null}], '
             '"request-id": "9e34123e-fa0c-11e8-ad9c-fa163ed1cc55"}')
         self.patch_kr('get_local', None)
         self.patch_kr('set_local')
@@ -230,7 +195,10 @@ class TestCephClientRequires(unittest.TestCase):
                 'group': None,
                 'group-namespace': None,
                 'pg_num': None,
-                'weight': None}])
+                'weight': None,
+                'app-name': None,
+                'max-bytes': None,
+                'max-objects': None}])
 
     @mock.patch.object(charmhelpers.contrib.storage.linux.ceph.uuid, 'uuid1')
     def test_create_pool_existing_request(self, _uuid1):
@@ -242,27 +210,18 @@ class TestCephClientRequires(unittest.TestCase):
             '"group-namespace": null}], '
             '"request-id": "9e34123e-fa0c-11e8-ad9c-fa163ed1cc55"}')
         self.patch_kr('get_local', req)
-        self.cr.create_pool('bill')
+        self.cr.create_pool('bob')
         ceph_broker_rq = self.send_request_if_needed.mock_calls[0][1][0]
         self.assertEqual(
             ceph_broker_rq.ops,
-            [
-                {
-                    'op': 'create-pool',
-                    'name': 'bob',
-                    'replicas': 3,
-                    'group': None,
-                    'group-namespace': None,
-                    'pg_num': None,
-                    'weight': None},
-                {
-                    'op': 'create-pool',
-                    'name': 'bill',
-                    'replicas': 3,
-                    'group': None,
-                    'group-namespace': None,
-                    'pg_num': None,
-                    'weight': None}])
+            [{
+                'op': 'create-pool',
+                'name': 'bob',
+                'replicas': 3,
+                'group': None,
+                'group-namespace': None,
+                'pg_num': None,
+                'weight': None}])
 
     @mock.patch.object(requires.hookenv, 'related_units')
     @mock.patch.object(requires.hookenv, 'relation_get')