From 9e05f681e8f21b7133308187738db3199615b6ad Mon Sep 17 00:00:00 2001 From: Maurice Escher Date: Tue, 4 May 2021 09:41:58 +0200 Subject: [PATCH] handle replica state on migration complete Set share instance replica_state to 'active' after successful migration if the share is involved in a replication setup. Change-Id: I6cf8a08379b2f2b0fd1bcada4cd20c39adc01f47 Closes-Bug: #1927060 (cherry picked from commit 68e3783522d363bdc6f21b9940a12ea36fc79375) (cherry picked from commit 9749a0a9a8ce21f4616f844114056cdb1ee8249f) (cherry picked from commit 4f45cd00e083373045ee6ff993ca78a255fe36fe) --- manila/share/manager.py | 32 +++++++----- manila/tests/share/test_manager.py | 49 +++++++++++++------ ...n-migration-complete-4fb4d8ba59b58505.yaml | 9 ++++ 3 files changed, 63 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/bug-1927060-fix-replica-state-on-migration-complete-4fb4d8ba59b58505.yaml diff --git a/manila/share/manager.py b/manila/share/manager.py index f72d4badab..3752f16a72 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -1377,15 +1377,26 @@ class ShareManager(manager.SchedulerDependentManager): helper.apply_new_access_rules(dest_share_instance) - self.db.share_instance_update( - context, dest_share_instance['id'], - {'status': constants.STATUS_AVAILABLE, 'progress': '100%'}) - - self.db.share_instance_update(context, src_share_instance['id'], - {'status': constants.STATUS_INACTIVE}) + self._migration_complete_instance(context, share_ref, + src_share_instance['id'], + dest_share_instance['id']) self._migration_delete_instance(context, src_share_instance['id']) + def _migration_complete_instance(self, context, share_ref, + src_instance_id, dest_instance_id): + dest_updates = { + 'status': constants.STATUS_AVAILABLE, + 'progress': '100%' + } + if share_ref.get('replication_type'): + dest_updates['replica_state'] = constants.REPLICA_STATE_ACTIVE + + self.db.share_instance_update(context, dest_instance_id, dest_updates) + + self.db.share_instance_update(context, src_instance_id, + {'status': constants.STATUS_INACTIVE}) + def _migration_delete_instance(self, context, instance_id): # refresh the share instance model @@ -1558,12 +1569,9 @@ class ShareManager(manager.SchedulerDependentManager): raise exception.ShareMigrationFailed(reason=msg) - self.db.share_instance_update( - context, dest_share_instance['id'], - {'status': constants.STATUS_AVAILABLE, 'progress': '100%'}) - - self.db.share_instance_update(context, src_instance_id, - {'status': constants.STATUS_INACTIVE}) + self._migration_complete_instance(context, share_ref, + src_share_instance['id'], + dest_share_instance['id']) helper.delete_instance_and_wait(src_share_instance) diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 70cfed9d48..5fb14372bc 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -1441,7 +1441,7 @@ class ShareManagerTestCase(test.TestCase): mock_replica_update = self.mock_object(db, 'share_replica_update') expected_update_calls = [mock.call( mock.ANY, r['id'], {'status': constants.STATUS_ERROR}) - for r in(replica, active_replica)] + for r in (replica, active_replica)] self.assertRaises(exception.ManilaException, self.share_manager.promote_share_replica, @@ -5553,6 +5553,7 @@ class ShareManagerTestCase(test.TestCase): mock.Mock(return_value=True)) self.mock_object(self.share_manager.db, 'share_instance_update') self.mock_object(self.share_manager.db, 'share_update') + self.mock_object(self.share_manager, '_migration_complete_instance') self.mock_object(self.share_manager, '_migration_delete_instance') self.mock_object(migration_api.ShareMigrationHelper, 'apply_new_access_rules') @@ -5583,14 +5584,11 @@ class ShareManagerTestCase(test.TestCase): snapshot_mappings, src_server, dest_server) (migration_api.ShareMigrationHelper.apply_new_access_rules. assert_called_once_with(dest_instance)) + (self.share_manager._migration_complete_instance. + assert_called_once_with(self.context, share, + src_instance['id'], dest_instance['id'])) self.share_manager._migration_delete_instance.assert_called_once_with( self.context, src_instance['id']) - self.share_manager.db.share_instance_update.assert_has_calls([ - mock.call(self.context, dest_instance['id'], - {'status': constants.STATUS_AVAILABLE, - 'progress': '100%'}), - mock.call(self.context, src_instance['id'], - {'status': constants.STATUS_INACTIVE})]) self.share_manager.db.share_update.assert_called_once_with( self.context, dest_instance['share_id'], {'task_state': constants.TASK_STATE_MIGRATION_COMPLETING}) @@ -5633,6 +5631,7 @@ class ShareManagerTestCase(test.TestCase): mock.Mock(side_effect=[instance, new_instance])) self.mock_object(self.share_manager.db, 'share_instance_update') self.mock_object(self.share_manager.db, 'share_update') + self.mock_object(self.share_manager, '_migration_complete_instance') delete_mock = self.mock_object(migration_api.ShareMigrationHelper, 'delete_instance_and_wait') self.mock_object(migration_api.ShareMigrationHelper, @@ -5648,20 +5647,15 @@ class ShareManagerTestCase(test.TestCase): mock.call(self.context, new_instance['id'], with_share_data=True) ]) - self.share_manager.db.share_instance_update.assert_has_calls([ - mock.call(self.context, new_instance['id'], - {'status': constants.STATUS_AVAILABLE, - 'progress': '100%'}), - mock.call(self.context, instance['id'], - {'status': constants.STATUS_INACTIVE}) - ]) - self.share_manager.db.share_update.assert_called_once_with( self.context, share['id'], {'task_state': constants.TASK_STATE_MIGRATION_COMPLETING}) (migration_api.ShareMigrationHelper.apply_new_access_rules. assert_called_once_with(new_instance)) delete_mock.assert_called_once_with(instance) + (self.share_manager._migration_complete_instance. + assert_called_once_with(self.context, share, instance['id'], + new_instance['id'])) @ddt.data(constants.TASK_STATE_MIGRATION_DRIVER_IN_PROGRESS, constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE, @@ -5828,6 +5822,31 @@ class ShareManagerTestCase(test.TestCase): (self.share_manager.db.share_snapshot_instance_delete. assert_called_once_with(self.context, snapshot.instance['id'])) + @ddt.data({}, {'replication_type': 'readable'}) + def test__migration_complete_instance(self, kwargs): + src_share = db_utils.create_share() + dest_share = db_utils.create_share(**kwargs) + src_instance_id = src_share['instance']['id'] + dest_instance_id = dest_share['instance']['id'] + src_updates = {'status': constants.STATUS_INACTIVE} + dest_updates = dest_updates = { + 'status': constants.STATUS_AVAILABLE, + 'progress': '100%' + } + if kwargs.get('replication_type'): + replication_info = { + 'replica_state': constants.REPLICA_STATE_ACTIVE} + dest_updates.update(replication_info) + + self.mock_object(self.share_manager.db, 'share_instance_update') + + self.share_manager._migration_complete_instance( + self.context, dest_share, src_instance_id, dest_instance_id) + + self.share_manager.db.share_instance_update.assert_has_calls( + [mock.call(self.context, dest_instance_id, dest_updates), + mock.call(self.context, src_instance_id, src_updates)]) + def test_migration_cancel_invalid(self): share = db_utils.create_share() diff --git a/releasenotes/notes/bug-1927060-fix-replica-state-on-migration-complete-4fb4d8ba59b58505.yaml b/releasenotes/notes/bug-1927060-fix-replica-state-on-migration-complete-4fb4d8ba59b58505.yaml new file mode 100644 index 0000000000..11a9847a12 --- /dev/null +++ b/releasenotes/notes/bug-1927060-fix-replica-state-on-migration-complete-4fb4d8ba59b58505.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed an issue that made migrated shares with replication support to do + not have a share instance with its `replica_state` set to active. Now, + when the share supports replication, the destination share instance will + have its replica state set as active right after the migration gets + completed. For more details, please refer to + `bug 1927060 `_