diff --git a/pungi/phases/buildinstall.py b/pungi/phases/buildinstall.py index c75020af..b10384de 100644 --- a/pungi/phases/buildinstall.py +++ b/pungi/phases/buildinstall.py @@ -27,7 +27,7 @@ from productmd.images import Image from pungi.arch import get_valid_arches from pungi.util import get_buildroot_rpms, get_volid, get_arch_variant_data -from pungi.util import get_file_size, get_mtime, failable, makedirs +from pungi.util import get_file_size, get_mtime, failable, makedirs, run_unmount_cmd from pungi.wrappers.lorax import LoraxWrapper from pungi.wrappers.kojiwrapper import KojiWrapper from pungi.wrappers import iso @@ -273,8 +273,7 @@ def tweak_buildinstall(compose, src, dst, arch, variant, label, volid, kickstart cmd = ["cp", "-v", "--remove-destination", config_path, config_in_image] run(cmd) - cmd = ["fusermount", "-u", mount_tmp_dir] - run(cmd) + run_unmount_cmd(["fusermount", "-u", mount_tmp_dir]) shutil.rmtree(mount_tmp_dir) # HACK: make buildinstall files world readable diff --git a/pungi/util.py b/pungi/util.py index 01cab53c..f98893d4 100644 --- a/pungi/util.py +++ b/pungi/util.py @@ -26,6 +26,7 @@ import urlparse import contextlib import traceback import tempfile +import time from kobo.shortcuts import run, force_list from productmd.common import get_major_version @@ -592,3 +593,22 @@ def temp_dir(log=None, *args, **kwargs): # Okay, we failed to delete temporary dir. if log: log.warning('Error removing %s: %s', dir, exc.strerror) + + +def run_unmount_cmd(cmd, max_retries=10): + """Attempt to run the command to unmount an image. + + If the command fails and stderr complains about device being busy, try + again. We will do up to ``max_retries`` attemps with increasing pauses. + """ + for i in xrange(max_retries): + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + out, err = proc.communicate() + if proc.returncode == 0: + # We were successful + return + if 'Device or resource busy' not in err: + raise RuntimeError('Unhandled error when running %r: %r' % (cmd, err)) + time.sleep(i) + # Still busy, there's something wrong. + raise RuntimeError('Failed to run %r: Device or resource busy.' % cmd) diff --git a/tests/test_util.py b/tests/test_util.py index 3052aab5..108b57cc 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -10,6 +10,7 @@ except ImportError: import unittest import tempfile import shutil +import subprocess sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) @@ -428,5 +429,60 @@ class TestTempFiles(unittest.TestCase): self.assertFalse(os.path.exists(tmp)) +class TestUnmountCmd(unittest.TestCase): + + def _fakeProc(self, ret, err): + proc = mock.Mock(returncode=ret) + proc.communicate.return_value = ('', err) + return proc + + @mock.patch('subprocess.Popen') + def test_unmount_cmd_success(self, mockPopen): + cmd = 'unmount' + mockPopen.side_effect = [self._fakeProc(0, '')] + util.run_unmount_cmd(cmd) + self.assertEqual(mockPopen.call_args_list, + [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)]) + + @mock.patch('subprocess.Popen') + def test_unmount_cmd_fail_other_reason(self, mockPopen): + cmd = 'unmount' + mockPopen.side_effect = [self._fakeProc(1, 'It is broken')] + with self.assertRaises(RuntimeError) as ctx: + util.run_unmount_cmd(cmd) + self.assertEqual(str(ctx.exception), + "Unhandled error when running 'unmount': 'It is broken'") + self.assertEqual(mockPopen.call_args_list, + [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)]) + + @mock.patch('time.sleep') + @mock.patch('subprocess.Popen') + def test_unmount_cmd_fail_then_retry(self, mockPopen, mock_sleep): + cmd = 'unmount' + mockPopen.side_effect = [self._fakeProc(1, 'Device or resource busy'), + self._fakeProc(1, 'Device or resource busy'), + self._fakeProc(0, '')] + util.run_unmount_cmd(cmd) + self.assertEqual(mockPopen.call_args_list, + [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)] * 3) + self.assertEqual(mock_sleep.call_args_list, + [mock.call(0), mock.call(1)]) + + @mock.patch('time.sleep') + @mock.patch('subprocess.Popen') + def test_unmount_cmd_fail_then_retry_and_fail(self, mockPopen, mock_sleep): + cmd = 'unmount' + mockPopen.side_effect = [self._fakeProc(1, 'Device or resource busy'), + self._fakeProc(1, 'Device or resource busy'), + self._fakeProc(1, 'Device or resource busy')] + with self.assertRaises(RuntimeError) as ctx: + util.run_unmount_cmd(cmd, max_retries=3) + self.assertEqual(mockPopen.call_args_list, + [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE)] * 3) + self.assertEqual(mock_sleep.call_args_list, + [mock.call(0), mock.call(1), mock.call(2)]) + self.assertEqual(str(ctx.exception), "Failed to run 'unmount': Device or resource busy.") + + if __name__ == "__main__": unittest.main()