glusterfs: Honor mount options when restarting cinder service

When restarting cinder service (cinder-volume specifically),
ensure that the gluster mounts are unmounted and remounted so
that any new mount options added to the shares config file
is taken effect, post service restart.

Change-Id: I9fe3442cd4899770b4c9ea4a73a7fe62967f2c64

diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py
index 8214850e34d8692b1f7ba893f868d5d95ad4b1f5..0dda5fe6ea287bd76f4c6117d340815df5ae598a 100644 (file)
--- a/cinder/tests/test_glusterfs.py
+++ b/cinder/tests/test_glusterfs.py
@@ -432,6 +432,9 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         drv._execute('mount.glusterfs', check_exit_code=False)
 
+        drv._execute('umount', '/mnt/test/8f0473c9ad824b8b6a27264b9cacb005',
+                     run_as_root=True)
+
         drv._execute('mkdir', '-p', mox_lib.IgnoreArg())
 
         os.path.exists(self.TEST_SHARES_CONFIG_FILE).AndReturn(True)
@@ -716,6 +719,208 @@ class GlusterFsDriverTestCase(test.TestCase):
             mock_local_path_volume_info.assert_called_once_with(volume)
             mock_delete_if_exists.assert_called_once_with(info_file)
 
+    def test_refresh_mounts(self):
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_unmount_shares'),
+            mock.patch.object(self._driver, '_ensure_shares_mounted')
+        ) as (mock_unmount_shares, mock_ensure_shares_mounted):
+            self._driver._refresh_mounts()
+
+            self.assertTrue(mock_unmount_shares.called)
+            self.assertTrue(mock_ensure_shares_mounted.called)
+
+    def test_refresh_mounts_with_excp(self):
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_unmount_shares'),
+            mock.patch.object(self._driver, '_ensure_shares_mounted'),
+            mock.patch.object(glusterfs, 'LOG')
+        ) as (mock_unmount_shares, mock_ensure_shares_mounted,
+              mock_logger):
+            mock_stderr = _("umount: <mnt_path>: target is busy")
+            mock_unmount_shares.side_effect = \
+                putils.ProcessExecutionError(stderr=mock_stderr)
+
+            self._driver._refresh_mounts()
+
+            self.assertTrue(mock_unmount_shares.called)
+            self.assertTrue(mock_logger.warn.called)
+            self.assertTrue(mock_ensure_shares_mounted.called)
+
+            mock_unmount_shares.reset_mock()
+            mock_ensure_shares_mounted.reset_mock()
+            mock_logger.reset_mock()
+            mock_logger.warn.reset_mock()
+
+            mock_stderr = _("umount: <mnt_path>: some other error")
+            mock_unmount_shares.side_effect = \
+                putils.ProcessExecutionError(stderr=mock_stderr)
+
+            self.assertRaises(putils.ProcessExecutionError,
+                              self._driver._refresh_mounts)
+
+            self.assertTrue(mock_unmount_shares.called)
+            self.assertFalse(mock_ensure_shares_mounted.called)
+
+    def test_unmount_shares_with_excp(self):
+        self._driver.shares = {'127.7.7.7:/gluster1': None}
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_load_shares_config'),
+            mock.patch.object(self._driver, '_do_umount'),
+            mock.patch.object(glusterfs, 'LOG')
+        ) as (mock_load_shares_config, mock_do_umount, mock_logger):
+            mock_do_umount.side_effect = Exception()
+
+            self._driver._unmount_shares()
+
+            self.assertTrue(mock_do_umount.called)
+            self.assertTrue(mock_logger.warning.called)
+            mock_logger.debug.assert_not_called()
+
+    def test_unmount_shares_1share(self):
+        self._driver.shares = {'127.7.7.7:/gluster1': None}
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_load_shares_config'),
+            mock.patch.object(self._driver, '_do_umount')
+        ) as (mock_load_shares_config, mock_do_umount):
+            self._driver._unmount_shares()
+
+            self.assertTrue(mock_do_umount.called)
+            mock_do_umount.assert_called_once_with(True,
+                                                   '127.7.7.7:/gluster1')
+
+    def test_unmount_shares_2share(self):
+        self._driver.shares = {'127.7.7.7:/gluster1': None,
+                               '127.7.7.8:/gluster2': None}
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_load_shares_config'),
+            mock.patch.object(self._driver, '_do_umount')
+        ) as (mock_load_shares_config, mock_do_umount):
+            self._driver._unmount_shares()
+
+            mock_do_umount.assert_any_call(True,
+                                           '127.7.7.7:/gluster1')
+            mock_do_umount.assert_any_call(True,
+                                           '127.7.7.8:/gluster2')
+
+    def test_do_umount(self):
+        test_share = '127.7.7.7:/gluster1'
+        test_hashpath = '/hashed/mnt/path'
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_get_mount_point_for_share'),
+            mock.patch.object(putils, 'execute')
+        ) as (mock_get_mntp_share, mock_execute):
+            mock_get_mntp_share.return_value = test_hashpath
+
+            self._driver._do_umount(True, test_share)
+
+            self.assertTrue(mock_get_mntp_share.called)
+            self.assertTrue(mock_execute.called)
+            mock_get_mntp_share.assert_called_once_with(test_share)
+
+            cmd = ['umount', test_hashpath]
+            self.assertEqual(cmd[0], mock_execute.call_args[0][0])
+            self.assertEqual(cmd[1], mock_execute.call_args[0][1])
+            self.assertEqual(True,
+                             mock_execute.call_args[1]['run_as_root'])
+
+            mock_get_mntp_share.reset_mock()
+            mock_get_mntp_share.return_value = test_hashpath
+            mock_execute.reset_mock()
+
+            self._driver._do_umount(False, test_share)
+
+            self.assertTrue(mock_get_mntp_share.called)
+            self.assertTrue(mock_execute.called)
+            mock_get_mntp_share.assert_called_once_with(test_share)
+            cmd = ['umount', test_hashpath]
+            self.assertEqual(cmd[0], mock_execute.call_args[0][0])
+            self.assertEqual(cmd[1], mock_execute.call_args[0][1])
+            self.assertEqual(True,
+                             mock_execute.call_args[1]['run_as_root'])
+
+    def test_do_umount_with_excp1(self):
+        test_share = '127.7.7.7:/gluster1'
+        test_hashpath = '/hashed/mnt/path'
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_get_mount_point_for_share'),
+            mock.patch.object(putils, 'execute'),
+            mock.patch.object(glusterfs, 'LOG')
+        ) as (mock_get_mntp_share, mock_execute, mock_logger):
+            mock_get_mntp_share.return_value = test_hashpath
+            mock_execute.side_effect = putils.ProcessExecutionError
+            self.assertRaises(putils.ProcessExecutionError,
+                              self._driver._do_umount, False,
+                              test_share)
+
+            mock_logger.reset_mock()
+            mock_logger.info.reset_mock()
+            mock_logger.error.reset_mock()
+            mock_execute.side_effect = putils.ProcessExecutionError
+            try:
+                self._driver._do_umount(False, test_share)
+            except putils.ProcessExecutionError:
+                self.assertFalse(mock_logger.info.called)
+                self.assertTrue(mock_logger.error.called)
+            except Exception as e:
+                self.fail('Unexpected exception thrown:', e)
+            else:
+                self.fail('putils.ProcessExecutionError not thrown')
+
+    def test_do_umount_with_excp2(self):
+        test_share = '127.7.7.7:/gluster1'
+        test_hashpath = '/hashed/mnt/path'
+
+        with contextlib.nested(
+            mock.patch.object(self._driver, '_get_mount_point_for_share'),
+            mock.patch.object(putils, 'execute'),
+            mock.patch.object(glusterfs, 'LOG')
+        ) as (mock_get_mntp_share, mock_execute, mock_logger):
+            mock_get_mntp_share.return_value = test_hashpath
+
+            mock_stderr = _("umount: %s: not mounted") % test_hashpath
+            mock_execute.side_effect = putils.ProcessExecutionError(
+                stderr=mock_stderr)
+
+            self._driver._do_umount(True, test_share)
+
+            self.assertTrue(mock_logger.info.called)
+            self.assertFalse(mock_logger.error.called)
+
+            mock_logger.reset_mock()
+            mock_logger.info.reset_mock()
+            mock_logger.error.reset_mock()
+            mock_stderr = _("umount: %s: target is busy") %\
+                           (test_hashpath)
+            mock_execute.side_effect = putils.ProcessExecutionError(
+                stderr=mock_stderr)
+
+            self.assertRaises(putils.ProcessExecutionError,
+                              self._driver._do_umount, True,
+                              test_share)
+
+            mock_logger.reset_mock()
+            mock_logger.info.reset_mock()
+            mock_logger.error.reset_mock()
+            mock_stderr = _('umount: %s: target is busy') %\
+                           (test_hashpath)
+            mock_execute.side_effect = putils.ProcessExecutionError(
+                stderr=mock_stderr)
+
+            try:
+                self._driver._do_umount(True, test_share)
+            except putils.ProcessExecutionError:
+                mock_logger.info.assert_not_called()
+                self.assertTrue(mock_logger.error.called)
+            except Exception as e:
+                self.fail('Unexpected exception thrown:', e)
+            else:
+                self.fail('putils.ProcessExecutionError not thrown')
+
     def test_delete_should_ensure_share_mounted(self):
         """delete_volume should ensure that corresponding share is mounted."""
         mox = self._mox

diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py
index 9df0fee262fc204cbb012492c1eda56d6ebdabed..b9ad94fe8bd0d7eb4dd9f3d8a0bc03c955b779aa 100644 (file)
--- a/cinder/volume/drivers/glusterfs.py
+++ b/cinder/volume/drivers/glusterfs.py
@@ -120,6 +120,39 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
             else:
                 raise
 
+        self._refresh_mounts()
+
+    def _unmount_shares(self):
+        self._load_shares_config(self.configuration.glusterfs_shares_config)
+        for share in self.shares.keys():
+            try:
+                self._do_umount(True, share)
+            except Exception as exc:
+                LOG.warning(_('Exception during unmounting %s') % (exc))
+
+    def _do_umount(self, ignore_not_mounted, share):
+        mount_path = self._get_mount_point_for_share(share)
+        command = ['umount', mount_path]
+        try:
+            self._execute(*command, run_as_root=True)
+        except processutils.ProcessExecutionError as exc:
+            if ignore_not_mounted and 'not mounted' in exc.stderr:
+                LOG.info(_("%s is already umounted"), share)
+            else:
+                LOG.error(_("Failed to umount %(share)s, reason=%(stderr)s"),
+                          {'share': share, 'stderr': exc.stderr})
+                raise
+
+    def _refresh_mounts(self):
+        try:
+            self._unmount_shares()
+        except processutils.ProcessExecutionError as exc:
+            if 'target is busy' in exc.stderr:
+                LOG.warn(_("Failed to refresh mounts, reason=%s") %
+                         exc.stderr)
+            else:
+                raise
+
         self._ensure_shares_mounted()
 
     def check_for_setup_error(self):

diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters
index 5b574040a7a86bb80c90fa8f5144183fdcecb404..caf8755c835a841adc2950a33602491aae704f23 100644 (file)
--- a/etc/cinder/rootwrap.d/volume.filters
+++ b/etc/cinder/rootwrap.d/volume.filters
@@ -74,6 +74,7 @@ netapp_nfs_find: RegExpFilter, find, root, find, ^[/]*([^/\0]+(/+)?)*$, -maxdept
 
 # cinder/volume/drivers/glusterfs.py
 chgrp: CommandFilter, chgrp, root
+umount: CommandFilter, umount, root
 
 # cinder/volumes/drivers/hds/hds.py:
 hus-cmd: CommandFilter, hus-cmd, root