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

.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 request_access_to_group(self, name, namespace=None, permission=None,
                                 key_name=None, object_prefix_permissions=None):

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}])
 
     def test_request_access_to_group_new_request(self):
         self.patch_kr('get_local', '{"ops": []}')