diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 3a9bd3f8f0..d3ef00586d 100755 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -592,12 +592,6 @@ function install_manila { sudo apt-get install -y build-essential sudo apt-get install -y ubuntu-zfs sudo modprobe zfs - - # TODO(vponomaryov): remove following line when we have this - # in 'requirements.txt' file. - # Package 'nsenter' is expected to be installed on host with - # ZFS, if it is remote for manila-share service host. - sudo pip install nsenter else echo "Manila Devstack plugin does not support installation "\ "of ZFS packages for non-'Ubuntu-trusty' distros. "\ diff --git a/etc/manila/rootwrap.d/share.filters b/etc/manila/rootwrap.d/share.filters index d9ca03cb1e..7dd79e6e51 100644 --- a/etc/manila/rootwrap.d/share.filters +++ b/etc/manila/rootwrap.d/share.filters @@ -146,9 +146,6 @@ zpool: CommandFilter, zpool, root # manila/share/drivers/zfsonlinux/utils.py zfs: CommandFilter, zfs, root -# manila/share/drivers/zfsonlinux/driver.py -nsenter: CommandFilter, nsenter, root - # LXD driver commands # manila/share/drivers/lxd.py lxc: CommandFilter, lxc, root diff --git a/manila/share/drivers/zfsonlinux/driver.py b/manila/share/drivers/zfsonlinux/driver.py index 5e82de3221..f312527468 100644 --- a/manila/share/drivers/zfsonlinux/driver.py +++ b/manila/share/drivers/zfsonlinux/driver.py @@ -27,7 +27,7 @@ from oslo_utils import timeutils from manila.common import constants from manila import exception -from manila.i18n import _, _LW +from manila.i18n import _, _LI, _LW from manila.share import driver from manila.share.drivers.zfsonlinux import utils as zfs_utils from manila.share import utils as share_utils @@ -186,46 +186,29 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver): msg=_("Could not destroy '%s' dataset, " "because it had opened files.") % name) - try: - self.zfs('destroy', '-f', name) - return - except exception.ProcessExecutionError as e: - LOG.debug("Failed to run command, got error: %s\n" - "Assuming other namespace-based services hold " - "ZFS mounts.", e) - - # NOTE(vponomaryov): perform workaround for Neutron bug #1546723 - # We should release ZFS mount from all namespaces. It should not be - # there at all. - get_pids_cmd = ( - "(echo $(grep -s %s /proc/*/mounts) ) 2>&1 " % mountpoint) - try: - raw_pids, err = self.execute('bash', '-c', get_pids_cmd) - except exception.ProcessExecutionError as e: - LOG.warning( - _LW("Failed to get list of PIDs that hold ZFS dataset " - "mountpoint. Got following error: %s"), e) - else: - pids = [s.split('/')[0] for s in raw_pids.split('/proc/') if s] - LOG.debug( - "List of pids that hold ZFS mount '%(mnt)s': %(list)s", { - 'mnt': mountpoint, 'list': ' '.join(pids)}) - for pid in pids: - try: - self.execute( - 'sudo', 'nsenter', '--mnt', '--target=%s' % pid, - '/bin/umount', mountpoint) - except exception.ProcessExecutionError as e: - LOG.warning( - _LW("Failed to run command with release of " - "ZFS dataset mount, got error: %s"), e) - - # NOTE(vponomaryov): sleep some time after unmount operations. - time.sleep(1) - # NOTE(vponomaryov): Now, when no file usages and mounts of dataset # exist, destroy dataset. - self.zfs_with_retry('destroy', '-f', name) + try: + self.zfs('destroy', '-f', name) + return + except exception.ProcessExecutionError: + LOG.info(_LI("Failed to destroy ZFS dataset, retrying one time")) + + # NOTE(bswartz): There appears to be a bug in ZFS when creating and + # destroying datasets concurrently where the filesystem remains mounted + # even though ZFS thinks it's unmounted. The most reliable workaround + # I've found is to force the unmount, then retry the destroy, with + # short pauses around the unmount. + time.sleep(1) + try: + self.execute('sudo', 'umount', mountpoint) + except exception.ProcessExecutionError: + # Ignore failed umount, it's normal + pass + time.sleep(1) + + # This time the destroy is expected to succeed. + self.zfs('destroy', '-f', name) def _setup_helpers(self): """Setups share helper for ZFS backend.""" @@ -268,6 +251,11 @@ class ZFSonLinuxShareDriver(zfs_utils.ExecuteMixin, driver.ShareDriver): reason=_("No zpools specified for usage: " "%s") % self.zpool_list) + # Make pool mounts shared so that cloned namespaces receive unmounts + # and don't prevent us from unmounting datasets + for zpool in self.configuration.zfs_zpool_list: + self.execute('sudo', 'mount', '--make-rshared', ('/%s' % zpool)) + if self.configuration.zfs_use_ssh: # Check workability of SSH executor self.ssh_executor('whoami') diff --git a/manila/share/drivers/zfsonlinux/utils.py b/manila/share/drivers/zfsonlinux/utils.py index c81e42bbbd..41c1efe891 100644 --- a/manila/share/drivers/zfsonlinux/utils.py +++ b/manila/share/drivers/zfsonlinux/utils.py @@ -124,10 +124,6 @@ class ExecuteMixin(driver.ExecuteMixin): """ZFS shell commands executor.""" return self.execute('sudo', 'zfs', *cmd, **kwargs) - def zfs_with_retry(self, *cmd, **kwargs): - """ZFS shell commands executor with retries.""" - return self.execute_with_retry('sudo', 'zfs', *cmd, **kwargs) - @six.add_metaclass(abc.ABCMeta) class NASHelperBase(object): diff --git a/manila/tests/share/drivers/zfsonlinux/test_driver.py b/manila/tests/share/drivers/zfsonlinux/test_driver.py index f0bba2a4c2..c87a00f774 100644 --- a/manila/tests/share/drivers/zfsonlinux/test_driver.py +++ b/manila/tests/share/drivers/zfsonlinux/test_driver.py @@ -121,8 +121,7 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase): self.assertTrue(hasattr(self.driver, '_helpers')) self.assertEqual(self.driver._helpers, {}) for attr_name in ('execute', 'execute_with_retry', 'parse_zfs_answer', - 'get_zpool_option', 'get_zfs_option', 'zfs', - 'zfs_with_retry'): + 'get_zpool_option', 'get_zfs_option', 'zfs'): self.assertTrue(hasattr(self.driver, attr_name)) def test_init_error_with_duplicated_zpools(self): @@ -177,9 +176,9 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase): self.driver._setup_helpers.assert_called_once_with() if use_ssh: - self.driver.ssh_executor.assert_called_once_with('whoami') + self.assertEqual(4, self.driver.ssh_executor.call_count) else: - self.assertEqual(0, self.driver.ssh_executor.call_count) + self.assertEqual(3, self.driver.ssh_executor.call_count) @ddt.data( ('foo', '127.0.0.1'), @@ -860,13 +859,13 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase): def test__delete_dataset_or_snapshot_with_retry_snapshot(self): self.mock_object(self.driver, 'get_zfs_option') - self.mock_object(self.driver, 'zfs_with_retry') + self.mock_object(self.driver, 'zfs') self.driver._delete_dataset_or_snapshot_with_retry('foo@bar') self.driver.get_zfs_option.assert_called_once_with( 'foo@bar', 'mountpoint') - self.driver.zfs_with_retry.assert_called_once_with( + self.driver.zfs.assert_called_once_with( 'destroy', '-f', 'foo@bar') def test__delete_dataset_or_snapshot_with_retry_of(self): @@ -917,92 +916,29 @@ class ZFSonLinuxShareDriverTestCase(test.TestCase): zfs_driver.time.sleep.assert_called_once_with(2) self.driver.zfs.assert_called_once_with('destroy', '-f', dataset_name) - def test__delete_dataset_or_snapshot_with_retry_mount_holders(self): - mnt = 'fake_mnt' + def test__delete_dataset_or_snapshot_with_retry_busy(self): + self.mock_object(self.driver, 'get_zfs_option') self.mock_object( - self.driver, 'get_zfs_option', mock.Mock(return_value=mnt)) - self.mock_object(self.driver, 'zfs_with_retry') + self.driver, 'execute', mock.Mock( + side_effect=exception.ProcessExecutionError( + 'FAKE lsof returns not found'))) self.mock_object( - self.driver, 'zfs', - mock.Mock(side_effect=exception.ProcessExecutionError('FAKE'))) - pids_raw = '/proc/1234/foo /proc/5678/bar' - self.mock_object( - self.driver, 'execute', mock.Mock(side_effect=[ - ('a', 'b'), - exception.ProcessExecutionError('Fake lsof empty'), - (pids_raw, 'c'), - exception.ProcessExecutionError('Fake PID not found error'), - ('d', 'e'), - ])) + self.driver, 'zfs', mock.Mock(side_effect=[ + exception.ProcessExecutionError( + 'cannot destroy FAKE: dataset is busy\n'), + None, None])) self.mock_object(zfs_driver.time, 'sleep') - self.mock_object(zfs_driver.LOG, 'debug') - self.mock_object(zfs_driver.LOG, 'warning') - self.mock_object( - zfs_driver.time, 'time', mock.Mock(side_effect=range(1, 70, 2))) + self.mock_object(zfs_driver.LOG, 'info') dataset_name = 'fake/dataset/name' self.driver._delete_dataset_or_snapshot_with_retry(dataset_name) self.driver.get_zfs_option.assert_called_once_with( dataset_name, 'mountpoint') - self.assertEqual(3, zfs_driver.time.time.call_count) - self.assertEqual(3, zfs_driver.LOG.debug.call_count) - self.assertEqual(5, self.driver.execute.call_count) - self.driver.execute.assert_has_calls([ - mock.call('lsof', '-w', mnt), - mock.call('lsof', '-w', mnt), - mock.call('bash', '-c', - '(echo $(grep -s %s /proc/*/mounts) ) 2>&1 ' % mnt), - mock.call('sudo', 'nsenter', '--mnt', '--target=1234', - '/bin/umount', mnt), - mock.call('sudo', 'nsenter', '--mnt', '--target=5678', - '/bin/umount', mnt), - ]) - zfs_driver.time.sleep.assert_has_calls([mock.call(2), mock.call(1)]) - self.driver.zfs.assert_called_once_with('destroy', '-f', dataset_name) - zfs_driver.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY) - self.driver.zfs_with_retry.asssert_called_once_with( - 'destroy', '-f', dataset_name) - - def test__delete_dataset_or_snapshot_with_retry_mount_holders_error(self): - mnt = 'fake_mnt' - self.mock_object( - self.driver, 'get_zfs_option', mock.Mock(return_value=mnt)) - self.mock_object(self.driver, 'zfs_with_retry') - self.mock_object( - self.driver, 'zfs', - mock.Mock(side_effect=exception.ProcessExecutionError('FAKE'))) - self.mock_object( - self.driver, 'execute', mock.Mock(side_effect=[ - ('a', 'b'), - exception.ProcessExecutionError('Fake lsof empty'), - exception.ProcessExecutionError('Failed to get list of PIDs'), - ])) - self.mock_object(zfs_driver.time, 'sleep') - self.mock_object(zfs_driver.LOG, 'debug') - self.mock_object(zfs_driver.LOG, 'warning') - self.mock_object( - zfs_driver.time, 'time', mock.Mock(side_effect=range(1, 70, 2))) - dataset_name = 'fake/dataset/name' - - self.driver._delete_dataset_or_snapshot_with_retry(dataset_name) - - self.driver.get_zfs_option.assert_called_once_with( - dataset_name, 'mountpoint') - self.assertEqual(3, zfs_driver.time.time.call_count) - self.assertEqual(2, zfs_driver.LOG.debug.call_count) - self.assertEqual(3, self.driver.execute.call_count) - self.driver.execute.assert_has_calls([ - mock.call('lsof', '-w', mnt), - mock.call('lsof', '-w', mnt), - mock.call('bash', '-c', - '(echo $(grep -s %s /proc/*/mounts) ) 2>&1 ' % mnt), - ]) - zfs_driver.time.sleep.assert_has_calls([mock.call(2), mock.call(1)]) - self.driver.zfs.assert_called_once_with('destroy', '-f', dataset_name) - zfs_driver.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY) - self.driver.zfs_with_retry.asssert_called_once_with( - 'destroy', '-f', dataset_name) + self.assertEqual(2, zfs_driver.time.sleep.call_count) + self.assertEqual(2, self.driver.execute.call_count) + self.assertEqual(1, zfs_driver.LOG.info.call_count) + self.assertEqual(2, self.driver.zfs.call_count) def test_create_replica(self): active_replica = { diff --git a/manila/tests/share/drivers/zfsonlinux/test_utils.py b/manila/tests/share/drivers/zfsonlinux/test_utils.py index fe6af57f13..bdf1c30994 100644 --- a/manila/tests/share/drivers/zfsonlinux/test_utils.py +++ b/manila/tests/share/drivers/zfsonlinux/test_utils.py @@ -213,16 +213,6 @@ foo_res opt_3 some_value local""" self.driver.execute.asssert_called_once_with( 'sudo', 'zfs', 'foo', 'bar') - def test_zfs_with_retrying(self): - self.mock_object(self.driver, 'execute') - self.mock_object(self.driver, 'execute_with_retry') - - self.driver.zfs_with_retry('foo', 'bar') - - self.assertEqual(0, self.driver.execute.call_count) - self.driver.execute_with_retry.asssert_called_once_with( - 'sudo', 'zfs', 'foo', 'bar') - @ddt.ddt class NFSviaZFSHelperTestCase(test.TestCase):