[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.

Author: leust

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,
+    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:
@@ -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():
@@ -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:

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
 

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,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.