From 8c418e930dad215f6a3a40bbc51a57ec14a0f999 Mon Sep 17 00:00:00 2001 From: Kiran Pawar Date: Mon, 29 Jul 2024 14:40:18 +0000 Subject: [PATCH] Pass share metadata updates to backend drivers New config option `driver_updatable_metadata` contains list of metadata keys. Share metadata if updated will be passed to backend driver if key is present in above list. Driver then can take action if supported and result will be updated in message. Implements: blueprint pass-resource-metadata-updates-to-backend-drivers Change-Id: If4297cca3249359f72976800db2112ea9c61c06f --- manila/api/v2/shares.py | 21 +++++- manila/common/config.py | 6 ++ manila/message/message_field.py | 12 ++++ manila/share/api.py | 46 +++++++++++++ manila/share/driver.py | 13 ++++ manila/share/manager.py | 24 ++++++- manila/share/rpcapi.py | 11 +++- manila/tests/api/v2/test_shares.py | 47 ++++++++++++++ manila/tests/share/test_api.py | 64 +++++++++++++++++++ manila/tests/share/test_rpcapi.py | 7 ++ ...s-to-backend-drivers-7fff302f64fda2d7.yaml | 11 ++++ 11 files changed, 257 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/bp-pass-resource-metadata-updates-to-backend-drivers-7fff302f64fda2d7.yaml diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index 4d4b9ca0bd..1234db5af2 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -671,7 +671,12 @@ class ShareController(wsgi.Controller, body['metadata'], delete=False) body['metadata'] = _metadata - return self._create_metadata(req, resource_id, body) + metadata = self._create_metadata(req, resource_id, body) + + context = req.environ['manila.context'] + self.share_api.update_share_from_metadata(context, resource_id, + metadata.get('metadata')) + return metadata @wsgi.Controller.api_version("2.0") @wsgi.Controller.authorize("update_share_metadata") @@ -682,7 +687,12 @@ class ShareController(wsgi.Controller, _metadata = self._validate_metadata_for_update(req, resource_id, body['metadata']) body['metadata'] = _metadata - return self._update_all_metadata(req, resource_id, body) + metadata = self._update_all_metadata(req, resource_id, body) + + context = req.environ['manila.context'] + self.share_api.update_share_from_metadata(context, resource_id, + metadata.get('metadata')) + return metadata @wsgi.Controller.api_version("2.0") @wsgi.Controller.authorize("update_share_metadata") @@ -694,7 +704,12 @@ class ShareController(wsgi.Controller, body['metadata'], delete=False) body['metadata'] = _metadata - return self._update_metadata_item(req, resource_id, body, key) + metadata = self._update_metadata_item(req, resource_id, body, key) + + context = req.environ['manila.context'] + self.share_api.update_share_from_metadata(context, resource_id, + metadata.get('metadata')) + return metadata @wsgi.Controller.api_version("2.0") @wsgi.Controller.authorize("get_share_metadata") diff --git a/manila/common/config.py b/manila/common/config.py index a52bfd07da..75a0300126 100644 --- a/manila/common/config.py +++ b/manila/common/config.py @@ -141,6 +141,12 @@ global_opts = [ default=constants.AdminOnlyMetadata.SCHEDULER_FILTERS, help='Metadata keys that should only be manipulated by ' 'administrators.'), + cfg.ListOpt('driver_updatable_metadata', + default=[], + help='Metadata keys that will decide which share metadata ' + '(element of the list is , ' + 'i.e max_files) can be passed to share drivers as part ' + 'of metadata create/update operations.'), ] CONF.register_opts(global_opts) diff --git a/manila/message/message_field.py b/manila/message/message_field.py index 5faf7c168b..2b433a2a48 100644 --- a/manila/message/message_field.py +++ b/manila/message/message_field.py @@ -37,6 +37,7 @@ class Action(object): UPDATE_ACCESS_RULES = ('010', _('update access rules')) ADD_UPDATE_SECURITY_SERVICE = ('011', _('add or update security service')) TRANSFER_ACCEPT = ('026', _('transfer accept')) + UPDATE_METADATA = ('027', _('update_metadata')) ALL = ( ALLOCATE_HOST, CREATE, @@ -50,6 +51,7 @@ class Action(object): UPDATE_ACCESS_RULES, ADD_UPDATE_SECURITY_SERVICE, TRANSFER_ACCEPT, + UPDATE_METADATA, ) @@ -154,6 +156,14 @@ class Detail(object): "request. Share back end services are not " "ready yet. Contact your administrator in case " "retrying does not help.")) + UPDATE_METADATA_SUCCESS = ( + '029', + _("Metadata passed to share driver successfully performed required " + "operation.")) + UPDATE_METADATA_FAILURE = ( + '030', + _("Metadata passed to share driver failed to perform required " + "operation.")) ALL = ( UNKNOWN_ERROR, @@ -184,6 +194,8 @@ class Detail(object): DRIVER_FAILED_TRANSFER_ACCEPT, SHARE_NETWORK_PORT_QUOTA_LIMIT_EXCEEDED, SHARE_BACKEND_NOT_READY_YET, + UPDATE_METADATA_SUCCESS, + UPDATE_METADATA_FAILURE, ) # Exception and detail mappings diff --git a/manila/share/api.py b/manila/share/api.py index 3a89a2f771..9f5bf63efd 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -427,6 +427,10 @@ class API(base.Base): "(%(group)s).") % params raise exception.InvalidParameterValue(msg) + if share_type: + metadata = self.update_metadata_from_share_type_extra_specs( + context, share_type, metadata) + options = { 'size': size, 'user_id': context.user_id, @@ -515,6 +519,48 @@ class API(base.Base): return share + def update_metadata_from_share_type_extra_specs(self, context, share_type, + user_metadata): + extra_specs = share_type.get('extra_specs', {}) + if not extra_specs: + return user_metadata + + driver_keys = getattr(CONF, 'driver_updatable_metadata', []) + if not driver_keys: + return user_metadata + + metadata_from_share_type = {} + for k, v in extra_specs.items(): + try: + prefix, metadata_key = k.split(':') + except Exception: + continue + + # consider prefix only with valid storage driver + if prefix.lower() == 'provisioning': + continue + + if metadata_key in driver_keys: + metadata_from_share_type.update({metadata_key: v}) + + metadata_from_share_type.update(user_metadata) + return metadata_from_share_type + + def update_share_from_metadata(self, context, share_id, metadata): + driver_keys = getattr(CONF, 'driver_updatable_metadata', []) + if not driver_keys: + return + + driver_metadata = {} + for k, v in metadata.items(): + if k in driver_keys: + driver_metadata.update({k: v}) + + if driver_metadata: + share = self.get(context, share_id) + self.share_rpcapi.update_share_from_metadata(context, share, + driver_metadata) + def get_share_attributes_from_share_type(self, share_type): """Determine share attributes from the share type. diff --git a/manila/share/driver.py b/manila/share/driver.py index 5bfee9fc0c..8b4fed96c1 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -3732,3 +3732,16 @@ class ShareDriver(object): :param share_server: share server in case of dhss_true """ raise NotImplementedError() + + def update_share_from_metadata(self, context, share, metadata): + """Update the share from metadata. + + Driver must implement this method if needs to perform some action + on given resource (i.e. share) based on provided metadata. + + :param context: The 'context.RequestContext' object for the request. + :param share: Share instance model with share data. + :param metadata: Dict contains key-value pair where driver will + perform necessary action based on key. + """ + raise NotImplementedError() diff --git a/manila/share/manager.py b/manila/share/manager.py index 6bef6a4bed..7fabfd694f 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -264,7 +264,7 @@ def add_hooks(f): class ShareManager(manager.SchedulerDependentManager): """Manages NAS storages.""" - RPC_API_VERSION = '1.27' + RPC_API_VERSION = '1.28' def __init__(self, share_driver=None, service_name=None, *args, **kwargs): """Load the driver from args, or from flags.""" @@ -6755,3 +6755,25 @@ class ShareManager(manager.SchedulerDependentManager): # order to properly update share network status. self._check_share_network_update_finished( context, share_network_id=share_network['id']) + + def update_share_from_metadata(self, context, share_id, metadata): + share = self.db.share_get(context, share_id) + share_instance = self._get_share_instance(context, share) + try: + self.driver.update_share_from_metadata(context, share_instance, + metadata) + self.message_api.create( + context, + message_field.Action.UPDATE_METADATA, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share_id, + detail=message_field.Detail.UPDATE_METADATA_SUCCESS) + except Exception: + self.message_api.create( + context, + message_field.Action.UPDATE_METADATA, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share_id, + detail=message_field.Detail.UPDATE_METADATA_FAILURE) diff --git a/manila/share/rpcapi.py b/manila/share/rpcapi.py index 93a02b46c3..c842fd2fbc 100644 --- a/manila/share/rpcapi.py +++ b/manila/share/rpcapi.py @@ -88,6 +88,7 @@ class ShareAPI(object): 1.26 - Add create_backup() and delete_backup() restore_backup() methods 1.27 - Update delete_share_instance() and delete_snapshot() methods + 1.28 - Add update_share_from_metadata() method """ BASE_RPC_API_VERSION = '1.0' @@ -96,7 +97,7 @@ class ShareAPI(object): super(ShareAPI, self).__init__() target = messaging.Target(topic=CONF.share_topic, version=self.BASE_RPC_API_VERSION) - self.client = rpc.get_client(target, version_cap='1.27') + self.client = rpc.get_client(target, version_cap='1.28') def create_share_instance(self, context, share_instance, host, request_spec, filter_properties, @@ -531,3 +532,11 @@ class ShareAPI(object): 'restore_backup', backup=backup, share_id=share_id) + + def update_share_from_metadata(self, context, share, metadata): + host = utils.extract_host(share['instance']['host']) + call_context = self.client.prepare(server=host, version='1.28') + return call_context.cast(context, + 'update_share_from_metadata', + share_id=share['id'], + metadata=metadata) diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index f13ce4e456..38265bbb9b 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -2251,6 +2251,53 @@ class ShareAPITest(test.TestCase): common.remove_invalid_options(ctx, search_opts, allowed_opts) self.assertEqual(expected_opts, search_opts) + def test_create_metadata(self): + id = 'fake_share_id' + body = {'metadata': {'key1': 'val1', 'key2': 'val2'}} + mock_validate = self.mock_object( + self.controller, '_validate_metadata_for_update', + mock.Mock(return_value=body['metadata'])) + mock_create = self.mock_object( + self.controller, '_create_metadata', + mock.Mock(return_value=body)) + self.mock_object(share_api.API, 'update_share_from_metadata') + + req = fakes.HTTPRequest.blank( + '/v2/shares/%s/metadata' % id) + + res = self.controller.create_metadata(req, id, body) + self.assertEqual(body, res) + mock_validate.assert_called_once_with(req, id, body['metadata'], + delete=False) + mock_create.assert_called_once_with(req, id, body) + + def test_update_all_metadata(self): + id = 'fake_share_id' + body = {'metadata': {'key1': 'val1', 'key2': 'val2'}} + mock_validate = self.mock_object( + self.controller, '_validate_metadata_for_update', + mock.Mock(return_value=body['metadata'])) + mock_update = self.mock_object( + self.controller, '_update_all_metadata', + mock.Mock(return_value=body)) + self.mock_object(share_api.API, 'update_share_from_metadata') + + req = fakes.HTTPRequest.blank( + '/v2/shares/%s/metadata' % id) + res = self.controller.update_all_metadata(req, id, body) + self.assertEqual(body, res) + mock_validate.assert_called_once_with(req, id, body['metadata']) + mock_update.assert_called_once_with(req, id, body) + + def test_delete_metadata(self): + mock_delete = self.mock_object( + self.controller, '_delete_metadata', mock.Mock()) + + req = fakes.HTTPRequest.blank( + '/v2/shares/%s/metadata/fake_key' % id) + self.controller.delete_metadata(req, id, 'fake_key') + mock_delete.assert_called_once_with(req, id, 'fake_key') + def _fake_access_get(self, ctxt, access_id): diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 1df56ec1e8..f7ea02f3e1 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -649,6 +649,70 @@ class ShareAPITestCase(test.TestCase): def test_get_all_filter_by_invalid_extra_specs(self): self._get_all_filter_metadata_or_extra_specs_invalid(key='extra_specs') + @ddt.data(True, False) + def test_update_metadata_from_share_type_extra_specs(self, with_metadata): + share_type = fakes.fake_share_type( + extra_specs={ + 'driver_handles_share_servers': 'False', + 'fake_driver:dedupe': 'True', + 'fake_driver:encrypt': 'True', + 'fake_driver:snapshot_policy': 'daily', + 'provisioning:max_share_size': '10', + } + ) + + user_metadata = {} + if with_metadata: + user_metadata = { + 'snapshot_policy': 'monthly', + 'tag': 't1', + 'max_share_size': '5', + } + + CONF.set_default( + "driver_updatable_metadata", + ['dedupe', 'snapshot_policy', 'thin_provisioning'], + ) + + result = self.api.update_metadata_from_share_type_extra_specs( + self.context, + share_type, + user_metadata + ) + + if with_metadata: + self.assertEqual( + result, + {'dedupe': 'True', 'snapshot_policy': 'monthly', 'tag': 't1', + 'max_share_size': '5'}) + else: + self.assertEqual( + result, + {'dedupe': 'True', 'snapshot_policy': 'daily'}) + + def test_update_share_from_metadata(self): + CONF.set_default( + "driver_updatable_metadata", + ['dedupe', 'snapshot_policy', 'thin_provisioning'], + ) + metadata = { + 'dedupe': 'True', + 'snapshot_policy': 'monthly', + 'max_share_size': '10' + } + backend_metadata = { + k: v for k, v in metadata.items() if k != 'max_share_size'} + + self.mock_object(self.api, 'get', mock.Mock(return_value='fake_share')) + mock_call = self.mock_object( + self.api.share_rpcapi, + 'update_share_from_metadata' + ) + + self.api.update_share_from_metadata(self.context, 'fake_id', metadata) + mock_call.assert_called_once_with( + self.context, 'fake_share', backend_metadata) + @ddt.data(True, False) def test_create_public_and_private_share(self, is_public): share, share_data = self._setup_create_mocks(is_public=is_public) diff --git a/manila/tests/share/test_rpcapi.py b/manila/tests/share/test_rpcapi.py index e1f3013dad..cef55344ad 100644 --- a/manila/tests/share/test_rpcapi.py +++ b/manila/tests/share/test_rpcapi.py @@ -391,6 +391,13 @@ class ShareRpcAPITestCase(test.TestCase): host='fake_host', reservations={'fake': 'fake'}) + def test_update_share_from_metadata(self): + self._test_share_api('update_share_from_metadata', + rpc_method='cast', + version='1.28', + share=self.fake_share, + metadata={'fake': 'fake'}) + def test_create_replicated_snapshot(self): self._test_share_api('create_replicated_snapshot', rpc_method='cast', diff --git a/releasenotes/notes/bp-pass-resource-metadata-updates-to-backend-drivers-7fff302f64fda2d7.yaml b/releasenotes/notes/bp-pass-resource-metadata-updates-to-backend-drivers-7fff302f64fda2d7.yaml new file mode 100644 index 0000000000..6c591e0bb5 --- /dev/null +++ b/releasenotes/notes/bp-pass-resource-metadata-updates-to-backend-drivers-7fff302f64fda2d7.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + OpenStack operators can now make use of a new config option named + `driver_updatable_metadata` to determine which share metadata updates the + back end driver needs to be notified about. The config option contains + list of share metadata keys. When the share's metadata gets updated and + Manila identifies that the new metadata keys match the metadata keys from + the provided list, the share back end will be notified and it will apply + the necessary changes. The result will be communicated through user + messages.