From da4f08b062024ee84889b7fc582a1c72b6e4d472 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Sat, 10 Jan 2015 23:02:00 +0530 Subject: [PATCH] VMware: Change inventory folder hierarchy In vCenter inventory, volumes are organized under the folder: /, where is the value of driver config option 'vmware_volume_folder'. Due to this organization certain project-specific maintenance tasks (using vSphere client) might be difficult for vCenter admin. For example, the admin may need to migrate the volumes of a particular project from one vCenter datastore to another. This patch changes the folder hierarchy to: /OpenStack/Project ()/ for better organization. DocImpact Modified default value of config option 'vmware_volume_folder' and updated its description. Closes-Bug: #1485550 Change-Id: I37a3bd00efaa3b4a0bbcb22cbe9fff3f4fb80456 --- cinder/tests/unit/test_vmware_vmdk.py | 45 ++++---- cinder/tests/unit/test_vmware_volumeops.py | 116 ++++++++++++++++++++- cinder/volume/drivers/vmware/vmdk.py | 39 ++++--- cinder/volume/drivers/vmware/volumeops.py | 35 ++++++- 4 files changed, 195 insertions(+), 40 deletions(-) diff --git a/cinder/tests/unit/test_vmware_vmdk.py b/cinder/tests/unit/test_vmware_vmdk.py index ca6fef068..3c4e40c2a 100644 --- a/cinder/tests/unit/test_vmware_vmdk.py +++ b/cinder/tests/unit/test_vmware_vmdk.py @@ -1227,7 +1227,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): # Test with in-use volume. vol = {'size': 1, 'status': 'retyping', 'name': 'vol-1', 'id': 'd11a82de-ddaa-448d-b50a-a255a7e61a1e', - 'volume_type_id': 'def'} + 'volume_type_id': 'def', + 'project_id': '63c19a12292549818c09946a5e59ddaf'} vol['volume_attachment'] = [mock.sentinel.volume_attachment] self.assertFalse(self._driver.retype(context, vol, new_type, diff, host)) @@ -1865,8 +1866,10 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): get_volume_group_folder.return_value = folder host = mock.sentinel.host + project_id = '63c19a12292549818c09946a5e59ddaf' vol = {'id': 'c1037b23-c5e9-4446-815f-3e097cbf5bb0', 'size': 1, - 'name': 'vol-c1037b23-c5e9-4446-815f-3e097cbf5bb0'} + 'name': 'vol-c1037b23-c5e9-4446-815f-3e097cbf5bb0', + 'project_id': project_id} ret = self._driver._select_ds_for_volume(vol, host) self.assertEqual((host_ref, rp, folder, summary), ret) @@ -1874,7 +1877,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): hub.DatastoreSelector.PROFILE_NAME: profile} ds_sel.select_datastore.assert_called_once_with(exp_req, hosts=[host]) vops.get_dc.assert_called_once_with(rp) - get_volume_group_folder.assert_called_once_with(dc) + get_volume_group_folder.assert_called_once_with(dc, project_id) @mock.patch.object(VMDK_DRIVER, '_get_storage_profile') @mock.patch.object(VMDK_DRIVER, 'ds_sel') @@ -1896,8 +1899,10 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): folder = mock.sentinel.folder get_volume_group_folder.return_value = folder + project_id = '63c19a12292549818c09946a5e59ddaf' vol = {'id': 'c1037b23-c5e9-4446-815f-3e097cbf5bb0', 'size': 1, - 'name': 'vol-c1037b23-c5e9-4446-815f-3e097cbf5bb0'} + 'name': 'vol-c1037b23-c5e9-4446-815f-3e097cbf5bb0', + 'project_id': project_id} ret = self._driver._select_ds_for_volume(vol) self.assertEqual((host_ref, rp, folder, summary), ret) @@ -1905,7 +1910,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): hub.DatastoreSelector.PROFILE_NAME: profile} ds_sel.select_datastore.assert_called_once_with(exp_req, hosts=None) vops.get_dc.assert_called_once_with(rp) - get_volume_group_folder.assert_called_once_with(dc) + get_volume_group_folder.assert_called_once_with(dc, project_id) @mock.patch.object(VMDK_DRIVER, '_get_storage_profile') @mock.patch.object(VMDK_DRIVER, 'ds_sel') @@ -2004,22 +2009,19 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): self.assertEqual(volume['id'], conn_info['data']['volume_id']) - def test_get_volume_group_folder(self): - """Test _get_volume_group_folder.""" - m = self.mox - m.StubOutWithMock(self._driver.__class__, 'volumeops') - self._driver.volumeops = self._volumeops - datacenter = FakeMor('Datacenter', 'my_dc') - m.StubOutWithMock(self._volumeops, 'get_vmfolder') - self._volumeops.get_vmfolder(datacenter) - m.StubOutWithMock(self._volumeops, 'create_folder') - self._volumeops.create_folder(mox.IgnoreArg(), - self._config.vmware_volume_folder) + @mock.patch.object(VMDK_DRIVER, 'volumeops') + def test_get_volume_group_folder(self, vops): + folder = mock.sentinel.folder + vops.create_vm_inventory_folder.return_value = folder - m.ReplayAll() - self._driver._get_volume_group_folder(datacenter) - m.UnsetStubs() - m.VerifyAll() + datacenter = mock.sentinel.dc + project_id = '63c19a12292549818c09946a5e59ddaf' + self.assertEqual(folder, + self._driver._get_volume_group_folder(datacenter, + project_id)) + project_folder_name = 'Project (%s)' % project_id + vops.create_vm_inventory_folder.assert_called_once_with( + datacenter, ['OpenStack', project_folder_name, self.VOLUME_FOLDER]) @mock.patch.object(VMDK_DRIVER, '_extend_vmdk_virtual_disk') @mock.patch.object(VMDK_DRIVER, 'volumeops') @@ -2779,7 +2781,8 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): @mock.patch.object(VMDK_DRIVER, 'ds_sel') def test_relocate_backing( self, ds_sel, get_volume_group_folder, vops): - volume = {'name': 'vol-1', 'size': 1} + volume = {'name': 'vol-1', 'size': 1, + 'project_id': '63c19a12292549818c09946a5e59ddaf'} vops.is_datastore_accessible.return_value = False ds_sel.is_datastore_compliant.return_value = True diff --git a/cinder/tests/unit/test_vmware_volumeops.py b/cinder/tests/unit/test_vmware_volumeops.py index fbad28415..44a37bd26 100644 --- a/cinder/tests/unit/test_vmware_volumeops.py +++ b/cinder/tests/unit/test_vmware_volumeops.py @@ -454,13 +454,14 @@ class VolumeOpsTestCase(test.TestCase): def test_create_folder_not_present(self): """Test create_folder when child not present.""" - parent_folder = mock.sentinel.parent_folder - child_name = 'child_folder' prop_val = mock.Mock(spec=object) - prop_val.ManagedObjectReference = [] child_folder = mock.sentinel.child_folder self.session.invoke_api.side_effect = [prop_val, child_folder] + + child_name = 'child_folder' + parent_folder = mock.sentinel.parent_folder ret = self.vops.create_folder(parent_folder, child_name) + self.assertEqual(child_folder, ret) expected_invoke_api = [mock.call(vim_util, 'get_object_property', self.session.vim, parent_folder, @@ -545,6 +546,115 @@ class VolumeOpsTestCase(test.TestCase): # Reset side effects. self.session.invoke_api.side_effect = None + def test_create_folder_with_duplicate_name(self): + parent_folder = mock.sentinel.parent_folder + child_name = 'child_folder' + + prop_val_1 = mock.Mock(spec=object) + prop_val_1.ManagedObjectReference = [] + + child_entity_2 = mock.Mock(spec=object) + child_entity_2._type = 'Folder' + prop_val_2 = mock.Mock(spec=object) + prop_val_2.ManagedObjectReference = [child_entity_2] + child_entity_2_name = child_name + + self.session.invoke_api.side_effect = [ + prop_val_1, + exceptions.DuplicateName, + prop_val_2, + child_entity_2_name] + + ret = self.vops.create_folder(parent_folder, child_name) + self.assertEqual(child_entity_2, ret) + expected_invoke_api = [mock.call(vim_util, 'get_object_property', + self.session.vim, parent_folder, + 'childEntity'), + mock.call(self.session.vim, 'CreateFolder', + parent_folder, name=child_name), + mock.call(vim_util, 'get_object_property', + self.session.vim, parent_folder, + 'childEntity'), + mock.call(vim_util, 'get_object_property', + self.session.vim, child_entity_2, + 'name')] + self.assertEqual(expected_invoke_api, + self.session.invoke_api.mock_calls) + + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + 'get_vmfolder') + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + 'create_folder') + def test_create_vm_inventory_folder(self, create_folder, get_vmfolder): + vm_folder_1 = mock.sentinel.vm_folder_1 + get_vmfolder.return_value = vm_folder_1 + + folder_1a = mock.sentinel.folder_1a + folder_1b = mock.sentinel.folder_1b + create_folder.side_effect = [folder_1a, folder_1b] + + datacenter_1 = mock.Mock(value='dc-1') + path_comp = ['a', 'b'] + ret = self.vops.create_vm_inventory_folder(datacenter_1, path_comp) + + self.assertEqual(folder_1b, ret) + get_vmfolder.assert_called_once_with(datacenter_1) + exp_calls = [mock.call(vm_folder_1, 'a'), mock.call(folder_1a, 'b')] + self.assertEqual(exp_calls, create_folder.call_args_list) + exp_cache = {'/dc-1': vm_folder_1, + '/dc-1/a': folder_1a, + '/dc-1/a/b': folder_1b} + self.assertEqual(exp_cache, self.vops._folder_cache) + + # Test cache + get_vmfolder.reset_mock() + create_folder.reset_mock() + + folder_1c = mock.sentinel.folder_1c + create_folder.side_effect = [folder_1c] + + path_comp = ['a', 'c'] + ret = self.vops.create_vm_inventory_folder(datacenter_1, path_comp) + + self.assertEqual(folder_1c, ret) + self.assertFalse(get_vmfolder.called) + exp_calls = [mock.call(folder_1a, 'c')] + self.assertEqual(exp_calls, create_folder.call_args_list) + exp_cache = {'/dc-1': vm_folder_1, + '/dc-1/a': folder_1a, + '/dc-1/a/b': folder_1b, + '/dc-1/a/c': folder_1c} + self.assertEqual(exp_cache, self.vops._folder_cache) + + # Test cache with different datacenter + get_vmfolder.reset_mock() + create_folder.reset_mock() + + vm_folder_2 = mock.sentinel.vm_folder_2 + get_vmfolder.return_value = vm_folder_2 + + folder_2a = mock.sentinel.folder_2a + folder_2b = mock.sentinel.folder_2b + create_folder.side_effect = [folder_2a, folder_2b] + + datacenter_2 = mock.Mock(value='dc-2') + path_comp = ['a', 'b'] + ret = self.vops.create_vm_inventory_folder(datacenter_2, path_comp) + + self.assertEqual(folder_2b, ret) + get_vmfolder.assert_called_once_with(datacenter_2) + exp_calls = [mock.call(vm_folder_2, 'a'), mock.call(folder_2a, 'b')] + self.assertEqual(exp_calls, create_folder.call_args_list) + exp_cache = {'/dc-1': vm_folder_1, + '/dc-1/a': folder_1a, + '/dc-1/a/b': folder_1b, + '/dc-1/a/c': folder_1c, + '/dc-2': vm_folder_2, + '/dc-2/a': folder_2a, + '/dc-2/a/b': folder_2b + } + self.assertEqual(exp_cache, self.vops._folder_cache) + def test_create_disk_backing_thin(self): backing = mock.Mock() del backing.eagerlyScrub diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 93ee673f8..5767fe130 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -92,9 +92,11 @@ vmdk_opts = [ help='The interval (in seconds) for polling remote tasks ' 'invoked on VMware ESX/VC server.'), cfg.StrOpt('vmware_volume_folder', - default='cinder-volumes', - help='Name for the folder in the VC datacenter that will ' - 'contain cinder volumes.'), + default='Volumes', + help='Name of the vCenter inventory folder that will ' + 'contain Cinder volumes. This folder will be created ' + 'under "OpenStack/", where project_folder ' + 'is of format "Project ()".'), cfg.IntOpt('vmware_image_transfer_timeout_secs', default=7200, help='Timeout in seconds for VMDK volume transfer between ' @@ -534,7 +536,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): (host_ref, resource_pool, summary) = self._select_datastore(req, host) dc = self.volumeops.get_dc(resource_pool) - folder = self._get_volume_group_folder(dc) + folder = self._get_volume_group_folder(dc, volume['project_id']) return (host_ref, resource_pool, folder, summary) @@ -1461,7 +1463,8 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): backing, new_datastore, rp, host, new_disk_type) dc = self.volumeops.get_dc(rp) - folder = self._get_volume_group_folder(dc) + folder = self._get_volume_group_folder(dc, + volume['project_id']) self.volumeops.move_backing_to_folder(backing, folder) elif need_disk_type_conversion: # Same datastore, but clone is needed for disk type conversion. @@ -1901,19 +1904,25 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver): "%(ip)s."), {'driver': self.__class__.__name__, 'ip': self.configuration.vmware_host_ip}) - def _get_volume_group_folder(self, datacenter): - """Get volume group folder. + def _get_volume_group_folder(self, datacenter, project_id): + """Get inventory folder for organizing volume backings. - Creates a folder under the vmFolder of the input datacenter with the - volume group name if it does not exists. + The inventory folder for organizing volume backings has the following + hierarchy: + /OpenStack/Project ()/ + + where volume_folder is the vmdk driver config option + "vmware_volume_folder". :param datacenter: Reference to the datacenter - :return: Reference to the volume folder + :param project_id: OpenStack project ID + :return: Reference to the inventory folder """ - vm_folder = super(VMwareVcVmdkDriver, - self)._get_volume_group_folder(datacenter) - volume_folder = self.configuration.vmware_volume_folder - return self.volumeops.create_folder(vm_folder, volume_folder) + volume_folder_name = self.configuration.vmware_volume_folder + project_folder_name = "Project (%s)" % project_id + folder_names = ['OpenStack', project_folder_name, volume_folder_name] + return self.volumeops.create_vm_inventory_folder(datacenter, + folder_names) def _relocate_backing(self, volume, backing, host): """Relocate volume backing to a datastore accessible to the given host. @@ -1952,7 +1961,7 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver): # Select datastore satisfying the requirements. (host, resource_pool, summary) = self._select_datastore(req, host) dc = self.volumeops.get_dc(resource_pool) - folder = self._get_volume_group_folder(dc) + folder = self._get_volume_group_folder(dc, volume['project_id']) self.volumeops.relocate_backing(backing, summary.datastore, resource_pool, host) diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index 47172e5c5..c3d4c2691 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -273,6 +273,7 @@ class VMwareVolumeOps(object): def __init__(self, session, max_objects): self._session = session self._max_objects = max_objects + self._folder_cache = {} def get_backing(self, name): """Get the backing based on name. @@ -567,13 +568,45 @@ class VMwareVolumeOps(object): child_folder_name) return child_folder + def create_vm_inventory_folder(self, datacenter, path_comp): + """Create and return a VM inventory folder. + + This method caches references to inventory folders returned. + + :param datacenter: Reference to datacenter + :param path_comp: Path components as a list + """ + LOG.debug("Creating inventory folder: %(path_comp)s under VM folder " + "of datacenter: %(datacenter)s.", + {'path_comp': path_comp, + 'datacenter': datacenter}) + path = "/" + datacenter.value + parent = self._folder_cache.get(path) + if not parent: + parent = self.get_vmfolder(datacenter) + self._folder_cache[path] = parent + + folder = None + for folder_name in path_comp: + path = "/".join([path, folder_name]) + folder = self._folder_cache.get(path) + if not folder: + folder = self.create_folder(parent, folder_name) + self._folder_cache[path] = folder + parent = folder + + LOG.debug("Inventory folder for path: %(path)s is %(folder)s.", + {'path': path, + 'folder': folder}) + return folder + def extend_virtual_disk(self, requested_size_in_gb, name, dc_ref, eager_zero=False): """Extend the virtual disk to the requested size. :param requested_size_in_gb: Size of the volume in GB :param name: Name of the backing - :param dc_ref: Reference datacenter + :param dc_ref: Reference to datacenter :param eager_zero: Boolean determining if the free space is zeroed out """ -- 2.45.2