openstack
/
manila
Code Issues Proposed changes

Add a new config option to specify admin metadata 

- A new config option named ``admin_metadata_keys`` was introduced
and we expect it to be set in the DEFAULT section of the manila
configuration file. It is expected that administrators will provide
a list of metadata keys that can only be updated by administrators
through this configuration option.

- Drivers will be able to set metadata while creating shares
through the `get_optional_share_creation_data` driver interface.

Closes-Bug: #2050010
Change-Id: I6412710c7db89747d23033e1a5a6be9de5886b0b
This commit is contained in:
silvacarloss 2024-02-15 20:26:24 -03:00
parent 05a231e370
commit 3429717601
8 changed files with 171 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
manila/api/v1/share_metadata.py
View File

@ -18,9 +18,10 @@ from oslo_log import log
import webob
from webob import exc
from oslo_config import cfg
from manila.api import common as api_common
from manila.api.openstack import wsgi
from manila.common import constants
from manila import db
from manila import exception
from manila.i18n import _
@ -29,6 +30,7 @@ from manila import share
LOG = log.getLogger(__name__)
CONF = cfg.CONF
class ShareMetadataController(object):
@ -107,7 +109,7 @@ class ShareMetadataController(object):
def _update_share_metadata(self, context,
share_id, metadata,
delete=False):
ignore_keys = constants.AdminOnlyMetadata.SCHEDULER_FILTERS
ignore_keys = getattr(CONF, 'admin_metadata_keys', [])
try:
share = self.share_api.get(context, share_id)
if set(metadata).intersection(set(ignore_keys)):
@ -176,7 +178,8 @@ class ShareMetadataController(object):
try:
share = self.share_api.get(context, share_id)
if id in constants.AdminOnlyMetadata.SCHEDULER_FILTERS:
admin_metadata_keys = getattr(CONF, 'admin_metadata_keys', set())
if id in admin_metadata_keys:
policy.check_policy(context, 'share',
'update_admin_only_metadata')
db.share_metadata_delete(context, share['id'], id)

13
manila/api/v2/shares.py
View File

@ -15,6 +15,7 @@
from http import client as http_client
from oslo_config import cfg
from oslo_log import log
import webob
from webob import exc
@ -39,6 +40,7 @@ from manila import share
from manila import utils
LOG = log.getLogger(__name__)
CONF = cfg.CONF
class ShareController(wsgi.Controller,
@ -57,6 +59,9 @@ class ShareController(wsgi.Controller,
self.resource_locks_api = resource_locks.API()
self._access_view_builder = share_access_views.ViewBuilder()
self._migration_view_builder = share_migration_views.ViewBuilder()
self._conf_admin_metadata_keys = getattr(
CONF, 'admin_metadata_keys', []
)
@wsgi.Controller.authorize('revert_to_snapshot')
def _revert(self, req, id, body=None):
@ -622,11 +627,9 @@ class ShareController(wsgi.Controller,
def _validate_metadata_for_update(self, req, share_id, metadata,
delete=True):
admin_metadata_ignore_keys = (
constants.AdminOnlyMetadata.SCHEDULER_FILTERS
)
admin_metadata_ignore_keys = set(self._conf_admin_metadata_keys)
context = req.environ['manila.context']
if set(metadata).intersection(set(admin_metadata_ignore_keys)):
if set(metadata).intersection(admin_metadata_ignore_keys):
try:
policy.check_policy(
context, 'share', 'update_admin_only_metadata')
@ -702,7 +705,7 @@ class ShareController(wsgi.Controller,
@wsgi.Controller.authorize("delete_share_metadata")
def delete_metadata(self, req, resource_id, key):
context = req.environ['manila.context']
if key in constants.AdminOnlyMetadata.SCHEDULER_FILTERS:
if key in self._conf_admin_metadata_keys:
policy.check_policy(context, 'share',
'update_admin_only_metadata')
return self._delete_metadata(req, resource_id, key)

4
manila/common/config.py
View File

@ -137,6 +137,10 @@ global_opts = [
help='Maximum time (in seconds) to keep a share in '
'awaiting_transfer state, after timeout, the share will '
'automatically be rolled back to the available state'),
cfg.ListOpt('admin_metadata_keys',
default=constants.AdminOnlyMetadata.SCHEDULER_FILTERS,
help='Metadata keys that should only be manipulated by '
'administrators.'),
]
CONF.register_opts(global_opts)

6
manila/common/constants.py
View File

@ -352,7 +352,7 @@ class AdminOnlyMetadata(object):
AFFINITY_KEY = "__affinity_same_host"
ANTI_AFFINITY_KEY = "__affinity_different_host"
SCHEDULER_FILTERS = (
SCHEDULER_FILTERS = [
AFFINITY_KEY,
ANTI_AFFINITY_KEY
)
ANTI_AFFINITY_KEY,
]

19
manila/share/driver.py
View File

@ -2834,6 +2834,25 @@ class ShareDriver(object):
"""
raise NotImplementedError()
def get_optional_share_creation_data(self, share, share_server=None):
"""Get info to set in shares after their creation.
Driver can use this method to get the special info and
return for assessment. The share manager service uses this
assessment to set this info to shares after the creation.
:returns: A dictionary containing driver-specific info.
Example::
{
'metadata': {'__mount_options': 'fake_key=fake_val'}
...
}
"""
return {}
def ensure_shares(self, context, shares):
"""Invoked to ensure that shares are exported.

18
manila/share/manager.py
View File

@ -482,6 +482,7 @@ class ShareManager(manager.SchedulerDependentManager):
self.db.backend_info_update(
ctxt, self.host, new_backend_info_hash)
shares_with_metadata_already_updated = set()
for share_instance in share_instances:
if share_instance['id'] not in update_share_instances:
continue
@ -494,6 +495,15 @@ class ShareManager(manager.SchedulerDependentManager):
{'status': share_instance_update_dict.get('status'),
'host': share_instance['host']}
)
metadata_updates = share_instance_update_dict.get('metadata')
if metadata_updates:
share_id = share_instance['share_id']
# NOTE(carloss): Multiple instances might exist, and in such
# cases, a single share metadata update would be enough.
if share_id not in shares_with_metadata_already_updated:
self.db.share_metadata_update(
ctxt, share_id, metadata_updates, False)
shares_with_metadata_already_updated.add(share_id)
update_export_locations = (
share_instance_update_dict.get('export_locations')
@ -2186,6 +2196,14 @@ class ShareManager(manager.SchedulerDependentManager):
msg = _('Driver returned an invalid status: %s') % status
raise exception.InvalidShareInstance(reason=msg)
share_backend_info = (
self.driver.get_optional_share_creation_data(
share, share_server=share_server))
if share_backend_info:
metadata_updates = share_backend_info.get("metadata")
if metadata_updates:
self.db.share_metadata_update(
context, share_id, metadata_updates, False)
if export_locations:
self.db.export_locations_update(
context, share_instance['id'], export_locations)

107
manila/tests/share/test_manager.py
View File

@ -282,6 +282,83 @@ class ShareManagerTestCase(test.TestCase):
(self.share_manager.access_helper.update_access_rules
.assert_not_called())
def test_ensure_driver_resources_share_metadata_updates(self):
old_hash = {'info_hash': '1e5ff444cfdc4a154126ddebc0223ffeae2d10c9'}
self.mock_object(self.share_manager.db,
'backend_info_get',
mock.Mock(return_value=old_hash))
self.mock_object(self.share_manager.driver,
'get_backend_info',
mock.Mock(return_value={'val': 'newval'}))
instances, rules = self._setup_init_mocks()
metadata_updates = {'update_meta_key': 'update_meta_val'}
fake_update_instances = {
instances[0]['id']: {
'metadata': metadata_updates,
},
instances[2]['id']: {
'metadata': metadata_updates,
},
}
mock_backend_info_update = self.mock_object(
self.share_manager.db, 'backend_info_update')
mock_share_get_all_by_host = self.mock_object(
self.share_manager.db, 'share_instance_get_all_by_host',
mock.Mock(return_value=instances))
self.mock_object(self.share_manager.db, 'share_instance_get',
mock.Mock(side_effect=[instances[0], instances[2],
instances[4]]))
self.mock_object(self.share_manager.db, 'share_metadata_update')
mock_ensure_shares = self.mock_object(
self.share_manager.driver, 'ensure_shares',
mock.Mock(return_value=fake_update_instances))
self.mock_object(self.share_manager, '_ensure_share_instance_has_pool')
self.mock_object(self.share_manager, '_get_share_server',
mock.Mock(return_value='share_server'))
self.mock_object(self.share_manager,
'_get_share_server_dict',
mock.Mock(return_value='share_server'))
mock_reset_rules_method = self.mock_object(
self.share_manager.access_helper, 'reset_rules_to_queueing_states')
dict_instances = [self._get_share_instance_dict(
instance, share_server='share_server') for instance in instances]
self.share_manager.ensure_driver_resources(self.context)
mock_backend_info_update.assert_called_once_with(
utils.IsAMatcher(context.RequestContext),
self.share_manager.host,
'7026776d9aaf3563799b8bbe1a020ea8cbd4dd22')
mock_ensure_shares.assert_called_once_with(
utils.IsAMatcher(context.RequestContext),
[dict_instances[0], dict_instances[2], dict_instances[4]])
mock_share_get_all_by_host.assert_called_once_with(
utils.IsAMatcher(context.RequestContext),
self.share_manager.host)
self.share_manager._ensure_share_instance_has_pool.assert_has_calls([
mock.call(utils.IsAMatcher(context.RequestContext),
instances[0]),
mock.call(utils.IsAMatcher(context.RequestContext),
instances[2]),
])
self.share_manager.db.share_metadata_update.assert_has_calls([
mock.call(
self.context, instances[0]['share_id'], metadata_updates, False
),
mock.call(
self.context, instances[2]['share_id'], metadata_updates, False
),
])
self.share_manager._get_share_server.assert_has_calls([
mock.call(utils.IsAMatcher(context.RequestContext),
instances[0]),
mock.call(utils.IsAMatcher(context.RequestContext),
instances[2]),
])
# none of the share instances in the fake data have syncing rules
mock_reset_rules_method.assert_not_called()
def test_init_host_with_no_shares(self):
self.mock_object(self.share_manager.db,
'share_instance_get_all_by_host',
@ -2703,6 +2780,7 @@ class ShareManagerTestCase(test.TestCase):
driver_mock.choose_share_server_compatible_with_share.return_value = (
share_srv
)
driver_mock.get_optional_share_creation_data.return_value = {}
self.share_manager.driver = driver_mock
self.share_manager.driver.\
dhss_mandatory_security_service_association = {}
@ -2791,6 +2869,35 @@ class ShareManagerTestCase(test.TestCase):
utils.IsAMatcher(context.RequestContext), fake_server,
fake_metadata)
def test_create_share_instance_with_backend_provided_metadata(self):
"""Test share can be created and share server is created."""
share_type = db_utils.create_share_type()
share = db_utils.create_share(share_type_id=share_type['id'],
availability_zone=None)
share_id = share['id']
share_backend_info_return = {
'metadata': {'meta_key': 'meta_val'}
}
share_metadata = share_backend_info_return['metadata']
self.mock_object(
self.share_manager.driver, 'get_optional_share_creation_data',
mock.Mock(return_value=share_backend_info_return)
)
self.mock_object(db, 'share_metadata_update')
self.share_manager.create_share_instance(self.context,
share.instance['id'])
self.assertEqual(share_id, db.share_get(context.get_admin_context(),
share_id).id)
shr = db.share_get(self.context, share_id)
self.assertEqual(constants.STATUS_AVAILABLE, shr['status'])
(self.share_manager.driver.get_optional_share_creation_data
.assert_called_once())
db.share_metadata_update.assert_called_once_with(
mock.ANY, share_id, share_metadata, False)
def test_create_share_instance_update_replica_state(self):
share_net = db_utils.create_share_network()
share_net_subnet = db_utils.create_share_network_subnet(

6
releasenotes/notes/bug-2050010-allow-configuring-admin-and-driver-metadata-b0ede7d7cf057b5b.yaml Normal file
View File

@ -0,0 +1,6 @@
---
upgrades:
- |
A new configuration option called ``admin_metadata_keys`` has been
introduced to assist cloud administrators while defining share
metadata that should only be modified by administrators.