buildinstall: Retry unmounting image

If the image can not be unmounted because the device is busy, we should
retry. There will be increasing pauses between the attempts. At most 10
attempts will be done before giving up.

Fixes: #559
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

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

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)

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()