From 4287bfa9d4a06a45b60eff4f903df0b92390c59d Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 14 Dec 2016 16:56:25 +0200 Subject: [PATCH] RemoteFS: pass volume object to '_find_share' This change ensures that volume objects are passed to the '_find_share' driver method. This allows the RemoteFS based volume drivers to have a more flexible way to choose which share should host a specific volume. Partially Implements: blueprint remotefs-pools-support Change-Id: Id8ca91c29db08e6a320c7f3642c34dceff76dbba --- cinder/tests/unit/volume/drivers/test_coho.py | 6 +++--- cinder/tests/unit/volume/drivers/test_nfs.py | 13 ++++++++----- cinder/tests/unit/volume/drivers/test_quobyte.py | 10 +++++----- cinder/tests/unit/volume/drivers/test_vzstorage.py | 6 +++--- cinder/tests/unit/windows/test_smbfs.py | 6 +++--- cinder/volume/drivers/coho.py | 4 ++-- cinder/volume/drivers/ibm/gpfs.py | 6 +++--- cinder/volume/drivers/nfs.py | 9 +++++---- cinder/volume/drivers/quobyte.py | 4 ++-- cinder/volume/drivers/remotefs.py | 6 +++--- cinder/volume/drivers/vzstorage.py | 8 ++++---- cinder/volume/drivers/windows/smbfs.py | 9 ++++----- 12 files changed, 45 insertions(+), 42 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/test_coho.py b/cinder/tests/unit/volume/drivers/test_coho.py index 8325b43bf36..4945f2b74c1 100644 --- a/cinder/tests/unit/volume/drivers/test_coho.py +++ b/cinder/tests/unit/volume/drivers/test_coho.py @@ -243,7 +243,7 @@ class CohoDriverTest(test.TestCase): drv.create_volume_from_snapshot(VOLUME, SNAPSHOT) mock_find_share.assert_has_calls( - [mock.call(VOLUME['size'])]) + [mock.call(VOLUME)]) self.assertTrue(mock_get_admin_context.called) mock_get_volume_type.assert_has_calls( [mock.call('test', VOLUME_TYPE['id'])]) @@ -277,7 +277,7 @@ class CohoDriverTest(test.TestCase): drv.create_cloned_volume(VOLUME, CLONE_VOL) mock_find_share.assert_has_calls( - [mock.call(VOLUME['size'])]) + [mock.call(VOLUME)]) mock_local_path.assert_has_calls( [mock.call(VOLUME), mock.call(CLONE_VOL)]) mock_execute.assert_has_calls( @@ -333,7 +333,7 @@ class CohoDriverTest(test.TestCase): drv.create_cloned_volume(CLONE_VOL, VOLUME) mock_find_share.assert_has_calls( - [mock.call(CLONE_VOL['size'])]) + [mock.call(CLONE_VOL)]) mock_local_path.assert_has_calls( [mock.call(CLONE_VOL), mock.call(VOLUME)]) mock_execute.assert_has_calls( diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index cfff772a39c..4a060cce92c 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -639,7 +639,7 @@ class NfsDriverTestCase(test.TestCase): drv._mounted_shares = [] self.assertRaises(exception.NfsNoSharesMounted, drv._find_share, - self.TEST_SIZE_IN_GB) + self._simple_volume()) def test_find_share(self): """_find_share simple use case.""" @@ -647,13 +647,16 @@ class NfsDriverTestCase(test.TestCase): drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB) + with mock.patch.object( drv, '_get_capacity_info') as mock_get_capacity_info: mock_get_capacity_info.side_effect = [ (5 * units.Gi, 2 * units.Gi, 2 * units.Gi), (10 * units.Gi, 3 * units.Gi, 1 * units.Gi)] self.assertEqual(self.TEST_NFS_EXPORT2, - drv._find_share(self.TEST_SIZE_IN_GB)) + drv._find_share(volume)) calls = [mock.call(self.TEST_NFS_EXPORT1), mock.call(self.TEST_NFS_EXPORT2)] mock_get_capacity_info.assert_has_calls(calls) @@ -672,7 +675,7 @@ class NfsDriverTestCase(test.TestCase): (10 * units.Gi, 0, 10 * units.Gi)] self.assertRaises(exception.NfsNoSuitableShareFound, - drv._find_share, self.TEST_SIZE_IN_GB) + drv._find_share, self._simple_volume()) calls = [mock.call(self.TEST_NFS_EXPORT1), mock.call(self.TEST_NFS_EXPORT2)] mock_get_capacity_info.assert_has_calls(calls) @@ -753,7 +756,7 @@ class NfsDriverTestCase(test.TestCase): result = drv.create_volume(volume) self.assertEqual(self.TEST_NFS_EXPORT1, result['provider_location']) - mock_find_share.assert_called_once_with(self.TEST_SIZE_IN_GB) + mock_find_share.assert_called_once_with(volume) def test_delete_volume(self): """delete_volume simple test case.""" @@ -1262,7 +1265,7 @@ class NfsDriverTestCase(test.TestCase): src_volume_path, new_volume_path, 'qcow2' if used_qcow else 'raw', run_as_root=True) mock_ensure.assert_called_once() - mock_find_share.assert_called_once_with(new_volume.size) + mock_find_share.assert_called_once_with(new_volume) def test_create_volume_from_snapshot_status_not_available(self): """Expect an error when the snapshot's status is not 'available'.""" diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index 9f44c3251d6..c4936fd3bb1 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -388,7 +388,7 @@ class QuobyteDriverTestCase(test.TestCase): self.assertRaises(exception.NotFound, drv._find_share, - self.TEST_SIZE_IN_GB) + self._simple_volume()) def test_find_share(self): """_find_share simple use case.""" @@ -397,7 +397,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._mounted_shares = [self.TEST_QUOBYTE_VOLUME] self.assertEqual(self.TEST_QUOBYTE_VOLUME, - drv._find_share(self.TEST_SIZE_IN_GB)) + drv._find_share(self._simple_volume())) def test_find_share_does_not_throw_error_if_there_isnt_enough_space(self): """_find_share intentionally does not throw when no space is left.""" @@ -412,7 +412,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._mounted_shares = [self.TEST_QUOBYTE_VOLUME] self.assertEqual(self.TEST_QUOBYTE_VOLUME, - drv._find_share(self.TEST_SIZE_IN_GB)) + drv._find_share(self._simple_volume())) # The current implementation does not call _get_available_capacity. # Future ones might do and therefore we mocked it. @@ -516,7 +516,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._do_create_volume.assert_called_once_with(volume) drv._ensure_shares_mounted.assert_called_once_with() - drv._find_share.assert_called_once_with(self.TEST_SIZE_IN_GB) + drv._find_share.assert_called_once_with(volume) @mock.patch('oslo_utils.fileutils.delete_if_exists') def test_delete_volume(self, mock_delete_if_exists): @@ -724,7 +724,7 @@ class QuobyteDriverTestCase(test.TestCase): drv.create_volume_from_snapshot(new_volume, snap_ref) drv._ensure_shares_mounted.assert_called_once_with() - drv._find_share.assert_called_once_with(new_volume['size']) + drv._find_share.assert_called_once_with(new_volume) drv._do_create_volume.assert_called_once_with(new_volume) (drv._copy_volume_from_snapshot. assert_called_once_with(snap_ref, new_volume, new_volume['size'])) diff --git a/cinder/tests/unit/volume/drivers/test_vzstorage.py b/cinder/tests/unit/volume/drivers/test_vzstorage.py index 9191468b3bb..d7d2a5531e1 100644 --- a/cinder/tests/unit/volume/drivers/test_vzstorage.py +++ b/cinder/tests/unit/volume/drivers/test_vzstorage.py @@ -178,21 +178,21 @@ class VZStorageTestCase(test.TestCase): drv = self._vz_driver drv._mounted_shares = [self._FAKE_SHARE] with mock.patch.object(drv, '_is_share_eligible', return_value=True): - ret = drv._find_share(1) + ret = drv._find_share(self.vol) self.assertEqual(self._FAKE_SHARE, ret) def test_find_share_no_shares_mounted(self): drv = self._vz_driver with mock.patch.object(drv, '_is_share_eligible', return_value=True): self.assertRaises(exception.VzStorageNoSharesMounted, - drv._find_share, 1) + drv._find_share, self.vol) def test_find_share_no_shares_suitable(self): drv = self._vz_driver drv._mounted_shares = [self._FAKE_SHARE] with mock.patch.object(drv, '_is_share_eligible', return_value=False): self.assertRaises(exception.VzStorageNoSuitableShareFound, - drv._find_share, 1) + drv._find_share, self.vol) def test_is_share_eligible_false(self): drv = self._vz_driver diff --git a/cinder/tests/unit/windows/test_smbfs.py b/cinder/tests/unit/windows/test_smbfs.py index c5267a88f23..0ad9700cb31 100644 --- a/cinder/tests/unit/windows/test_smbfs.py +++ b/cinder/tests/unit/windows/test_smbfs.py @@ -296,14 +296,14 @@ class WindowsSmbFsTestCase(test.TestCase): if not mounted_shares: self.assertRaises(exception.SmbfsNoSharesMounted, self._smbfs_driver._find_share, - self.volume.size) + self.volume) elif not eligible_shares: self.assertRaises(exception.SmbfsNoSuitableShareFound, self._smbfs_driver._find_share, - self.volume.size) + self.volume) else: ret_value = self._smbfs_driver._find_share( - self.volume.size) + self.volume) # The eligible share with the minimum allocated space # will be selected self.assertEqual('fake_share3', ret_value) diff --git a/cinder/volume/drivers/coho.py b/cinder/volume/drivers/coho.py index e9c3eec120d..a8d64451e66 100644 --- a/cinder/volume/drivers/coho.py +++ b/cinder/volume/drivers/coho.py @@ -443,7 +443,7 @@ class CohoDriver(nfs.NfsDriver): def create_volume_from_snapshot(self, volume, snapshot): """Create a volume from a snapshot.""" - volume['provider_location'] = self._find_share(volume['size']) + volume['provider_location'] = self._find_share(volume) addr, path = volume['provider_location'].split(":") volume_path = os.path.join(path, volume['name']) snapshot_name = snapshot['name'] @@ -462,7 +462,7 @@ class CohoDriver(nfs.NfsDriver): path, run_as_root=self._execute_as_root) def create_cloned_volume(self, volume, src_vref): - volume['provider_location'] = self._find_share(volume['size']) + volume['provider_location'] = self._find_share(volume) self._do_clone_volume(volume, src_vref) diff --git a/cinder/volume/drivers/ibm/gpfs.py b/cinder/volume/drivers/ibm/gpfs.py index 8dcead8a33d..c4f488aea84 100644 --- a/cinder/volume/drivers/ibm/gpfs.py +++ b/cinder/volume/drivers/ibm/gpfs.py @@ -1494,7 +1494,7 @@ class GPFSNFSDriver(GPFSDriver, nfs.NfsDriver, san.SanDriver): def create_volume(self, volume): """Creates a GPFS volume.""" super(GPFSNFSDriver, self).create_volume(volume) - volume['provider_location'] = self._find_share(volume['size']) + volume['provider_location'] = self._find_share(volume) return {'provider_location': volume['provider_location']} def delete_volume(self, volume): @@ -1511,13 +1511,13 @@ class GPFSNFSDriver(GPFSDriver, nfs.NfsDriver, san.SanDriver): def create_volume_from_snapshot(self, volume, snapshot): """Creates a GPFS volume from a snapshot.""" self._create_volume_from_snapshot(volume, snapshot) - volume['provider_location'] = self._find_share(volume['size']) + volume['provider_location'] = self._find_share(volume) self._resize_volume_file(volume, volume['size']) return {'provider_location': volume['provider_location']} def create_cloned_volume(self, volume, src_vref): """Create a GPFS volume from another volume.""" self._create_cloned_volume(volume, src_vref) - volume['provider_location'] = self._find_share(volume['size']) + volume['provider_location'] = self._find_share(volume) self._resize_volume_file(volume, volume['size']) return {'provider_location': volume['provider_location']} diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 375d25aff8a..e5c7b4bb5cc 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -219,13 +219,13 @@ class NfsDriver(remotefs.RemoteFSSnapDriver, driver.ExtendVD): {'attempt': attempt, 'exc': e}) time.sleep(1) - def _find_share(self, volume_size_in_gib): + def _find_share(self, volume): """Choose NFS share among available ones for given volume size. For instances with more than one share that meets the criteria, the share with the least "allocated" space will be selected. - :param volume_size_in_gib: int size in GB + :param volume: the volume to be created. """ if not self._mounted_shares: @@ -241,7 +241,8 @@ class NfsDriver(remotefs.RemoteFSSnapDriver, driver.ExtendVD): 'total_available': total_available, 'total_allocated': total_allocated, } - if not self._is_share_eligible(nfs_share, volume_size_in_gib, + if not self._is_share_eligible(nfs_share, + volume.size, share_info): continue if target_share is not None: @@ -254,7 +255,7 @@ class NfsDriver(remotefs.RemoteFSSnapDriver, driver.ExtendVD): if target_share is None: raise exception.NfsNoSuitableShareFound( - volume_size=volume_size_in_gib) + volume_size=volume.size) LOG.debug('Selected %s as target NFS share.', target_share) diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index 80891f9a65c..63cc2e4cbe4 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -384,7 +384,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): LOG.debug('Available shares %s', self._mounted_shares) - def _find_share(self, volume_size_in_gib): + def _find_share(self, volume): """Returns the mounted Quobyte volume. Multiple shares are not supported because the virtualization of @@ -393,7 +393,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): For different types of volumes e.g., SSD vs. rotating disks, use multiple backends in Cinder. - :param volume_size_in_gib: int size in GB. Ignored by this driver. + :param volume: the volume to be created. """ if not self._mounted_shares: diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 77753336522..ff6d79f3eef 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -242,7 +242,7 @@ class RemoteFSDriver(driver.BaseVD): LOG.debug('Creating volume %(vol)s', {'vol': volume.id}) self._ensure_shares_mounted() - volume.provider_location = self._find_share(volume.size) + volume.provider_location = self._find_share(volume) LOG.info('casted to %s', volume.provider_location) @@ -549,7 +549,7 @@ class RemoteFSDriver(driver.BaseVD): def _get_capacity_info(self, share): raise NotImplementedError() - def _find_share(self, volume_size_in_gib): + def _find_share(self, volume): raise NotImplementedError() def _ensure_share_mounted(self, share): @@ -1157,7 +1157,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): self._ensure_shares_mounted() - volume.provider_location = self._find_share(volume.size) + volume.provider_location = self._find_share(volume) self._do_create_volume(volume) diff --git a/cinder/volume/drivers/vzstorage.py b/cinder/volume/drivers/vzstorage.py index 72313c3b999..90f87b30b57 100644 --- a/cinder/volume/drivers/vzstorage.py +++ b/cinder/volume/drivers/vzstorage.py @@ -295,24 +295,24 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): mnt_flags.extend(extra_flags) self._remotefsclient.mount(share, mnt_flags) - def _find_share(self, volume_size_in_gib): + def _find_share(self, volume): """Choose VzStorage share among available ones for given volume size. For instances with more than one share that meets the criteria, the first suitable share will be selected. - :param volume_size_in_gib: int size in GB + :param volume: the volume to be created. """ if not self._mounted_shares: raise exception.VzStorageNoSharesMounted() for share in self._mounted_shares: - if self._is_share_eligible(share, volume_size_in_gib): + if self._is_share_eligible(share, volume.size): break else: raise exception.VzStorageNoSuitableShareFound( - volume_size=volume_size_in_gib) + volume_size=volume.size) LOG.debug('Selected %s as target VzStorage share.', share) diff --git a/cinder/volume/drivers/windows/smbfs.py b/cinder/volume/drivers/windows/smbfs.py index 265f2a2f605..0185a3eebbd 100644 --- a/cinder/volume/drivers/windows/smbfs.py +++ b/cinder/volume/drivers/windows/smbfs.py @@ -244,15 +244,14 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver): total_allocated = share_alloc_data.get('total_allocated', 0) << 30 return float(total_allocated) - def _find_share(self, volume_size_in_gib): + def _find_share(self, volume): """Choose SMBFS share among available ones for given volume size. For instances with more than one share that meets the criteria, the share with the least "allocated" space will be selected. - :param volume_size_in_gib: int size in GB + :param volume: the volume to be created. """ - if not self._mounted_shares: raise exception.SmbfsNoSharesMounted() @@ -260,7 +259,7 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver): target_share_reserved = 0 for smbfs_share in self._mounted_shares: - if not self._is_share_eligible(smbfs_share, volume_size_in_gib): + if not self._is_share_eligible(smbfs_share, volume.size): continue total_allocated = self._get_total_allocated(smbfs_share) if target_share is not None: @@ -273,7 +272,7 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSSnapDriver): if target_share is None: raise exception.SmbfsNoSuitableShareFound( - volume_size=volume_size_in_gib) + volume_size=volume.size) LOG.debug('Selected %s as target smbfs share.', target_share)