From bc3241cf7f06871f8c15d04b77871185625165e2 Mon Sep 17 00:00:00 2001 From: Tom Barron Date: Wed, 26 Nov 2014 16:01:14 -0500 Subject: [PATCH] Fixes intermittent NFS driver mount failure During cinder volume driver initialization, NFS drivers often fail to mount the NFS share backing their volumes, complaining that the share in question is 'busy or already mounted'. This commit introduces a retry loop around the ensure_mounted() call inside set_nas_security_options() so that if there is contention between volume process and backup process mounting the same share the driver will not be stopped from loading. Change-Id: I672433c1c31f420e5dcdbe565db3bb29af3abe7b Closes-bug: 1395823 --- cinder/tests/test_nfs.py | 59 ++++++++++++++++++++++++++++++++++++ cinder/volume/drivers/nfs.py | 26 ++++++++++++++-- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/cinder/tests/test_nfs.py b/cinder/tests/test_nfs.py index c7a72645f..46df23fbf 100644 --- a/cinder/tests/test_nfs.py +++ b/cinder/tests/test_nfs.py @@ -16,6 +16,7 @@ import errno import os +import random import mock import mox as mox_lib @@ -365,6 +366,7 @@ class NfsDriverTestCase(test.TestCase): self.configuration.nfs_oversub_ratio = 1.0 self.configuration.nfs_mount_point_base = self.TEST_MNT_POINT_BASE self.configuration.nfs_mount_options = None + self.configuration.nfs_mount_attempts = 3 self.configuration.nas_secure_file_permissions = 'false' self.configuration.nas_secure_file_operations = 'false' self.configuration.volume_dd_blocksize = '1M' @@ -1039,3 +1041,60 @@ class NfsDriverTestCase(test.TestCase): self.assertEqual(drv.configuration.nas_secure_file_permissions, 'false') self.assertTrue(LOG.warn.called) + + def test_set_nas_security_options_exception_if_no_mounted_shares(self): + """Ensure proper exception is raised if there are no mounted shares.""" + + drv = self._driver + drv._ensure_shares_mounted = mock.Mock() + drv._mounted_shares = [] + is_new_cinder_install = 'does not matter' + + self.assertRaises(exception.NfsNoSharesMounted, + drv.set_nas_security_options, + is_new_cinder_install) + + def test_ensure_share_mounted(self): + """Case where the mount works the first time.""" + + num_attempts = random.randint(1, 5) + self.mock_object(self._driver._remotefsclient, 'mount') + drv = self._driver + drv.configuration.nfs_mount_attempts = num_attempts + drv.shares = {self.TEST_NFS_EXPORT1: ''} + + drv._ensure_share_mounted(self.TEST_NFS_EXPORT1) + + drv._remotefsclient.mount.called_once() + + def test_ensure_share_mounted_exception(self): + """Make the configured number of attempts when mounts fail.""" + + num_attempts = random.randint(1, 5) + self.mock_object(self._driver._remotefsclient, 'mount', + mock.Mock(side_effect=Exception)) + drv = self._driver + drv.configuration.nfs_mount_attempts = num_attempts + drv.shares = {self.TEST_NFS_EXPORT1: ''} + + self.assertRaises(exception.NfsException, drv._ensure_share_mounted, + self.TEST_NFS_EXPORT1) + + self.assertEqual(num_attempts, drv._remotefsclient.mount.call_count) + + def test_ensure_share_mounted_at_least_one_attempt(self): + """Make at least one mount attempt even if configured for less.""" + + min_num_attempts = 1 + num_attempts = 0 + self.mock_object(self._driver._remotefsclient, 'mount', + mock.Mock(side_effect=Exception)) + drv = self._driver + drv.configuration.nfs_mount_attempts = num_attempts + drv.shares = {self.TEST_NFS_EXPORT1: ''} + + self.assertRaises(exception.NfsException, drv._ensure_share_mounted, + self.TEST_NFS_EXPORT1) + + self.assertEqual(min_num_attempts, + drv._remotefsclient.mount.call_count) diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 9276ab9a0..3b70b2128 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -15,14 +15,16 @@ import errno import os +import time from oslo.concurrency import processutils as putils from oslo.config import cfg from oslo.utils import units +import six from cinder.brick.remotefs import remotefs as remotefs_brick from cinder import exception -from cinder.i18n import _ +from cinder.i18n import _, _LE from cinder.image import image_utils from cinder.openstack.common import log as logging from cinder import utils @@ -58,6 +60,12 @@ nfs_opts = [ default=None, help=('Mount options passed to the nfs client. See section ' 'of the nfs man page for details.')), + cfg.IntOpt('nfs_mount_attempts', + default=3, + help=('The number of attempts to mount nfs shares before ' + 'raising an error. At least one attempt will be ' + 'made to mount an nfs share, regardless of the ' + 'value specified.')), ] CONF = cfg.CONF @@ -148,7 +156,21 @@ class NfsDriver(remotefs.RemoteFSDriver): mnt_flags = [] if self.shares.get(nfs_share) is not None: mnt_flags = self.shares[nfs_share].split() - self._remotefsclient.mount(nfs_share, mnt_flags) + num_attempts = max(1, self.configuration.nfs_mount_attempts) + for attempt in range(num_attempts): + try: + self._remotefsclient.mount(nfs_share, mnt_flags) + return + except Exception as e: + if attempt == (num_attempts - 1): + LOG.error(_LE('Mount failure for %(share)s after ' + '%(count)d attempts.') % { + 'share': nfs_share, + 'count': num_attempts}) + raise exception.NfsException(e) + LOG.debug('Mount attempt %d failed: %s.\nRetrying mount ...' % + (attempt, six.text_type(e))) + time.sleep(1) def _find_share(self, volume_size_in_gib): """Choose NFS share among available ones for given volume size. -- 2.45.2