From: Deepak C Shetty Date: Fri, 11 Apr 2014 12:19:16 +0000 (+0000) Subject: glusterfs: Honor mount options when restarting cinder service X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=df31f2ac5894e01444877b0f307ab738e74404a5;p=openstack-build%2Fcinder-build.git 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 8214850e3..0dda5fe6e 100644 --- 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: : 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: : 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 9df0fee26..b9ad94fe8 100644 --- 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 5b574040a..caf8755c8 100644 --- 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