From 7270e3fbfb0272ac7812fa3032cafcabec6d3742 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Mon, 13 Oct 2014 19:34:36 +0530 Subject: [PATCH] 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 (cherry picked from commit 6ac6225e72bde92f66da8e92c563c140471b949b) --- cinder/tests/test_vmware_vmdk.py | 71 +++++++++++++++++----------- cinder/volume/drivers/vmware/vmdk.py | 4 +- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py index b00868ffc1b..82da1a0c31b 100644 --- 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_vc_version') + @mock.patch('cinder.volume.drivers.vmware.vmdk.VMwareVcVmdkDriver.' + 'session', new_callable=mock.PropertyMock) + 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') + + self._driver.do_setup(mock.ANY) + + self.assertFalse(self._driver._storage_policy_enabled) + get_vc_version.assert_called_once_with() + self.assertEqual(session_obj, self._driver.volumeops._session) + self.assertEqual(session_obj, self._driver.ds_sel._session) + + @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.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) + @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(self, session, get_vc_version, get_pbm_wsdl_location): + session_obj = mock.Mock(name='session') + session.return_value = session_obj - # 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() - - # pbm is enabled and invalid 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 - 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.return_value = vc_version + get_pbm_wsdl_location.return_value = 'file:///pbm.wsdl' - # 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' 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 b8f48ec517d..13d32a1f497 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -1893,9 +1893,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__,