diff --git a/manila/share/driver.py b/manila/share/driver.py index 53e2e39061..163ecb7a54 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -326,7 +326,8 @@ class ShareDriver(object): 2nd phase call. Driver may throw exception when validating this parameter, exception if does not support 1-phase or 2-phase approach. :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 @@ -342,7 +343,8 @@ class ShareDriver(object): :param share_server: Share server model or None. :param dest_driver_migration_info: Migration information provided by 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 diff --git a/manila/share/manager.py b/manila/share/manager.py index 8e2a8e0416..08fa2860a4 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -614,7 +614,6 @@ class ShareManager(manager.SchedulerDependentManager): if not force_host_copy: try: - dest_driver_migration_info = rpcapi.migration_get_driver_info( context, share_instance) @@ -632,12 +631,6 @@ class ShareManager(manager.SchedulerDependentManager): context, share_instance, share_server, host, 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 # changes even if it has not performed migration. While this # 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) if not moved: - try: - LOG.debug("Starting generic migration " - "for share %s.", share_id) + LOG.debug("Starting generic migration " + "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, share_instance, host, notify) except Exception: @@ -670,6 +667,22 @@ class ShareManager(manager.SchedulerDependentManager): context, share_instance['id'], {'status': constants.STATUS_AVAILABLE}) 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, notify): diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index a2bd3b9f04..4ae4ef7c65 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -3682,8 +3682,94 @@ class ShareManagerTestCase(test.TestCase): self.share_manager.driver.migration_get_driver_info.\ assert_called_once_with(self.context, share_instance, share_server) - @ddt.data((True, 'fake_model_update'), exception.ManilaException()) - def test_migration_start(self, exc): + @ddt.data({'return_value': (True, 'fake_model_update'), 'notify': True}, + {'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' instance = db_utils.create_share_instance( @@ -3706,24 +3792,16 @@ class ShareManagerTestCase(test.TestCase): self.mock_object(rpcapi.ShareAPI, 'migration_get_driver_info', mock.Mock(return_value=driver_migration_info)) - if isinstance(exc, exception.ManilaException): - self.mock_object(self.share_manager.driver, 'migration_start', - mock.Mock(side_effect=exc)) - self.mock_object(self.share_manager, '_migration_start_generic', - mock.Mock(side_effect=Exception('fake'))) - self.mock_object(manager.LOG, 'exception') - else: - self.mock_object(self.share_manager.driver, 'migration_start', - mock.Mock(return_value=exc)) + self.mock_object(self.share_manager.driver, 'migration_start', + mock.Mock(side_effect=Exception('fake'))) + self.mock_object(self.share_manager, '_migration_start_generic', + mock.Mock(side_effect=Exception('fake'))) + self.mock_object(manager.LOG, 'exception') # run - if isinstance(exc, exception.ManilaException): - self.assertRaises(exception.ShareMigrationFailed, - self.share_manager.migration_start, - self.context, 'fake_id', host, False, False) - else: - self.share_manager.migration_start( - self.context, 'fake_id', host, False, False) + self.assertRaises(exception.ShareMigrationFailed, + self.share_manager.migration_start, + self.context, 'fake_id', host, False, False) # asserts self.share_manager.db.share_get.assert_called_once_with( @@ -3741,30 +3819,24 @@ class ShareManagerTestCase(test.TestCase): mock.call( self.context, share['id'], {'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 = [ 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): - share_update_calls.append(mock.call( - self.context, share['id'], - {'task_state': constants.TASK_STATE_MIGRATION_ERROR})) - 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._migration_start_generic. + assert_called_once_with(self.context, share, instance, host, + False)) + self.assertTrue(manager.LOG.exception.called) self.share_manager.db.share_update.assert_has_calls(share_update_calls) self.share_manager.db.share_instance_update.assert_has_calls(