commit b143db91dee48765f43803e135d305dbc8dcca74 Author: Frode Nordahl Date: Mon Sep 21 11:27:20 2020 +0200 Allow passing keyword arguments to create_*_pool The CephBrokerRq class will be used to validate the arguments passed so that the caller will get immediate feedback if any unknown arguments or invalid values are used. Change-Id: Ideb142ead729977dedd2c00435e98846fc842da3 diff --git a/src/lib/base_requires.py b/src/lib/base_requires.py index 42bc1b6..c442c85 100644 --- a/src/lib/base_requires.py +++ b/src/lib/base_requires.py @@ -78,33 +78,47 @@ class CephRequires(reactive.Endpoint): def create_replicated_pool(self, name, replicas=3, weight=None, pg_num=None, group=None, namespace=None, - app_name=None): + app_name=None, **kwargs): """ Request pool setup - @param name: Name of pool to create - @param replicas: Number of replicas for supporting pools - @param weight: The percentage of data the pool makes up - @param pg_num: If not provided, this value will be calculated by the + :param name: Name of pool to create + :type name: str + :param replicas: Number of replicas for supporting pools + :type replicas: int + :param weight: The percentage of data the pool makes up + :type weight: Optional[float] + :param pg_num: If not provided, this value will be calculated by the broker based on how many OSDs are in the cluster at the time of creation. Note that, if provided, this value will be capped at the current available maximum. - @param group: Group to add pool to. - @param namespace: A group can optionally have a namespace defined that + :type pg_num: Optional[int] + :param group: Group to add pool to. + :type group: Optional[str] + :param namespace: A group can optionally have a namespace defined that will be used to further restrict pool access. - @param app_name: (Optional) Tag pool with application name. Note that + :type namespace: Optional[str] + :param app_name: (Optional) Tag pool with application name. Note that there is certain protocols emerging upstream with regard to meaningful application names to use. Examples are ``rbd`` and ``rgw``. + :type app_name: Optional[str] + :param kwargs: Additional keyword arguments subject to validation. + Refer to CephBrokerRq.add_op_create_replicated_pool + method for documentation. + :type kwargs: Dict[str,any] """ rq = self.get_current_request() or CephBrokerRq() - rq.add_op_create_replicated_pool(name=name, - replica_count=replicas, - pg_num=pg_num, - weight=weight, - group=group, - namespace=namespace, - app_name=app_name) + kwargs.update({ + 'name': name, + 'replica_count': replicas, + 'pg_num': pg_num, + 'weight': weight, + 'group': group, + 'namespace': namespace, + 'app_name': app_name, + }) + rq.add_op_create_replicated_pool(**kwargs) self.send_request_if_needed(rq) reactive.clear_flag( self.expand_name('{endpoint_name}.pools.available')) @@ -132,28 +146,44 @@ class CephRequires(reactive.Endpoint): def create_erasure_pool(self, name, erasure_profile=None, weight=None, group=None, app_name=None, max_bytes=None, max_objects=None, - allow_ec_overwrites=False): + allow_ec_overwrites=False, + **kwargs): """ Request erasure coded pool setup - @param name: Name of pool to create - @param erasure_profile: Name of erasure profile for pool - @param weight: The percentage of data the pool makes up - @param group: Group to add pool to. - @param app_name: Name of application using pool - @param max_bytes: Maximum bytes of quota to apply - @param max_objects: Maximum object quota to apply - @param allow_ec_overwrites: Allow EC pools to be overwritten + :param name: Name of pool to create + :type name: str + :param erasure_profile: Name of erasure profile for pool + :type erasure_profile: str + :param weight: The percentage of data the pool makes up + :type weight: Optional[float] + :param group: Group to add pool to. + :type group: Optional[str] + :param app_name: Name of application using pool + :type app_name: Optional[str] + :param max_bytes: Maximum bytes of quota to apply + :type max_bytes: Optional[int] + :param max_objects: Maximum object quota to apply + :type max_objects: Optional[int] + :param allow_ec_overwrites: Allow EC pools to be overwritten + :type allow_ec_overwrites: bool + :param kwargs: Additional keyword arguments subject to validation. + Refer to CephBrokerRq.add_op_create_replicated_pool + method for documentation. + :type kwargs: Dict[str,any] """ rq = self.get_current_request() or CephBrokerRq() - rq.add_op_create_erasure_pool(name=name, - erasure_profile=erasure_profile, - weight=weight, - group=group, - app_name=app_name, - max_bytes=max_bytes, - max_objects=max_objects, - allow_ec_overwrites=allow_ec_overwrites) + kwargs.update({ + 'name': name, + 'erasure_profile': erasure_profile, + 'weight': weight, + 'group': group, + 'app_name': app_name, + 'max_bytes': max_bytes, + 'max_objects': max_objects, + 'allow_ec_overwrites': allow_ec_overwrites, + }) + rq.add_op_create_erasure_pool(**kwargs) self.send_request_if_needed(rq) reactive.clear_flag( self.expand_name('{endpoint_name}.pools.available')) diff --git a/unit_tests/test_requires.py b/unit_tests/test_requires.py index 34da223..b975e80 100644 --- a/unit_tests/test_requires.py +++ b/unit_tests/test_requires.py @@ -22,8 +22,6 @@ with mock.patch('charmhelpers.core.hookenv.metadata') as _meta: _meta.return_Value = 'ss' from ceph_client import requires -import charmhelpers - _hook_args = {} @@ -189,55 +187,33 @@ class TestCephClientRequires(unittest.TestCase): mock.call('some-relation.connected'), mock.call('some-relation.pools.available')]) - @mock.patch.object(charmhelpers.contrib.storage.linux.ceph.uuid, 'uuid1') - def test_create_pool_new_request(self, _uuid1): - self.patch_kr('get_current_request', None) - self.patch_kr('send_request_if_needed') - _uuid1.return_value = '9e34123e-fa0c-11e8-ad9c-fa163ed1cc55' - 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, - '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): + def test_create_replicated_pool(self): + self.patch_kr('get_current_request') + self.patch_object(requires.base_requires, 'CephBrokerRq') self.patch_kr('send_request_if_needed') - _uuid1.return_value = '9e34123e-fa0c-11e8-ad9c-fa163ed1cc55' - req = ( - '{"api-version": 1, ' - '"ops": [{"op": "create-pool", "name": "bob", "replicas": 3, ' - '"pg_num": null, "weight": null, "group": null, ' - '"group-namespace": null, "app-name": null, "max-bytes": null, ' - '"max-objects": null}], ' - '"request-id": "9e34123e-fa0c-11e8-ad9c-fa163ed1cc55"}') - existing_request = DummyRequest(req_json=req) - self.patch_kr('get_current_request', existing_request) - 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, - 'max-bytes': None, - 'max-objects': None, - 'app-name': None, - 'weight': None}]) + self.get_current_request.return_value = None + rq = mock.MagicMock() + self.CephBrokerRq.return_value = rq + self.cr.create_replicated_pool('bob') + rq.add_op_create_replicated_pool.assert_called_once_with( + name='bob', + replica_count=3, + pg_num=None, + weight=None, + group=None, + namespace=None, + app_name=None) + existing_request = mock.MagicMock() + self.get_current_request.return_value = existing_request + self.cr.create_replicated_pool('bob') + existing_request.add_op_create_replicated_pool.assert_called_once_with( + name='bob', + replica_count=3, + pg_num=None, + weight=None, + group=None, + namespace=None, + app_name=None) def test_request_access_to_group_new_request(self): self.patch_kr('send_request_if_needed')