openstack/manila
Code Issues Proposed changes

Merge "Fix Share status when driver migrates"

Browse Source
This commit is contained in:
Jenkins 2016-05-06 02:18:17 +00:00 committed by Gerrit Code Review
commit 148bac3216
3 changed files with 137 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
manila/share/driver.py
View File

@ -326,7 +326,8 @@ class ShareDriver(object):
2nd phase call. Driver may throw exception when validating this 2nd phase call. Driver may throw exception when validating this
parameter, exception if does not support 1-phase or 2-phase approach. parameter, exception if does not support 1-phase or 2-phase approach.
:returns: Boolean value indicating if driver migration succeeded. :returns: Boolean value indicating if driver migration succeeded.
:returns: Dictionary containing a model update. :returns: Dictionary containing a model update with relevant data to
be updated after migration, such as export locations.
""" """
return None, None return None, None
@ -342,7 +343,8 @@ class ShareDriver(object):
:param share_server: Share server model or None. :param share_server: Share server model or None.
:param dest_driver_migration_info: Migration information provided by :param dest_driver_migration_info: Migration information provided by
destination host. destination host.
:returns: Dictionary containing a model update. :returns: Dictionary containing a model update with relevant data to
be updated after migration, such as export locations.
""" """
return None return None

33
manila/share/manager.py
View File

@ -614,7 +614,6 @@ class ShareManager(manager.SchedulerDependentManager):
if not force_host_copy: if not force_host_copy:
try: try:
dest_driver_migration_info = rpcapi.migration_get_driver_info( dest_driver_migration_info = rpcapi.migration_get_driver_info(
context, share_instance) context, share_instance)
@ -632,12 +631,6 @@ class ShareManager(manager.SchedulerDependentManager):
context, share_instance, share_server, host, context, share_instance, share_server, host,
dest_driver_migration_info, notify) dest_driver_migration_info, notify)
if moved and not notify:
self.db.share_update(
context, share_ref['id'],
{'task_state':
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE})
# NOTE(ganso): Here we are allowing the driver to perform # NOTE(ganso): Here we are allowing the driver to perform
# changes even if it has not performed migration. While this # changes even if it has not performed migration. While this
# scenario may not be valid, I do not think it should be # scenario may not be valid, I do not think it should be
@ -654,10 +647,14 @@ class ShareManager(manager.SchedulerDependentManager):
"with generic migration approach.") % share_id) "with generic migration approach.") % share_id)
if not moved: if not moved:
try: LOG.debug("Starting generic migration "
LOG.debug("Starting generic migration " "for share %s.", share_id)
"for share %s.", share_id)
self.db.share_update(
context, share_id,
{'task_state': constants.TASK_STATE_MIGRATION_IN_PROGRESS})
try:
self._migration_start_generic(context, share_ref, self._migration_start_generic(context, share_ref,
share_instance, host, notify) share_instance, host, notify)
except Exception: except Exception:
@ -670,6 +667,22 @@ class ShareManager(manager.SchedulerDependentManager):
context, share_instance['id'], context, share_instance['id'],
{'status': constants.STATUS_AVAILABLE}) {'status': constants.STATUS_AVAILABLE})
raise exception.ShareMigrationFailed(reason=msg) raise exception.ShareMigrationFailed(reason=msg)
elif not notify:
self.db.share_update(
context, share_ref['id'],
{'task_state':
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE})
else:
self.db.share_instance_update(
context, share_instance['id'],
{'status': constants.STATUS_AVAILABLE,
'host': host['host']})
self.db.share_update(
context, share_ref['id'],
{'task_state': constants.TASK_STATE_MIGRATION_SUCCESS})
LOG.info(_LI("Share Migration for share %s"
" completed successfully."), share_ref['id'])
def _migration_start_generic(self, context, share, share_instance, host, def _migration_start_generic(self, context, share, share_instance, host,
notify): notify):

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

@ -3682,8 +3682,94 @@ class ShareManagerTestCase(test.TestCase):
self.share_manager.driver.migration_get_driver_info.\ self.share_manager.driver.migration_get_driver_info.\
assert_called_once_with(self.context, share_instance, share_server) assert_called_once_with(self.context, share_instance, share_server)
@ddt.data((True, 'fake_model_update'), exception.ManilaException()) @ddt.data({'return_value': (True, 'fake_model_update'), 'notify': True},
def test_migration_start(self, exc): {'return_value': (False, 'fake_model_update'), 'notify': True},
{'return_value': (True, 'fake_model_update'), 'notify': False},
{'return_value': (False, 'fake_model_update'), 'notify': False})
@ddt.unpack
def test_migration_start(self, return_value, notify):
server = 'fake_share_server'
instance = db_utils.create_share_instance(
share_id='fake_id',
status=constants.STATUS_AVAILABLE,
share_server_id='fake_server_id')
share = db_utils.create_share(id='fake_id', instances=[instance])
host = {'host': 'fake_host'}
driver_migration_info = 'driver_fake_info'
# mocks
self.mock_object(self.share_manager.db, 'share_get',
mock.Mock(return_value=share))
self.mock_object(self.share_manager.db, 'share_instance_get',
mock.Mock(return_value=instance))
self.mock_object(self.share_manager.db, 'share_server_get',
mock.Mock(return_value=server))
self.mock_object(self.share_manager.db, 'share_update')
self.mock_object(self.share_manager.db, 'share_instance_update')
self.mock_object(rpcapi.ShareAPI, 'migration_get_driver_info',
mock.Mock(return_value=driver_migration_info))
self.mock_object(self.share_manager.driver, 'migration_start',
mock.Mock(return_value=return_value))
if not return_value[0]:
self.mock_object(self.share_manager, '_migration_start_generic')
# run
self.share_manager.migration_start(
self.context, 'fake_id', host, False, notify)
# asserts
self.share_manager.db.share_get.assert_called_once_with(
self.context, share['id'])
self.share_manager.db.share_instance_get.assert_called_once_with(
self.context, instance['id'], with_share_data=True)
self.share_manager.db.share_server_get.assert_called_once_with(
utils.IsAMatcher(context.RequestContext),
instance['share_server_id'])
share_update_calls = [
mock.call(
self.context, share['id'],
{'task_state': constants.TASK_STATE_MIGRATION_IN_PROGRESS}),
mock.call(
self.context, share['id'],
{'task_state': (
constants.TASK_STATE_MIGRATION_DRIVER_IN_PROGRESS)})
]
share_instance_update_calls = [
mock.call(self.context, instance['id'],
{'status': constants.STATUS_MIGRATING}),
mock.call(self.context, instance['id'], 'fake_model_update')
]
if not notify and return_value[0]:
share_update_calls.append(mock.call(
self.context, share['id'],
{'task_state':
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE}))
elif notify and return_value[0]:
share_update_calls.append(mock.call(
self.context, share['id'],
{'task_state': constants.TASK_STATE_MIGRATION_SUCCESS}))
share_instance_update_calls.append(mock.call(
self.context, instance['id'],
{'status': constants.STATUS_AVAILABLE,
'host': host['host']}))
elif not return_value[0]:
share_update_calls.append(mock.call(
self.context, share['id'],
{'task_state': constants.TASK_STATE_MIGRATION_IN_PROGRESS}))
self.share_manager.db.share_update.assert_has_calls(share_update_calls)
self.share_manager.db.share_instance_update.assert_has_calls(
share_instance_update_calls)
rpcapi.ShareAPI.migration_get_driver_info.assert_called_once_with(
self.context, instance)
self.share_manager.driver.migration_start.assert_called_once_with(
self.context, instance, server, host, driver_migration_info,
notify)
def test_migration_start_exception(self):
server = 'fake_share_server' server = 'fake_share_server'
instance = db_utils.create_share_instance( instance = db_utils.create_share_instance(
@ -3706,24 +3792,16 @@ class ShareManagerTestCase(test.TestCase):
self.mock_object(rpcapi.ShareAPI, 'migration_get_driver_info', self.mock_object(rpcapi.ShareAPI, 'migration_get_driver_info',
mock.Mock(return_value=driver_migration_info)) mock.Mock(return_value=driver_migration_info))
if isinstance(exc, exception.ManilaException): self.mock_object(self.share_manager.driver, 'migration_start',
self.mock_object(self.share_manager.driver, 'migration_start', mock.Mock(side_effect=Exception('fake')))
mock.Mock(side_effect=exc)) self.mock_object(self.share_manager, '_migration_start_generic',
self.mock_object(self.share_manager, '_migration_start_generic', mock.Mock(side_effect=Exception('fake')))
mock.Mock(side_effect=Exception('fake'))) self.mock_object(manager.LOG, 'exception')
self.mock_object(manager.LOG, 'exception')
else:
self.mock_object(self.share_manager.driver, 'migration_start',
mock.Mock(return_value=exc))
# run # run
if isinstance(exc, exception.ManilaException): self.assertRaises(exception.ShareMigrationFailed,
self.assertRaises(exception.ShareMigrationFailed, self.share_manager.migration_start,
self.share_manager.migration_start, self.context, 'fake_id', host, False, False)
self.context, 'fake_id', host, False, False)
else:
self.share_manager.migration_start(
self.context, 'fake_id', host, False, False)
# asserts # asserts
self.share_manager.db.share_get.assert_called_once_with( self.share_manager.db.share_get.assert_called_once_with(
@ -3741,30 +3819,24 @@ class ShareManagerTestCase(test.TestCase):
mock.call( mock.call(
self.context, share['id'], self.context, share['id'],
{'task_state': ( {'task_state': (
constants.TASK_STATE_MIGRATION_DRIVER_IN_PROGRESS)}) constants.TASK_STATE_MIGRATION_DRIVER_IN_PROGRESS)}),
mock.call(
self.context, share['id'],
{'task_state': constants.TASK_STATE_MIGRATION_IN_PROGRESS}),
mock.call(
self.context, share['id'],
{'task_state': constants.TASK_STATE_MIGRATION_ERROR})
] ]
share_instance_update_calls = [ share_instance_update_calls = [
mock.call(self.context, instance['id'], mock.call(self.context, instance['id'],
{'status': constants.STATUS_MIGRATING}) {'status': constants.STATUS_MIGRATING}),
mock.call(self.context, instance['id'],
{'status': constants.STATUS_AVAILABLE})
] ]
if isinstance(exc, exception.ManilaException): (self.share_manager._migration_start_generic.
share_update_calls.append(mock.call( assert_called_once_with(self.context, share, instance, host,
self.context, share['id'], False))
{'task_state': constants.TASK_STATE_MIGRATION_ERROR})) self.assertTrue(manager.LOG.exception.called)
share_instance_update_calls.append(
mock.call(self.context, instance['id'],
{'status': constants.STATUS_AVAILABLE}))
self.share_manager._migration_start_generic.\
assert_called_once_with(self.context, share, instance, host,
False)
self.assertTrue(manager.LOG.exception.called)
else:
share_update_calls.append(mock.call(
self.context, share['id'],
{'task_state':
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE}))
share_instance_update_calls.append(
mock.call(self.context, instance['id'], 'fake_model_update'))
self.share_manager.db.share_update.assert_has_calls(share_update_calls) self.share_manager.db.share_update.assert_has_calls(share_update_calls)
self.share_manager.db.share_instance_update.assert_has_calls( self.share_manager.db.share_instance_update.assert_has_calls(