From 53e75f6ed64342f1dd07f6a8bae857b8592ebf97 Mon Sep 17 00:00:00 2001
From: Alex Meade <mr.alex.meade@gmail.com>
Date: Thu, 7 Aug 2014 10:08:36 -0400
Subject: [PATCH] Log an error on nfs mount failure

Change from a log warning to a log error when an nfs driver
cannot mount the backend share. Add tests to ensure the error is
logged correctly.

Change-Id: Id9d51aebe349e7a4e09d1211b17efd8d47af97f5
---
 cinder/brick/remotefs/remotefs.py |  2 +-
 cinder/test.py                    | 14 ++++++++++++++
 cinder/tests/test_nfs.py          | 27 ++++++++++++++-------------
 cinder/volume/drivers/remotefs.py |  2 +-
 4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/cinder/brick/remotefs/remotefs.py b/cinder/brick/remotefs/remotefs.py
index 50c6bf84a..b20ebb96a 100755
--- a/cinder/brick/remotefs/remotefs.py
+++ b/cinder/brick/remotefs/remotefs.py
@@ -133,7 +133,7 @@ class RemoteFsClient(object):
             except Exception as e:
                 mnt_errors[mnt_type] = six.text_type(e)
                 LOG.debug('Failed to do %s mount.', mnt_type)
-        raise exception.BrickException(_("NFS mount failed for share %(sh)s."
+        raise exception.BrickException(_("NFS mount failed for share %(sh)s. "
                                          "Error - %(error)s")
                                        % {'sh': nfs_share,
                                           'error': mnt_errors})
diff --git a/cinder/test.py b/cinder/test.py
index 3c5321fb1..5f36942bc 100644
--- a/cinder/test.py
+++ b/cinder/test.py
@@ -28,6 +28,7 @@ import tempfile
 import uuid
 
 import fixtures
+import mock
 import mox
 from oslo.config import cfg
 from oslo.messaging import conffixture as messaging_conffixture
@@ -239,6 +240,19 @@ class TestCase(testtools.TestCase):
         self._services.append(svc)
         return svc
 
+    def mock_object(self, obj, attr_name, new_attr=None, **kwargs):
+        """Use python mock to mock an object attribute
+
+        Mocks the specified objects attribute with the given value.
+        Automatically performs 'addCleanup' for the mock.
+
+        """
+        if not new_attr:
+            new_attr = mock.Mock()
+        patcher = mock.patch.object(obj, attr_name, new_attr, **kwargs)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+
     # Useful assertions
     def assertDictMatch(self, d1, d2, approx_equal=False, tolerance=0.001):
         """Assert two dicts are equivalent.
diff --git a/cinder/tests/test_nfs.py b/cinder/tests/test_nfs.py
index d9f9ad8b5..f3693ab3f 100644
--- a/cinder/tests/test_nfs.py
+++ b/cinder/tests/test_nfs.py
@@ -1,4 +1,3 @@
-
 # Copyright (c) 2012 NetApp, Inc.
 # All Rights Reserved.
 #
@@ -323,26 +322,28 @@ class NfsDriverTestCase(test.TestCase):
 
     def test_ensure_shares_mounted_should_not_save_mounting_with_error(self):
         """_ensure_shares_mounted should not save share if failed to mount."""
-        mox = self._mox
-        drv = self._driver
 
-        mox.StubOutWithMock(drv, '_read_config_file')
         config_data = []
         config_data.append(self.TEST_NFS_EXPORT1)
-        drv._read_config_file(self.TEST_SHARES_CONFIG_FILE).\
-            AndReturn(config_data)
+        self._driver.configuration.nfs_shares_config =\
+            self.TEST_SHARES_CONFIG_FILE
 
-        mox.StubOutWithMock(drv, '_ensure_share_mounted')
-        drv.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE
-        drv._ensure_share_mounted(self.TEST_NFS_EXPORT1).AndRaise(Exception())
+        self.mock_object(self._driver, '_read_config_file',
+                         mock.Mock(return_value=config_data))
+        self.mock_object(self._driver, '_ensure_share_mounted',
+                         mock.Mock(side_effect=Exception()))
+        self.mock_object(remotefs, 'LOG')
 
-        mox.ReplayAll()
+        self._driver._ensure_shares_mounted()
 
-        drv._ensure_shares_mounted()
+        self.assertEqual(0, len(self._driver._mounted_shares))
+        self._driver._read_config_file.assert_called_once_with(
+            self.TEST_SHARES_CONFIG_FILE)
 
-        self.assertEqual(0, len(drv._mounted_shares))
+        self._driver._ensure_share_mounted.assert_called_once_with(
+            self.TEST_NFS_EXPORT1)
 
-        mox.VerifyAll()
+        self.assertEqual(1, remotefs.LOG.error.call_count)
 
     def test_setup_should_throw_error_if_shares_config_not_configured(self):
         """do_setup should throw error if shares config is not configured."""
diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py
index 3a07c1a6a..d72d57f5d 100644
--- a/cinder/volume/drivers/remotefs.py
+++ b/cinder/volume/drivers/remotefs.py
@@ -149,7 +149,7 @@ class RemoteFSDriver(driver.VolumeDriver):
                 self._ensure_share_mounted(share)
                 self._mounted_shares.append(share)
             except Exception as exc:
-                LOG.warning(_('Exception during mounting %s') % (exc,))
+                LOG.error(_('Exception during mounting %s') % (exc,))
 
         LOG.debug('Available shares %s' % self._mounted_shares)
 
-- 
2.45.2