openstack
/
ironic-lib
Code Issues Proposed changes

utils.mounted: retry umount and allow retrying mount 

It seems common in IPA to retry umount 3 times, so default to it.
Also permit retrying mount as well just in case.

Change-Id: I79fc069ff07af37bfc8109421f79c3175c4b2749
This commit is contained in:
Dmitry Tantsur 2021-02-10 18:30:53 +01:00
parent 7fa890fb01
commit 3db78b001c
2 changed files with 28 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
ironic_lib/tests/test_utils.py
View File

@ -680,8 +680,9 @@ class MountedTestCase(base.IronicLibTestCase):
self.assertIs(path, mock_temp.return_value)
mock_execute.assert_has_calls([
mock.call("mount", '/dev/fake', mock_temp.return_value,
run_as_root=True),
mock.call("umount", mock_temp.return_value, run_as_root=True),
run_as_root=True, attempts=1, delay_on_retry=True),
mock.call("umount", mock_temp.return_value, run_as_root=True,
attempts=3, delay_on_retry=True),
])
mock_rmtree.assert_called_once_with(mock_temp.return_value)
@ -689,8 +690,10 @@ class MountedTestCase(base.IronicLibTestCase):
with utils.mounted('/dev/fake', '/mnt/fake') as path:
self.assertEqual('/mnt/fake', path)
mock_execute.assert_has_calls([
mock.call("mount", '/dev/fake', '/mnt/fake', run_as_root=True),
mock.call("umount", '/mnt/fake', run_as_root=True),
mock.call("mount", '/dev/fake', '/mnt/fake', run_as_root=True,
attempts=1, delay_on_retry=True),
mock.call("umount", '/mnt/fake', run_as_root=True,
attempts=3, delay_on_retry=True),
])
self.assertFalse(mock_temp.called)
self.assertFalse(mock_rmtree.called)
@ -701,8 +704,9 @@ class MountedTestCase(base.IronicLibTestCase):
self.assertEqual('/mnt/fake', path)
mock_execute.assert_has_calls([
mock.call("mount", '/dev/fake', '/mnt/fake', '-o', 'ro,foo=bar',
run_as_root=True),
mock.call("umount", '/mnt/fake', run_as_root=True),
run_as_root=True, attempts=1, delay_on_retry=True),
mock.call("umount", '/mnt/fake', run_as_root=True,
attempts=3, delay_on_retry=True),
])
def test_with_type(self, mock_temp, mock_execute, mock_rmtree):
@ -711,8 +715,9 @@ class MountedTestCase(base.IronicLibTestCase):
self.assertEqual('/mnt/fake', path)
mock_execute.assert_has_calls([
mock.call("mount", '/dev/fake', '/mnt/fake', '-t', 'iso9660',
run_as_root=True),
mock.call("umount", '/mnt/fake', run_as_root=True),
run_as_root=True, attempts=1, delay_on_retry=True),
mock.call("umount", '/mnt/fake', run_as_root=True,
attempts=3, delay_on_retry=True),
])
def test_failed_to_mount(self, mock_temp, mock_execute, mock_rmtree):
@ -720,7 +725,9 @@ class MountedTestCase(base.IronicLibTestCase):
self.assertRaises(OSError, utils.mounted('/dev/fake').__enter__)
mock_execute.assert_called_once_with("mount", '/dev/fake',
mock_temp.return_value,
run_as_root=True)
run_as_root=True,
attempts=1,
delay_on_retry=True)
mock_rmtree.assert_called_once_with(mock_temp.return_value)
def test_failed_to_unmount(self, mock_temp, mock_execute, mock_rmtree):
@ -729,7 +736,9 @@ class MountedTestCase(base.IronicLibTestCase):
with utils.mounted('/dev/fake', '/mnt/fake') as path:
self.assertEqual('/mnt/fake', path)
mock_execute.assert_has_calls([
mock.call("mount", '/dev/fake', '/mnt/fake', run_as_root=True),
mock.call("umount", '/mnt/fake', run_as_root=True),
mock.call("mount", '/dev/fake', '/mnt/fake', run_as_root=True,
attempts=1, delay_on_retry=True),
mock.call("umount", '/mnt/fake', run_as_root=True,
attempts=3, delay_on_retry=True),
])
self.assertFalse(mock_rmtree.called)

11
ironic_lib/utils.py
View File

@ -591,7 +591,8 @@ def get_route_source(dest, ignore_link_local=True):
@contextlib.contextmanager
def mounted(source, dest=None, opts=None, fs_type=None):
def mounted(source, dest=None, opts=None, fs_type=None,
mount_attempts=1, umount_attempts=3):
"""A context manager for a temporary mount.
:param source: A device to mount.
@ -600,6 +601,8 @@ def mounted(source, dest=None, opts=None, fs_type=None):
not removed.
:param opts: Mount options (``-o`` argument).
:param fs_type: File system type (``-t`` argument).
:param mount_attempts: A number of attempts to mount the device.
:param umount_attempts: A number of attempts to unmount the device.
:returns: A generator yielding the destination.
"""
params = []
@ -616,13 +619,15 @@ def mounted(source, dest=None, opts=None, fs_type=None):
mounted = False
try:
execute("mount", source, dest, *params, run_as_root=True)
execute("mount", source, dest, *params, run_as_root=True,
attempts=mount_attempts, delay_on_retry=True)
mounted = True
yield dest
finally:
if mounted:
try:
execute("umount", dest, run_as_root=True)
execute("umount", dest, run_as_root=True,
attempts=umount_attempts, delay_on_retry=True)
except (EnvironmentError,
processutils.ProcessExecutionError) as exc:
LOG.warning(