From 8c1ec2276208f4b57ec2fa47ca0d6cdfd9010c2e Mon Sep 17 00:00:00 2001 From: Marius Leustean Date: Thu, 25 Mar 2021 15:28:16 +0200 Subject: [PATCH] [vmware] VmdkDriverRemoteApi: RPC initialization improvements Currently, creating the RPC client at the driver initialization might lead into an issue that, while upgrading to newer releases, the RPC client will limit the maximum RPC version to the older release's version, since that version is never refreshed while the volume backend is running. Initializing the RPC client every time it's needed will make sure it always refreshes the available RPC service versions. Also, inherit the RPC_DEFAULT_VERSION from the VolumeAPI, so that the smallest available major version is requested by default, instead of the biggest available version. --- .../volume/drivers/vmware/test_vmware_vmdk.py | 32 +++++++++++-------- cinder/volume/drivers/vmware/remote.py | 2 +- cinder/volume/drivers/vmware/vmdk.py | 12 +++---- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py index b7f9bdb7496..09fad0a1f93 100644 --- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py +++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py @@ -3444,12 +3444,18 @@ def test_revert_to_snapshot(self, vops): vops.revert_to_snapshot.assert_called_once_with(backing, snapshot.name) + @mock.patch('cinder.volume.drivers.vmware.remote.VmdkDriverRemoteApi.' + 'move_volume_backing_to_folder') + @mock.patch('cinder.volume.drivers.vmware.remote.VmdkDriverRemoteApi.' + 'select_ds_for_volume') + @mock.patch('cinder.volume.drivers.vmware.remote.VmdkDriverRemoteApi.' + 'get_service_locator_info') @mock.patch.object(VMDK_DRIVER, 'volumeops') @mock.patch('oslo_vmware.vim_util.get_moref') - def test_migrate_volume(self, get_moref, vops, backing=None, - raises_error=False, capabilities=None): - r_api = mock.Mock() - self._driver._remote_api = r_api + def test_migrate_volume(self, get_moref, vops, get_service_locator, + select_ds_for_volume, move_volume_backing, + backing=None, raises_error=False, + capabilities=None): volume = self._create_volume_obj() vops.get_backing.return_value = backing if capabilities is None: @@ -3467,11 +3473,11 @@ def test_migrate_volume(self, get_moref, vops, backing=None, mock.sentinel.rp_ref, mock.sentinel.ds_ref ] - r_api.get_service_locator_info.return_value = \ + get_service_locator.return_value = \ mock.sentinel.service_locator - r_api.select_ds_for_volume.return_value = ds_info + select_ds_for_volume.return_value = ds_info if raises_error: - r_api.move_volume_backing_to_folder.side_effect = Exception + move_volume_backing.side_effect = Exception ret_val = self._driver.migrate_volume(mock.sentinel.context, volume, host) @@ -3479,10 +3485,10 @@ def test_migrate_volume(self, get_moref, vops, backing=None, dest_host = host['host'] def _assertions_migration_not_called(): - r_api.get_service_locator_info.assert_not_called() - r_api.select_ds_for_volume.assert_not_called() + get_service_locator.assert_not_called() + select_ds_for_volume.assert_not_called() vops.relocate_backing.assert_not_called() - r_api.move_volume_backing_to_folder.assert_not_called() + move_volume_backing.assert_not_called() get_moref.assert_not_called() def _assertions_for_no_backing(): @@ -3496,10 +3502,10 @@ def _assertions_migration_not_performed(): def _assertions_for_migration(): vops.get_backing.assert_called_once_with(volume.name, volume.id) - r_api.get_service_locator_info.assert_called_once_with( + get_service_locator.assert_called_once_with( mock.sentinel.context, dest_host) - r_api.select_ds_for_volume.assert_called_once_with( + select_ds_for_volume.assert_called_once_with( mock.sentinel.context, dest_host, volume) get_moref.assert_has_calls([ @@ -3511,7 +3517,7 @@ def _assertions_for_migration(): backing, mock.sentinel.ds_ref, mock.sentinel.rp_ref, mock.sentinel.host_ref, service=mock.sentinel.service_locator) - r_api.move_volume_backing_to_folder.assert_called_once_with( + move_volume_backing.assert_called_once_with( mock.sentinel.context, dest_host, volume, ds_info['folder']) if raises_error: diff --git a/cinder/volume/drivers/vmware/remote.py b/cinder/volume/drivers/vmware/remote.py index c4d1ddd5be6..3653783d460 100644 --- a/cinder/volume/drivers/vmware/remote.py +++ b/cinder/volume/drivers/vmware/remote.py @@ -28,7 +28,7 @@ class VmdkDriverRemoteApi(rpc.RPCAPI): RPC_API_VERSION = VolumeAPI.RPC_API_VERSION - RPC_DEFAULT_VERSION = RPC_API_VERSION + RPC_DEFAULT_VERSION = VolumeAPI.RPC_DEFAULT_VERSION TOPIC = VolumeAPI.TOPIC BINARY = VolumeAPI.BINARY diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 558dd5ce202..67c301273f5 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -337,7 +337,6 @@ def __init__(self, *args, **kwargs): self.additional_endpoints.extend([ remote_api.VmdkDriverRemoteService(self) ]) - self._remote_api = remote_api.VmdkDriverRemoteApi() @staticmethod def get_driver_options(): @@ -2645,10 +2644,11 @@ def migrate_volume(self, context, volume, host): {'volume_name': volume.name, 'dest_host': dest_host}) return (True, None) - service_locator = self._remote_api.get_service_locator_info(context, - dest_host) - ds_info = self._remote_api.select_ds_for_volume(context, dest_host, - volume) + dest_api = remote_api.VmdkDriverRemoteApi() + + service_locator = dest_api.get_service_locator_info(context, dest_host) + ds_info = dest_api.select_ds_for_volume(context, dest_host, volume) + host_ref = vim_util.get_moref(ds_info['host'], 'HostSystem') rp_ref = vim_util.get_moref(ds_info['resource_pool'], 'ResourcePool') ds_ref = vim_util.get_moref(ds_info['datastore'], 'Datastore') @@ -2656,7 +2656,7 @@ def migrate_volume(self, context, volume, host): self.volumeops.relocate_backing(backing, ds_ref, rp_ref, host_ref, service=service_locator) try: - self._remote_api.move_volume_backing_to_folder( + dest_api.move_volume_backing_to_folder( context, dest_host, volume, ds_info['folder']) except Exception: # At this point the backing has been migrated to the new host.