From af3513d2b0f7bbcc968a4c445243de40d94714aa Mon Sep 17 00:00:00 2001 From: silvacarloss Date: Wed, 1 Dec 2021 18:08:47 -0300 Subject: [PATCH] Force share server selection on nondisruptive migration While performing nondisruptive migrations, share servers can not change, because such thing will cause the migration to be disruptive, since share servers have different network allocations and it will cause the destination share to change its export locations. This change fixes an issue that allowed manila/the share driver to chose where a share instance is going to land in case of a nondisruptive migration. Now, when such migration is required, manila will automatically place the destination share instance in the same share server that the source instance is located at. Closes-Bug: #1920942 Change-Id: I16110ec46c25be65760b15d7fbed67cd005f3873 --- manila/share/manager.py | 35 +++++++++++++------ manila/tests/share/test_manager.py | 28 ++++++++++----- ...ion-server-selection-3ad50e6c73ae03df.yaml | 8 +++++ 3 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/bug-1920942-fix-migration-server-selection-3ad50e6c73ae03df.yaml diff --git a/manila/share/manager.py b/manila/share/manager.py index e64c34c411..6e3537dfaf 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -1073,19 +1073,34 @@ class ShareManager(manager.SchedulerDependentManager): try: if dest_share_instance['share_network_id']: - rpcapi = share_rpcapi.ShareAPI() + # NOTE(carloss): For a nondisruptive migration request, we must + # not change the share server, otherwise the share's export + # location will change, disconnecting the user. Disruptive + # migration requests to the driver the share server. + if nondisruptive: + dest_share_server = self._get_share_server_dict( + context, share_server) + dest_share_instance = self.db.share_instance_update( + context, + dest_share_instance['id'], + {'share_server_id': dest_share_server['id']}, + with_share_data=True + ) + else: + rpcapi = share_rpcapi.ShareAPI() - # NOTE(ganso): Obtaining the share_server_id asynchronously so - # we can wait for it to be ready. - dest_share_server_id = rpcapi.provide_share_server( - context, dest_share_instance, - dest_share_instance['share_network_id']) + # NOTE(ganso): Obtaining the share_server_id asynchronously + # so we can wait for it to be ready. + dest_share_server_id = rpcapi.provide_share_server( + context, dest_share_instance, + dest_share_instance['share_network_id']) - rpcapi.create_share_server( - context, dest_share_instance, dest_share_server_id) + rpcapi.create_share_server( + context, dest_share_instance, dest_share_server_id) + + dest_share_server = helper.wait_for_share_server( + dest_share_server_id) - dest_share_server = helper.wait_for_share_server( - dest_share_server_id) else: dest_share_server = None diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 54c8abfa1a..91008c3cbf 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -5466,6 +5466,8 @@ class ShareManagerTestCase(test.TestCase): self.mock_object( migration_api.ShareMigrationHelper, 'wait_for_share_server', mock.Mock(return_value=dest_server)) + self.mock_object(self.share_manager.db, 'share_instance_update', + mock.Mock(return_value=migrating_instance)) self.mock_object(self.share_manager, '_migration_delete_instance') self.mock_object(self.share_manager.driver, 'migration_check_compatibility', @@ -5480,7 +5482,7 @@ class ShareManagerTestCase(test.TestCase): exception.ShareMigrationFailed, self.share_manager._migration_start_driver, self.context, share, src_instance, fake_dest_host, True, True, - True, not has_snapshots, 'fake_net_id', 'fake_az_id', + nondisruptive, not has_snapshots, 'fake_net_id', 'fake_az_id', 'fake_new_type_id') # asserts @@ -5488,13 +5490,23 @@ class ShareManagerTestCase(test.TestCase): utils.IsAMatcher(context.RequestContext), 'src_server_id') self.share_manager.db.share_instance_get.assert_called_once_with( self.context, migrating_instance['id'], with_share_data=True) - (rpcapi.ShareAPI.provide_share_server. - assert_called_once_with( - self.context, migrating_instance, 'fake_net_id')) - rpcapi.ShareAPI.create_share_server.assert_called_once_with( - self.context, migrating_instance, 'fake_dest_share_server_id') - (migration_api.ShareMigrationHelper.wait_for_share_server. - assert_called_once_with('fake_dest_share_server_id')) + if nondisruptive: + self.share_manager.db.share_instance_update.assert_called_with( + self.context, migrating_instance['id'], + {'share_server_id': src_server['id']}, + with_share_data=True + ) + rpcapi.ShareAPI.provide_share_server.assert_not_called() + rpcapi.ShareAPI.create_share_server.assert_not_called() + else: + (rpcapi.ShareAPI.provide_share_server. + assert_called_once_with( + self.context, migrating_instance, 'fake_net_id')) + rpcapi.ShareAPI.create_share_server.assert_called_once_with( + self.context, migrating_instance, 'fake_dest_share_server_id') + (migration_api.ShareMigrationHelper.wait_for_share_server. + assert_called_once_with('fake_dest_share_server_id')) + (api.API.create_share_instance_and_get_request_spec. assert_called_once_with(self.context, share, 'fake_az_id', None, 'fake_host', 'fake_net_id', diff --git a/releasenotes/notes/bug-1920942-fix-migration-server-selection-3ad50e6c73ae03df.yaml b/releasenotes/notes/bug-1920942-fix-migration-server-selection-3ad50e6c73ae03df.yaml new file mode 100644 index 0000000000..49a4a22a34 --- /dev/null +++ b/releasenotes/notes/bug-1920942-fix-migration-server-selection-3ad50e6c73ae03df.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Non-disruptive share migration will no longer choose a different + destination server even if limits of shares or gigabytes were exceeded in + the source. + For more details, please see + `bug #1920942 `_.