Skip to content

Commit

Permalink
[vmware] VmdkDriverRemoteApi: RPC initialization improvements
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
leust committed Mar 25, 2021
1 parent 893c218 commit 04edead
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 19 deletions.
30 changes: 18 additions & 12 deletions cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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):
r_api = mock.Mock()
self._driver._remote_api = r_api
volume = self._create_volume_obj()
vops.get_backing.return_value = backing
if capabilities is None:
Expand All @@ -3467,22 +3473,22 @@ 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)

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():
Expand All @@ -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([
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion cinder/volume/drivers/vmware/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions cinder/volume/drivers/vmware/vmdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -2645,18 +2644,19 @@ 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')

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.
Expand Down

0 comments on commit 04edead

Please sign in to comment.