openstack
/
manila
Code Issues Proposed changes

Fix manage API ignoring type extra specs 

Currently manage API allows managing a share with a share type that
may not make sense in the service host. This patch addresses this
by invoking the scheduler to validate the host before invoking the
backend manager.

APIImpact

Change-Id: I8c780f2518f4a6eacf37cc448c73fbb41f6b7507
Closes-bug: #1544725
This commit is contained in:
Rodrigo Barbieri 2016-03-17 17:07:47 -03:00
parent 9834c802b8
commit 2da94bb55e
7 changed files with 271 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
manila/scheduler/manager.py
View File

@ -59,7 +59,7 @@ MAPPING = {
class SchedulerManager(manager.Manager):
"""Chooses a host to create shares."""
RPC_API_VERSION = '1.5'
RPC_API_VERSION = '1.6'
def __init__(self, scheduler_driver=None, service_name=None,
*args, **kwargs):
@ -121,6 +121,28 @@ class SchedulerManager(manager.Manager):
"""Get active pools from the scheduler's cache."""
return self.driver.get_pools(context, filters)
def manage_share(self, context, share_id, driver_options, request_spec,
filter_properties=None):
"""Ensure that the host exists and can accept the share."""
def _manage_share_set_error(self, context, ex, request_spec):
self._set_share_state_and_notify(
'manage_share',
{'status': constants.STATUS_MANAGE_ERROR},
context, ex, request_spec)
share_ref = db.share_get(context, share_id)
try:
self.driver.host_passes_filters(
context, share_ref['host'], request_spec, filter_properties)
except Exception as ex:
with excutils.save_and_reraise_exception():
_manage_share_set_error(self, context, ex, request_spec)
else:
share_rpcapi.ShareAPI().manage_share(context, share_ref,
driver_options)
def migrate_share_to_host(self, context, share_id, host,
force_host_copy, notify, request_spec,
filter_properties=None):

15
manila/scheduler/rpcapi.py
View File

@ -37,15 +37,16 @@ class SchedulerAPI(object):
1.3 - Add create_consistency_group method
1.4 - Add migrate_share_to_host method
1.5 - Add create_share_replica
1.6 - Add manage_share
"""
RPC_API_VERSION = '1.5'
RPC_API_VERSION = '1.6'
def __init__(self):
super(SchedulerAPI, self).__init__()
target = messaging.Target(topic=CONF.scheduler_topic,
version=self.RPC_API_VERSION)
self.client = rpc.get_client(target, version_cap='1.5')
self.client = rpc.get_client(target, version_cap='1.6')
def create_share_instance(self, context, request_spec=None,
filter_properties=None):
@ -102,3 +103,13 @@ class SchedulerAPI(object):
'create_share_replica',
request_spec=request_spec_p,
filter_properties=filter_properties)
def manage_share(self, context, share_id, driver_options,
request_spec=None, filter_properties=None):
call_context = self.client.prepare(version='1.6')
return call_context.call(context, 'manage_share',
share_id=share_id,
driver_options=driver_options,
request_spec=request_spec,
filter_properties=filter_properties)

95
manila/share/api.py
View File

@ -476,14 +476,26 @@ class API(base.Base):
shares = self.get_all(context, {
'host': share_data['host'],
'export_location': share_data['export_location'],
'share_proto': share_data['share_proto']
'share_proto': share_data['share_proto'],
'share_type_id': share_data['share_type_id']
})
share_type = {}
share_type_id = share_data['share_type_id']
if share_type_id:
share_type = share_types.get_share_type(context, share_type_id)
snapshot_support = strutils.bool_from_string(
share_type.get('extra_specs', {}).get(
'snapshot_support', True) if share_type else True,
strict=True)
share_data.update({
'user_id': context.user_id,
'project_id': context.project_id,
'status': constants.STATUS_MANAGING,
'scheduled_at': timeutils.utcnow(),
'snapshot_support': snapshot_support,
})
LOG.debug("Manage: Found shares %s.", len(shares))
@ -505,9 +517,61 @@ class API(base.Base):
self.db.share_export_locations_update(context, share.instance['id'],
export_location)
self.share_rpcapi.manage_share(context, share, driver_options)
request_spec = self._get_request_spec_dict(share, share_type, size=0)
try:
self.scheduler_rpcapi.manage_share(context, share['id'],
driver_options, request_spec)
except Exception:
msg = _('Host %(host)s did not pass validation for managing of '
'share %(share)s with type %(type)s.') % {
'host': share['host'],
'share': share['id'],
'type': share['share_type_id']}
raise exception.InvalidHost(reason=msg)
return self.db.share_get(context, share['id'])
def _get_request_spec_dict(self, share, share_type, **kwargs):
share_instance = share['instance']
share_properties = {
'size': kwargs.get('size', share['size']),
'user_id': kwargs.get('user_id', share['user_id']),
'project_id': kwargs.get('project_id', share['project_id']),
'snapshot_support': kwargs.get(
'snapshot_support',
share_type['extra_specs']['snapshot_support']),
'share_proto': kwargs.get('share_proto', share['share_proto']),
'share_type_id': kwargs.get('share_type_id',
share['share_type_id']),
'is_public': kwargs.get('is_public', share['is_public']),
'consistency_group_id': kwargs.get('consistency_group_id',
share['consistency_group_id']),
'source_cgsnapshot_member_id': kwargs.get(
'source_cgsnapshot_member_id',
share['source_cgsnapshot_member_id']),
'snapshot_id': kwargs.get('snapshot_id', share['snapshot_id']),
}
share_instance_properties = {
'availability_zone_id': kwargs.get(
'availability_zone_id',
share_instance['availability_zone_id']),
'share_network_id': kwargs.get('share_network_id',
share_instance['share_network_id']),
'share_server_id': kwargs.get('share_server_id',
share_instance['share_server_id']),
'share_id': kwargs.get('share_id', share_instance['share_id']),
'host': kwargs.get('host', share_instance['host']),
'status': kwargs.get('status', share_instance['status']),
}
request_spec = {
'share_properties': share_properties,
'share_instance_properties': share_instance_properties,
'share_type': share_type,
'share_id': share['id']
}
return request_spec
def unmanage(self, context, share):
policy.check_policy(context, 'share', 'unmanage')
@ -811,32 +875,7 @@ class API(base.Base):
if share_type_id:
share_type = share_types.get_share_type(context, share_type_id)
share_properties = {
'size': share['size'],
'user_id': share['user_id'],
'project_id': share['project_id'],
'share_server_id': share_instance['share_server_id'],
'snapshot_support': share['snapshot_support'],
'share_proto': share['share_proto'],
'share_type_id': share['share_type_id'],
'is_public': share['is_public'],
'consistency_group_id': share['consistency_group_id'],
'source_cgsnapshot_member_id': share[
'source_cgsnapshot_member_id'],
'snapshot_id': share['snapshot_id'],
}
share_instance_properties = {
'availability_zone_id': share_instance['availability_zone_id'],
'share_network_id': share_instance['share_network_id'],
'share_server_id': share_instance['share_server_id'],
'share_id': share_instance['share_id'],
'host': share_instance['host'],
'status': share_instance['status'],
}
request_spec = {'share_properties': share_properties,
'share_instance_properties': share_instance_properties,
'share_type': share_type,
'share_id': share['id']}
request_spec = self._get_request_spec_dict(share, share_type)
try:
self.scheduler_rpcapi.migrate_share_to_host(context, share['id'],

23
manila/tests/scheduler/test_manager.py
View File

@ -243,6 +243,29 @@ class SchedulerManagerTestCase(test.TestCase):
exception.NoValidHost, self.manager.migrate_share_to_host,
self.context, share['id'], host, False, True, {}, None)
def test_manage_share(self):
share = db_utils.create_share()
self.mock_object(db, 'share_get', mock.Mock(return_value=share))
self.mock_object(share_rpcapi.ShareAPI, 'manage_share')
self.mock_object(base.Scheduler, 'host_passes_filters')
self.manager.manage_share(self.context, share['id'], 'driver_options',
{}, None)
def test_manage_share_exception(self):
share = db_utils.create_share()
self.mock_object(
base.Scheduler, 'host_passes_filters',
mock.Mock(side_effect=exception.NoValidHost('fake')))
self.assertRaises(
exception.NoValidHost, self.manager.manage_share,
self.context, share['id'], 'driver_options', {}, None)
def test_create_share_replica_exception_path(self):
"""Test 'raisable' exceptions for create_share_replica."""
db_update = self.mock_object(db, 'share_replica_update')

9
manila/tests/scheduler/test_rpcapi.py
View File

@ -118,3 +118,12 @@ class SchedulerRpcAPITestCase(test.TestCase):
request_spec='fake_request_spec',
filter_properties='filter_properties',
version='1.5')
def test_manage_share(self):
self._test_scheduler_api('manage_share',
rpc_method='call',
share_id='share_id',
driver_options='fake_driver_options',
request_spec='fake_request_spec',
filter_properties='filter_properties',
version='1.6')

161
manila/tests/share/test_api.py
View File

@ -724,11 +724,13 @@ class ShareAPITestCase(test.TestCase):
self.context, request_spec=mock.ANY, filter_properties={})
self.assertFalse(self.api.share_rpcapi.create_share_instance.called)
def test_manage_new(self):
@ddt.data('no_valid_host', None)
def test_manage_new(self, exc):
share_data = {
'host': 'fake',
'export_location': 'fake',
'share_proto': 'fake',
'share_type_id': 'fake',
}
driver_options = {}
date = datetime.datetime(1, 1, 1, 1, 1, 1)
@ -737,35 +739,57 @@ class ShareAPITestCase(test.TestCase):
'id': 'fakeid',
'status': constants.STATUS_CREATING,
}
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
share = db_api.share_create(self.context, fake_share_data)
if exc:
self.mock_object(self.scheduler_rpcapi, 'manage_share',
mock.Mock(side_effect=exception.NoValidHost))
self.mock_object(db_api, 'share_create',
mock.Mock(return_value=share))
self.mock_object(db_api, 'share_export_locations_update')
self.mock_object(db_api, 'share_get',
mock.Mock(return_value=share))
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.mock_object(self.api, 'get_all', mock.Mock(return_value=[]))
self.api.manage(self.context,
copy.deepcopy(share_data),
driver_options)
if exc:
self.assertRaises(exception.InvalidHost, self.api.manage,
self.context, copy.deepcopy(share_data),
driver_options)
else:
self.api.manage(self.context,
copy.deepcopy(share_data),
driver_options)
share_data.update({
'user_id': self.context.user_id,
'project_id': self.context.project_id,
'status': constants.STATUS_MANAGING,
'scheduled_at': date,
'snapshot_support': fake_type['extra_specs']['snapshot_support'],
})
expected_request_spec = self._get_request_spec_dict(
share, fake_type, size=0)
export_location = share_data.pop('export_location')
self.api.get_all.assert_called_once_with(self.context, mock.ANY)
db_api.share_create.assert_called_once_with(self.context, share_data)
db_api.share_get.assert_called_once_with(self.context, share['id'])
if not exc:
db_api.share_get.assert_called_once_with(self.context, share['id'])
db_api.share_export_locations_update.assert_called_once_with(
self.context, share.instance['id'], export_location
)
self.share_rpcapi.manage_share.assert_called_once_with(
self.context, share, driver_options)
self.scheduler_rpcapi.manage_share.assert_called_once_with(
self.context, share['id'], driver_options, expected_request_spec)
@ddt.data([{'id': 'fake', 'status': constants.STATUS_MANAGE_ERROR}])
def test_manage_retry(self, shares):
@ -773,14 +797,25 @@ class ShareAPITestCase(test.TestCase):
'host': 'fake',
'export_location': 'fake',
'share_proto': 'fake',
'share_type_id': 'fake',
}
driver_options = {}
fake_share_data = {'id': 'fakeid'}
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
share = db_api.share_create(self.context, fake_share_data)
self.mock_object(db_api, 'share_update',
mock.Mock(return_value=share))
self.mock_object(db_api, 'share_get',
mock.Mock(return_value=share))
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.mock_object(db_api, 'share_export_locations_update')
self.mock_object(self.api, 'get_all',
mock.Mock(return_value=shares))
@ -789,10 +824,13 @@ class ShareAPITestCase(test.TestCase):
copy.deepcopy(share_data),
driver_options)
expected_request_spec = self._get_request_spec_dict(
share, fake_type, size=0)
db_api.share_update.assert_called_once_with(
self.context, 'fake', mock.ANY)
self.share_rpcapi.manage_share.assert_called_once_with(
self.context, mock.ANY, driver_options)
self.scheduler_rpcapi.manage_share.assert_called_once_with(
self.context, share['id'], driver_options, expected_request_spec)
db_api.share_export_locations_update.assert_called_once_with(
self.context, share.instance['id'], mock.ANY
)
@ -802,14 +840,65 @@ class ShareAPITestCase(test.TestCase):
'host': 'fake',
'export_location': 'fake',
'share_proto': 'fake',
'share_type_id': 'fake',
}
driver_options = {}
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
self.mock_object(self.api, 'get_all',
mock.Mock(return_value=['fake', 'fake2']))
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.assertRaises(exception.ManilaException, self.api.manage,
self.context, share_data, driver_options)
def _get_request_spec_dict(self, share, share_type, **kwargs):
share_instance = share['instance']
share_properties = {
'size': kwargs.get('size', share['size']),
'user_id': kwargs.get('user_id', share['user_id']),
'project_id': kwargs.get('project_id', share['project_id']),
'snapshot_support': kwargs.get(
'snapshot_support',
share_type['extra_specs']['snapshot_support']),
'share_proto': kwargs.get('share_proto', share['share_proto']),
'share_type_id': kwargs.get('share_type_id',
share['share_type_id']),
'is_public': kwargs.get('is_public', share['is_public']),
'consistency_group_id': kwargs.get('consistency_group_id',
share['consistency_group_id']),
'source_cgsnapshot_member_id': kwargs.get(
'source_cgsnapshot_member_id',
share['source_cgsnapshot_member_id']),
'snapshot_id': kwargs.get('snapshot_id', share['snapshot_id']),
}
share_instance_properties = {
'availability_zone_id': kwargs.get(
'availability_zone_id',
share_instance['availability_zone_id']),
'share_network_id': kwargs.get('share_network_id',
share_instance['share_network_id']),
'share_server_id': kwargs.get('share_server_id',
share_instance['share_server_id']),
'share_id': kwargs.get('share_id', share_instance['share_id']),
'host': kwargs.get('host', share_instance['host']),
'status': kwargs.get('status', share_instance['status']),
}
request_spec = {
'share_properties': share_properties,
'share_instance_properties': share_instance_properties,
'share_type': share_type,
'share_id': share['id']
}
return request_spec
def test_unmanage(self):
share = db_utils.create_share(
@ -1692,39 +1781,24 @@ class ShareAPITestCase(test.TestCase):
def test_migration_start(self):
host = 'fake2@backend#pool'
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
share = db_utils.create_share(
status=constants.STATUS_AVAILABLE,
host='fake@backend#pool', share_type_id='fake_type_id')
request_spec = {
'share_properties': {
'size': share['size'],
'user_id': share['user_id'],
'project_id': share['project_id'],
'share_server_id': share['share_server_id'],
'snapshot_support': share['snapshot_support'],
'share_proto': share['share_proto'],
'share_type_id': share['share_type_id'],
'is_public': share['is_public'],
'consistency_group_id': share['consistency_group_id'],
'source_cgsnapshot_member_id': share[
'source_cgsnapshot_member_id'],
'snapshot_id': share['snapshot_id'],
},
'share_instance_properties': {
'availability_zone_id': share.instance['availability_zone_id'],
'share_network_id': share.instance['share_network_id'],
'share_server_id': share.instance['share_server_id'],
'share_id': share.instance['share_id'],
'host': share.instance['host'],
'status': share.instance['status'],
},
'share_type': 'fake_type',
'share_id': share['id'],
}
host='fake@backend#pool', share_type_id=fake_type['id'])
request_spec = self._get_request_spec_dict(
share, fake_type, size=0)
self.mock_object(self.scheduler_rpcapi, 'migrate_share_to_host')
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value='fake_type'))
mock.Mock(return_value=fake_type))
self.mock_object(utils, 'validate_service_host')
self.api.migration_start(self.context, share, host, True, True)
@ -1804,9 +1878,20 @@ class ShareAPITestCase(test.TestCase):
def test_migration_start_exception(self):
host = 'fake2@backend#pool'
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
share = db_utils.create_share(
host='fake@backend#pool', status=constants.STATUS_AVAILABLE)
host='fake@backend#pool', status=constants.STATUS_AVAILABLE,
share_type_id=fake_type['id'])
self.mock_object(self.scheduler_rpcapi, 'migrate_share_to_host')
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.mock_object(utils, 'validate_service_host')
self.mock_object(db_api, 'share_snapshot_get_all_for_share',
mock.Mock(return_value=False))

28
manila_tempest_tests/tests/api/admin/test_share_manage.py
View File

@ -159,24 +159,22 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke"])
def test_manage_retry(self):
# Manage share with invalid parameters
share = None
parameters = [(self.st_invalid['share_type']['id'], 'manage_error'),
(self.st['share_type']['id'], 'available')]
for share_type_id, status in parameters:
share = self.shares_v2_client.manage_share(
service_host=self.shares[1]['host'],
export_path=self.shares[1]['export_locations'][0],
protocol=self.shares[1]['share_proto'],
share_type_id=share_type_id)
self.assertRaises(
lib_exc.Conflict,
self.shares_v2_client.manage_share,
service_host=self.shares[1]['host'],
export_path=self.shares[1]['export_locations'][0],
protocol=self.shares[1]['share_proto'],
share_type_id=self.st_invalid['share_type']['id'])
# Add managed share to cleanup queue
self.method_resources.insert(
0, {'type': 'share', 'id': share['id'],
'client': self.shares_v2_client})
share = self.shares_v2_client.manage_share(
service_host=self.shares[1]['host'],
export_path=self.shares[1]['export_locations'][0],
protocol=self.shares[1]['share_proto'],
share_type_id=self.st['share_type']['id'])
# Wait for success
self.shares_v2_client.wait_for_share_status(share['id'], status)
self.shares_v2_client.wait_for_share_status(share['id'], 'available')
# Delete share
self.shares_v2_client.delete_share(share['id'])