VMware: Fix initialization of datastore selector

The WSDL URL of storage policy service is determined and a session is
created using it in do_setup(). This session is later used to initialize
the datastore selector property (ds_sel), which uses the session for all
storage policy related API calls.

After commit a8fa3ceb1e72bac2ab67f569a2ca009f995f59fd (Integrate
OSprofiler and Cinder), the properties defined in vmdk module are called
before do_setup(). As a result, the ds_sel (datastore selector) property
is initialized with a session instance containing a 'None' PBM (storage
policy service) WSDL URL. This results in failures of all storage policy
related APIs invoked using datastore selector. This patch fixes the
problem by re-initializing the property in do_setup().

Change-Id: Ibdf8b23f9e215000cf9053b81d374066fabd6851
Closes-Bug: #1380675

diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py
index b00868ffc1b25827221fb9463244a9227733c2ed..82da1a0c31b13806f927330de86b77710bc581ef 100644 (file)
--- a/cinder/tests/test_vmware_vmdk.py
+++ b/cinder/tests/test_vmware_vmdk.py
@@ -1956,44 +1956,61 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         version = self._driver._get_vc_version()
         self.assertEqual(LooseVersion('6.0.1'), version)
 
-    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
-                '_get_pbm_wsdl_location')
     @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
                 '_get_vc_version')
     @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
                 'session', new_callable=mock.PropertyMock)
-    def test_do_setup(self, session, _get_vc_version, _get_pbm_wsdl_location):
-        session = session.return_value
+    def test_do_setup_with_pbm_disabled(self, session, get_vc_version):
+        session_obj = mock.Mock(name='session')
+        session.return_value = session_obj
+        get_vc_version.return_value = LooseVersion('5.0')
 
-        # pbm is disabled
-        vc_version = LooseVersion('5.0')
-        _get_vc_version.return_value = vc_version
         self._driver.do_setup(mock.ANY)
+
         self.assertFalse(self._driver._storage_policy_enabled)
-        _get_vc_version.assert_called_once_with()
+        get_vc_version.assert_called_once_with()
+        self.assertEqual(session_obj, self._driver.volumeops._session)
+        self.assertEqual(session_obj, self._driver.ds_sel._session)
 
-        # pbm is enabled and invalid pbm wsdl location
+    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
+                '_get_pbm_wsdl_location')
+    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
+                '_get_vc_version')
+    def test_do_setup_with_invalid_pbm_wsdl(self, get_vc_version,
+                                            get_pbm_wsdl_location):
         vc_version = LooseVersion('5.5')
-        _get_vc_version.reset_mock()
-        _get_vc_version.return_value = vc_version
-        _get_pbm_wsdl_location.return_value = None
+        get_vc_version.return_value = vc_version
+        get_pbm_wsdl_location.return_value = None
+
         self.assertRaises(error_util.VMwareDriverException,
                           self._driver.do_setup,
                           mock.ANY)
+
         self.assertFalse(self._driver._storage_policy_enabled)
-        _get_vc_version.assert_called_once_with()
-        _get_pbm_wsdl_location.assert_called_once_with(vc_version)
+        get_vc_version.assert_called_once_with()
+        get_pbm_wsdl_location.assert_called_once_with(vc_version)
+
+    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
+                '_get_pbm_wsdl_location')
+    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
+                '_get_vc_version')
+    @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.'
+                'session', new_callable=mock.PropertyMock)
+    def test_do_setup(self, session, get_vc_version, get_pbm_wsdl_location):
+        session_obj = mock.Mock(name='session')
+        session.return_value = session_obj
 
-        # pbm is enabled and valid pbm wsdl location
         vc_version = LooseVersion('5.5')
-        _get_vc_version.reset_mock()
-        _get_vc_version.return_value = vc_version
-        _get_pbm_wsdl_location.reset_mock()
-        _get_pbm_wsdl_location.return_value = 'fake_pbm_location'
+        get_vc_version.return_value = vc_version
+        get_pbm_wsdl_location.return_value = 'file:///pbm.wsdl'
+
         self._driver.do_setup(mock.ANY)
+
         self.assertTrue(self._driver._storage_policy_enabled)
-        _get_vc_version.assert_called_once_with()
-        _get_pbm_wsdl_location.assert_called_once_with(vc_version)
+        get_vc_version.assert_called_once_with()
+        get_pbm_wsdl_location.assert_called_once_with(vc_version)
+        self.assertEqual(session_obj, self._driver.volumeops._session)
+        self.assertEqual(session_obj, self._driver.ds_sel._session)
 
     @mock.patch.object(VMDK_DRIVER, '_extend_volumeops_virtual_disk')
     @mock.patch.object(VMDK_DRIVER, '_create_backing_in_inventory')

diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py
index d087da207d89463c19791d44970a7cd247f850b0..a5c77efbed7bddf5c0783c29a1bf13353c810137 100644 (file)
--- a/cinder/volume/drivers/vmware/vmdk.py
+++ b/cinder/volume/drivers/vmware/vmdk.py
@@ -1894,9 +1894,11 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver):
             # Destroy current session so that it is recreated with pbm enabled
             self._session = None
 
-        # recreate session and initialize volumeops
+        # recreate session and initialize volumeops and ds_sel
+        # TODO(vbala) remove properties: session, volumeops and ds_sel
         max_objects = self.configuration.vmware_max_objects_retrieval
         self._volumeops = volumeops.VMwareVolumeOps(self.session, max_objects)
+        self._ds_sel = hub.DatastoreSelector(self.volumeops, self.session)
 
         LOG.info(_("Successfully setup driver: %(driver)s for server: "
                    "%(ip)s.") % {'driver': self.__class__.__name__,