diff --git a/manila/share/manager.py b/manila/share/manager.py index 95fd3991..c76802a6 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -824,7 +824,7 @@ class ShareManager(manager.SchedulerDependentManager): raise exception.ShareMigrationFailed(reason=msg) try: - helper.apply_new_access_rules(share_instance, new_share_instance) + helper.apply_new_access_rules(new_share_instance) except Exception: msg = _("Failed to apply new access rules during migration " "of share %s.") % share_ref['id'] diff --git a/manila/share/migration.py b/manila/share/migration.py index 5ae57b49..bfb0fa1b 100644 --- a/manila/share/migration.py +++ b/manila/share/migration.py @@ -120,27 +120,6 @@ class ShareMigrationHelper(object): return new_share_instance - def _add_rules_and_wait(self, share_instance, access_rules): - - for access in access_rules: - values = { - 'share_id': self.share['id'], - 'access_type': access['access_type'], - 'access_level': access['access_level'], - 'access_to': access['access_to'] - } - - # NOTE(ganso): Instance Access Mapping is created only on - # db.share_access_create. - - self.db.share_access_create(self.context, values) - - self.api.allow_access_to_instance(self.context, share_instance, - access_rules) - utils.wait_for_access_update( - self.context, self.db, share_instance, - self.migration_wait_access_rules_timeout) - # NOTE(ganso): Cleanup methods do not throw exceptions, since the # exceptions that should be thrown are the ones that call the cleanup @@ -203,13 +182,20 @@ class ShareMigrationHelper(object): add_rules=[], delete_rules=[], share_server=share_server) - def apply_new_access_rules(self, share_instance, new_share_instance): + def apply_new_access_rules(self, new_share_instance): + + self.db.share_instance_access_copy(self.context, self.share['id'], + new_share_instance['id']) rules = self.db.share_access_get_all_for_instance( - self.context, share_instance['id']) + self.context, new_share_instance['id']) if len(rules) > 0: LOG.debug("Restoring all of share %s access rules according to " "DB.", self.share['id']) - self._add_rules_and_wait(new_share_instance, rules) + self.api.allow_access_to_instance(self.context, new_share_instance, + rules) + utils.wait_for_access_update( + self.context, self.db, new_share_instance, + self.migration_wait_access_rules_timeout) diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 09a8d53c..034fb108 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -3605,7 +3605,7 @@ class ShareManagerTestCase(test.TestCase): {'task_state': constants.TASK_STATE_MIGRATION_CANCELLED}) if status == constants.TASK_STATE_DATA_COPYING_COMPLETED: migration_api.ShareMigrationHelper.apply_new_access_rules.\ - assert_called_once_with(instance, new_instance) + assert_called_once_with(new_instance) self.assertTrue(manager.LOG.exception.called) def test__migration_complete(self): @@ -3658,7 +3658,7 @@ class ShareManagerTestCase(test.TestCase): {'task_state': constants.TASK_STATE_MIGRATION_SUCCESS}), ]) migration_api.ShareMigrationHelper.apply_new_access_rules.\ - assert_called_once_with(instance, new_instance) + assert_called_once_with(new_instance) migration_api.ShareMigrationHelper.delete_instance_and_wait.\ assert_called_once_with(instance) diff --git a/manila/tests/share/test_migration.py b/manila/tests/share/test_migration.py index 18992694..de17dfb7 100644 --- a/manila/tests/share/test_migration.py +++ b/manila/tests/share/test_migration.py @@ -307,8 +307,6 @@ class ShareMigrationHelperTestCase(test.TestCase): def test_apply_new_access_rules(self): - share_instance = db_utils.create_share_instance( - share_id=self.share['id'], status=constants.STATUS_AVAILABLE) new_share_instance = db_utils.create_share_instance( share_id=self.share['id'], status=constants.STATUS_AVAILABLE, access_rules_status='active') @@ -318,18 +316,25 @@ class ShareMigrationHelperTestCase(test.TestCase): access_level='rw') # mocks + self.mock_object(db, 'share_instance_access_copy') self.mock_object(db, 'share_access_get_all_for_instance', mock.Mock(return_value=[access])) - self.mock_object(self.helper, '_add_rules_and_wait') + self.mock_object(share_api.API, 'allow_access_to_instance') + self.mock_object(utils, 'wait_for_access_update') # run - self.helper.apply_new_access_rules(share_instance, new_share_instance) + self.helper.apply_new_access_rules(new_share_instance) # asserts + db.share_instance_access_copy(self.context, self.share['id'], + new_share_instance['id']) db.share_access_get_all_for_instance.assert_called_once_with( - self.context, share_instance['id']) - self.helper._add_rules_and_wait.assert_called_once_with( - new_share_instance, [access]) + self.context, new_share_instance['id']) + share_api.API.allow_access_to_instance.assert_called_with( + self.context, new_share_instance, [access]) + utils.wait_for_access_update.assert_called_with( + self.context, db, new_share_instance, + self.helper.migration_wait_access_rules_timeout) @ddt.data(None, Exception('fake')) def test_cleanup_new_instance(self, exc): @@ -371,35 +376,3 @@ class ShareMigrationHelperTestCase(test.TestCase): if exc: migration.LOG.warning.called - - def test__add_rules_and_wait(self): - - access = db_utils.create_access(share_id=self.share['id']) - - values = { - 'share_id': self.share['id'], - 'access_type': access['access_type'], - 'access_level': access['access_level'], - 'access_to': access['access_to'] - } - - self.helper.migration_wait_access_rules_timeout = 60 - - # mocks - self.mock_object(db, 'share_access_create') - - self.mock_object(share_api.API, 'allow_access_to_instance') - - self.mock_object(utils, 'wait_for_access_update') - - # run - self.helper._add_rules_and_wait(self.share_instance, [access]) - - # asserts - db.share_access_create.assert_called_once_with(self.context, values) - - share_api.API.allow_access_to_instance.assert_called_once_with( - self.context, self.share_instance, [access]) - - utils.wait_for_access_update.assert_called_once_with( - self.context, db, self.share_instance, 60)