From: Morgan Fainberg Date: Tue, 14 May 2013 20:04:20 +0000 (-0700) Subject: Make NFS share selection more intelligent. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5bf7b9be798296b08de42f4cb6295071dc299ef8;p=openstack-build%2Fcinder-build.git Make NFS share selection more intelligent. Make the NFS share selection more intelligent by using a used_ratio and oversub_ratio to determine the valid target. The used_ratio is a hard-cap of maximum "actual" used space, oversub_ratio allows for oversubscription of the share based upon the apparant-allocated space for sparse files. Removed Options: nfs_disk_util New options added: * nfs_used_ratio: Float representation of the maximum allowed usage (e.g. 0.95 calculated by available_space / total_space) before the share is no longer concidered a valid target. Default: 0.95 * nfs_oversub_ratio: Float representation of the oversubscriotion ratio allowed when utilizing sparse files (determined via "du"). This gets multiplied against the total_capacity before comparing the apparant space used by the backing volume files. Default ratio is 1.0 (meaning reserved/allocated space cannot exceed total space available on the mount). DocImpact: New Configuration Options / Removal of Config Option bp nfs-share-selection-logic-improvement Change-Id: I3572e106eb4c794b08bf6f499e2da2d494c4072d --- diff --git a/cinder/tests/test_nfs.py b/cinder/tests/test_nfs.py index 4c87fef8f..e961018fb 100644 --- a/cinder/tests/test_nfs.py +++ b/cinder/tests/test_nfs.py @@ -31,6 +31,7 @@ from cinder import context from cinder import exception from cinder.exception import ProcessExecutionError from cinder import test +from cinder import units from cinder.volume import configuration as conf from cinder.volume.drivers import nfs @@ -116,7 +117,6 @@ class NfsDriverTestCase(test.TestCase): TEST_LOCAL_PATH = '/mnt/nfs/volume-123' TEST_FILE_NAME = 'test.txt' TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf' - ONE_GB_IN_BYTES = 1024 * 1024 * 1024 def setUp(self): self._mox = mox_lib.Mox() @@ -126,8 +126,9 @@ class NfsDriverTestCase(test.TestCase): self.configuration.nfs_shares_config = None self.configuration.nfs_mount_options = None self.configuration.nfs_mount_point_base = '$state_path/mnt' - self.configuration.nfs_disk_util = 'df' self.configuration.nfs_sparsed_volumes = True + self.configuration.nfs_used_ratio = 0.95 + self.configuration.nfs_oversub_ratio = 1.0 self._driver = nfs.NfsDriver(configuration=self.configuration) self._driver.shares = {} @@ -258,50 +259,18 @@ class NfsDriverTestCase(test.TestCase): self.assertEqual('/mnt/test/2f4f60214cf43c595666dd815f0360a4', drv._get_mount_point_for_share(self.TEST_NFS_EXPORT1)) - def test_get_available_capacity_with_df(self): - """_get_available_capacity should calculate correct value.""" + def test_get_capacity_info(self): + """_get_capacity_info should calculate correct value.""" mox = self._mox drv = self._driver df_total_size = 2620544 - df_avail = 1490560 + df_avail = 2129984 df_head = 'Filesystem 1K-blocks Used Available Use% Mounted on\n' df_data = 'nfs-host:/export %d 996864 %d 41%% /mnt' % (df_total_size, df_avail) df_output = df_head + df_data - self.configuration.nfs_disk_util = 'df' - - mox.StubOutWithMock(drv, '_get_mount_point_for_share') - drv._get_mount_point_for_share(self.TEST_NFS_EXPORT1).\ - AndReturn(self.TEST_MNT_POINT) - - mox.StubOutWithMock(drv, '_execute') - drv._execute('df', '-P', '-B', '1', self.TEST_MNT_POINT, - run_as_root=True).AndReturn((df_output, None)) - - mox.ReplayAll() - - self.assertEquals((df_avail, df_total_size), - drv._get_available_capacity(self.TEST_NFS_EXPORT1)) - - mox.VerifyAll() - - def test_get_available_capacity_with_du(self): - """_get_available_capacity should calculate correct value.""" - mox = self._mox - drv = self._driver - self.configuration.nfs_disk_util = 'du' - - df_total_size = 2620544 - df_used_size = 996864 - df_avail_size = 1490560 - df_title = 'Filesystem 1-blocks Used Available Use% Mounted on\n' - df_mnt_data = 'nfs-host:/export %d %d %d 41%% /mnt' % (df_total_size, - df_used_size, - df_avail_size) - df_output = df_title + df_mnt_data - du_used = 490560 du_output = '%d /mnt' % du_used @@ -311,8 +280,8 @@ class NfsDriverTestCase(test.TestCase): mox.StubOutWithMock(drv, '_execute') drv._execute('df', '-P', '-B', '1', self.TEST_MNT_POINT, - run_as_root=True).\ - AndReturn((df_output, None)) + run_as_root=True).AndReturn((df_output, None)) + drv._execute('du', '-sb', '--apparent-size', '--exclude', '*snapshot*', self.TEST_MNT_POINT, @@ -320,8 +289,8 @@ class NfsDriverTestCase(test.TestCase): mox.ReplayAll() - self.assertEquals((df_total_size - du_used, df_total_size), - drv._get_available_capacity(self.TEST_NFS_EXPORT1)) + self.assertEquals((df_total_size, df_avail, du_used), + drv._get_capacity_info(self.TEST_NFS_EXPORT1)) mox.VerifyAll() @@ -426,6 +395,27 @@ class NfsDriverTestCase(test.TestCase): self.assertRaises(exception.NfsException, drv.do_setup, IsA(context.RequestContext)) + def test_setup_should_throw_error_if_oversub_ratio_less_than_zero(self): + """do_setup should throw error if nfs_oversub_ratio is less than 0.""" + drv = self._driver + self.configuration.nfs_oversub_ratio = -1 + self.assertRaises(exception.NfsException, + drv.do_setup, IsA(context.RequestContext)) + + def test_setup_should_throw_error_if_used_ratio_less_than_zero(self): + """do_setup should throw error if nfs_used_ratio is less than 0.""" + drv = self._driver + self.configuration.nfs_used_ratio = -1 + self.assertRaises(exception.NfsException, + drv.do_setup, IsA(context.RequestContext)) + + def test_setup_should_throw_error_if_used_ratio_greater_than_one(self): + """do_setup should throw error if nfs_used_ratio is greater than 1.""" + drv = self._driver + self.configuration.nfs_used_ratio = 2 + self.assertRaises(exception.NfsException, + drv.do_setup, IsA(context.RequestContext)) + def test_setup_should_throw_exception_if_nfs_client_is_not_installed(self): """do_setup should throw error if nfs client is not installed.""" mox = self._mox @@ -461,11 +451,13 @@ class NfsDriverTestCase(test.TestCase): drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] - mox.StubOutWithMock(drv, '_get_available_capacity') - drv._get_available_capacity(self.TEST_NFS_EXPORT1).\ - AndReturn((2 * self.ONE_GB_IN_BYTES, 5 * self.ONE_GB_IN_BYTES)) - drv._get_available_capacity(self.TEST_NFS_EXPORT2).\ - AndReturn((3 * self.ONE_GB_IN_BYTES, 10 * self.ONE_GB_IN_BYTES)) + mox.StubOutWithMock(drv, '_get_capacity_info') + drv._get_capacity_info(self.TEST_NFS_EXPORT1).\ + AndReturn((5 * units.GiB, 2 * units.GiB, + 2 * units.GiB)) + drv._get_capacity_info(self.TEST_NFS_EXPORT2).\ + AndReturn((10 * units.GiB, 3 * units.GiB, + 1 * units.GiB)) mox.ReplayAll() @@ -481,11 +473,12 @@ class NfsDriverTestCase(test.TestCase): drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] - mox.StubOutWithMock(drv, '_get_available_capacity') - drv._get_available_capacity(self.TEST_NFS_EXPORT1).\ - AndReturn((0, 5 * self.ONE_GB_IN_BYTES)) - drv._get_available_capacity(self.TEST_NFS_EXPORT2).\ - AndReturn((0, 10 * self.ONE_GB_IN_BYTES)) + mox.StubOutWithMock(drv, '_get_capacity_info') + drv._get_capacity_info(self.TEST_NFS_EXPORT1).\ + AndReturn((5 * units.GiB, 0, 5 * units.GiB)) + drv._get_capacity_info(self.TEST_NFS_EXPORT2).\ + AndReturn((10 * units.GiB, 0, + 10 * units.GiB)) mox.ReplayAll() @@ -656,14 +649,16 @@ class NfsDriverTestCase(test.TestCase): drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] mox.StubOutWithMock(drv, '_ensure_shares_mounted') - mox.StubOutWithMock(drv, '_get_available_capacity') + mox.StubOutWithMock(drv, '_get_capacity_info') drv._ensure_shares_mounted() - drv._get_available_capacity(self.TEST_NFS_EXPORT1).\ - AndReturn((2 * self.ONE_GB_IN_BYTES, 10 * self.ONE_GB_IN_BYTES)) - drv._get_available_capacity(self.TEST_NFS_EXPORT2).\ - AndReturn((3 * self.ONE_GB_IN_BYTES, 20 * self.ONE_GB_IN_BYTES)) + drv._get_capacity_info(self.TEST_NFS_EXPORT1).\ + AndReturn((10 * units.GiB, 2 * units.GiB, + 2 * units.GiB)) + drv._get_capacity_info(self.TEST_NFS_EXPORT2).\ + AndReturn((20 * units.GiB, 3 * units.GiB, + 3 * units.GiB)) mox.ReplayAll() diff --git a/cinder/units.py b/cinder/units.py new file mode 100644 index 000000000..8cfafa088 --- /dev/null +++ b/cinder/units.py @@ -0,0 +1,22 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 OpenStack LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +""" +A module where we define some basic units for use across Cinder. +""" + +KiB = 1024 +MiB = KiB * 1024 +GiB = MiB * 1024 diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 7b04a792e..a162a7d25 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -24,6 +24,7 @@ from oslo.config import cfg from cinder import exception from cinder.image import image_utils from cinder.openstack.common import log as logging +from cinder import units from cinder.volume import driver LOG = logging.getLogger(__name__) @@ -35,9 +36,6 @@ volume_opts = [ cfg.StrOpt('nfs_mount_point_base', default='$state_path/mnt', help='Base dir containing mount points for nfs shares'), - cfg.StrOpt('nfs_disk_util', - default='df', - help='Use du or df for free space calculation'), cfg.BoolOpt('nfs_sparsed_volumes', default=True, help=('Create volumes as sparsed files which take no space.' @@ -47,9 +45,19 @@ volume_opts = [ default=None, help='Mount options passed to the nfs client. See section ' 'of the nfs man page for details'), + cfg.FloatOpt('nfs_used_ratio', + default=0.95, + help=('Percent of ACTUAL usage of the underlying volume ' + 'before no new volumes can be allocated to the volume ' + 'destination.')), + cfg.FloatOpt('nfs_oversub_ratio', + default=1.0, + help=('This will compare the allocated to available space on ' + 'the volume destination. If the ratio exceeds this ' + 'number, the destination will no longer be valid.')) ] -VERSION = '1.0' +VERSION = '1.1' class RemoteFsDriver(driver.VolumeDriver): @@ -81,12 +89,9 @@ class RemoteFsDriver(driver.VolumeDriver): def _create_regular_file(self, path, size): """Creates regular file of given size. Takes a lot of time for large files.""" - KB = 1024 - MB = KB * 1024 - GB = MB * 1024 block_size_mb = 1 - block_count = size * GB / (block_size_mb * MB) + block_count = size * units.GiB / (block_size_mb * units.MiB) self._execute('dd', 'if=/dev/zero', 'of=%s' % path, 'bs=%dM' % block_size_mb, @@ -154,6 +159,9 @@ class RemoteFsDriver(driver.VolumeDriver): LOG.debug("shares loaded: %s", self.shares) + def _get_mount_point_for_share(self, path): + raise NotImplementedError() + class NfsDriver(RemoteFsDriver): """NFS based cinder driver. Creates file on NFS share for using it @@ -176,6 +184,19 @@ class NfsDriver(RemoteFsDriver): msg = _("NFS config file at %(config)s doesn't exist") % locals() LOG.warn(msg) raise exception.NfsException(msg) + if not self.configuration.nfs_oversub_ratio > 0: + msg = _("NFS config 'nfs_oversub_ratio' invalid. Must be > 0: " + "%s") % self.configuration.nfs_oversub_ratio + + LOG.error(msg) + raise exception.NfsException(msg) + + if ((not self.configuration.nfs_used_ratio > 0) and + (self.configuration.nfs_used_ratio <= 1)): + msg = _("NFS config 'nfs_used_ratio' invalid. Must be > 0 " + "and <= 1.0: %s") % self.configuration.nfs_used_ratio + LOG.error(msg) + raise exception.NfsException(msg) self.shares = {} # address : options @@ -242,11 +263,11 @@ class NfsDriver(RemoteFsDriver): } def terminate_connection(self, volume, connector, **kwargs): - """Disallow connection from connector""" + """Disallow connection from connector.""" pass def _do_create_volume(self, volume): - """Create a volume on given nfs_share + """Create a volume on given nfs_share. :param volume: volume reference """ volume_path = self.local_path(volume) @@ -260,7 +281,7 @@ class NfsDriver(RemoteFsDriver): self._set_rw_permissions_for_all(volume_path) def _ensure_shares_mounted(self): - """Look for NFS shares in the flags and tries to mount them locally""" + """Look for NFS shares in the flags and tries to mount them locally.""" self._mounted_shares = [] self._load_shares_config(self.configuration.nfs_shares_config) @@ -275,34 +296,80 @@ class NfsDriver(RemoteFsDriver): LOG.debug('Available shares %s' % str(self._mounted_shares)) def _ensure_share_mounted(self, nfs_share): - """Mount NFS share - :param nfs_share: string - """ mount_path = self._get_mount_point_for_share(nfs_share) self._mount_nfs(nfs_share, mount_path, ensure=True) - def _find_share(self, volume_size_for): - """Choose NFS share among available ones for given volume size. Current - implementation looks for greatest capacity - :param volume_size_for: int size in Gb + def _find_share(self, volume_size_in_gib): + """Choose NFS share among available ones for given volume size. + + First validation step: ratio of actual space (used_space / total_space) + is less than 'nfs_used_ratio'. + + Second validation step: apparent space allocated (differs from actual + space used when using sparse files) and compares the apparent available + space (total_available * nfs_oversub_ratio) to ensure enough space is + available for the new volume. + + 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 """ if not self._mounted_shares: raise exception.NfsNoSharesMounted() - greatest_size = 0 - greatest_share = None + target_share = None + target_share_reserved = 0 + + used_ratio = self.configuration.nfs_used_ratio + oversub_ratio = self.configuration.nfs_oversub_ratio + + requested_volume_size = volume_size_in_gib * units.GiB for nfs_share in self._mounted_shares: - capacity = self._get_available_capacity(nfs_share)[0] - if capacity > greatest_size: - greatest_share = nfs_share - greatest_size = capacity + total_size, total_available, total_allocated = \ + self._get_capacity_info(nfs_share) + used = (total_size - total_available) / total_size + if used > used_ratio: + # NOTE(morganfainberg): We check the used_ratio first since + # with oversubscription it is possible to not have the actual + # available space but be within our oversubscription limit + # therefore allowing this share to still be selected as a valid + # target. + LOG.debug(_('%s is above nfs_used_ratio'), nfs_share) + continue + if oversub_ratio >= 1.0: + # NOTE(morganfainberg): If we are setup for oversubscription + # we need to calculate the apparent available space instead + # of just using the _actual_ available space. + if (total_available * oversub_ratio) < requested_volume_size: + LOG.debug(_('%s is above nfs_oversub_ratio'), nfs_share) + continue + elif total_available <= requested_volume_size: + LOG.debug(_('%s is above nfs_oversub_ratio'), nfs_share) + continue + + if total_allocated / total_size >= oversub_ratio: + LOG.debug(_('%s reserved space is above nfs_oversub_ratio'), + nfs_share) + continue + + if target_share is not None: + if target_share_reserved > total_allocated: + target_share = nfs_share + target_share_reserved = total_allocated + else: + target_share = nfs_share + target_share_reserved = total_allocated - if volume_size_for * 1024 * 1024 * 1024 > greatest_size: + if target_share is None: raise exception.NfsNoSuitableShareFound( - volume_size=volume_size_for) - return greatest_share + volume_size=volume_size_in_gib) + + LOG.debug(_('Selected %s as target nfs share.'), target_share) + + return target_share def _get_mount_point_for_share(self, nfs_share): """ @@ -311,32 +378,25 @@ class NfsDriver(RemoteFsDriver): return os.path.join(self.configuration.nfs_mount_point_base, self._get_hash_str(nfs_share)) - def _get_available_capacity(self, nfs_share): - """Calculate available space on the NFS share + def _get_capacity_info(self, nfs_share): + """Calculate available space on the NFS share. :param nfs_share: example 172.18.194.100:/var/nfs """ mount_point = self._get_mount_point_for_share(nfs_share) - out, _ = self._execute('df', '-P', '-B', '1', mount_point, - run_as_root=True) - out = out.splitlines()[1] - - available = 0 - - size = int(out.split()[1]) - if self.configuration.nfs_disk_util == 'df': - available = int(out.split()[3]) - else: - out, _ = self._execute('du', '-sb', '--apparent-size', - '--exclude', '*snapshot*', mount_point, - run_as_root=True) - used = int(out.split()[0]) - available = size - used + df, _ = self._execute('df', '-P', '-B', '1', mount_point, + run_as_root=True) + df = df.splitlines()[1] + total_available = float(df.split()[3]) + total_size = float(df.split()[1]) - return available, size + du, _ = self._execute('du', '-sb', '--apparent-size', '--exclude', + '*snapshot*', mount_point, run_as_root=True) + total_allocated = float(du.split()[0]) + return total_size, total_available, total_allocated def _mount_nfs(self, nfs_share, mount_path, ensure=False): - """Mount NFS share to mount path""" + """Mount NFS share to mount path.""" self._execute('mkdir', '-p', mount_path) # Construct the NFS mount command. @@ -379,12 +439,12 @@ class NfsDriver(RemoteFsDriver): global_capacity = 0 global_free = 0 for nfs_share in self._mounted_shares: - free, capacity = self._get_available_capacity(nfs_share) + capacity, free, allocated = self._get_capacity_info(nfs_share) global_capacity += capacity global_free += free - data['total_capacity_gb'] = global_capacity / 1024.0 ** 3 - data['free_capacity_gb'] = global_free / 1024.0 ** 3 + data['total_capacity_gb'] = global_capacity / float(units.GiB) + data['free_capacity_gb'] = global_free / float(units.GiB) data['reserved_percentage'] = 0 data['QoS_support'] = False self._stats = data diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index b3a8afc94..055d698f7 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -1035,9 +1035,6 @@ # Base dir where nfs expected to be mounted (string value) #nfs_mount_point_base=$state_path/mnt -# Use du or df for free space calculation (string value) -#nfs_disk_util=df - # Create volumes as sparsed files which take no space.If set # to False volume is created as regular file.In such case # volume creation takes a lot of time. (boolean value) @@ -1047,6 +1044,16 @@ # nfs man page for details (string value) #nfs_mount_options= +# Percent of ACTUAL usage of the underlying volume before no +# new volumes can be allocated to the volume destination. (floating point +# value) +#nfs_used_ratio=0.95 + +# This will compare the allocated to available space on the +# volume destination. If the ratio exceeds this number, the +# destination will no longer be valid. (floating point value) +#nfs_oversub_ratio=1.0 + # # Options defined in cinder.volume.drivers.rbd