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

diff --git a/cinder/tests/test_nfs.py b/cinder/tests/test_nfs.py
index c7a72645fbc6ca3d6a78d7c08f0e0fcb7eb20ccd..46df23fbfd49eb46c8cc4d8e66ee868071904c56 100644 (file)
--- 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 9276ab9a05f5c9f49d8daef2b1b5bef80476177d..3b70b212895b409b6412a1ae778cd689326c7862 100644 (file)
--- 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.