218 lines
10 KiB
Diff
218 lines
10 KiB
Diff
|
From a07674254acb562332579a42a25c44c1a0e0bac6 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Ondrej Nosek <onosek@redhat.com>
|
||
|
|
Date: Thu, 21 Dec 2017 03:18:00 +0100
|
||
|
|
Subject: [PATCH 1/2] Fall back to mount if guestmount is not available
|
||
|
|
|
||
|
|
Relates: https://pagure.io/pungi/issue/803
|
||
|
|
|
||
|
|
Signed-off-by: Ondrej Nosek <onosek@redhat.com>
|
||
|
|
---
|
||
|
|
pungi.spec | 1 -
|
||
|
|
pungi/phases/product_img.py | 9 +++++++--
|
||
|
|
pungi/util.py | 6 +-----
|
||
|
|
pungi/wrappers/iso.py | 19 ++++++++++++++-----
|
||
|
|
tests/test_iso_wrapper.py | 37 +++++++++++++++++++++++++++++++------
|
||
|
|
tests/test_util.py | 2 +-
|
||
|
|
6 files changed, 54 insertions(+), 20 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/pungi.spec b/pungi.spec
|
||
|
|
index 545e409..82d305f 100644
|
||
|
|
--- a/pungi.spec
|
||
|
|
+++ b/pungi.spec
|
||
|
|
@@ -42,7 +42,6 @@ Requires: gettext
|
||
|
|
Requires: syslinux
|
||
|
|
Requires: git
|
||
|
|
Requires: python-jsonschema
|
||
|
|
-Requires: libguestfs-tools-c
|
||
|
|
Requires: python-enum34
|
||
|
|
Requires: python2-dnf
|
||
|
|
Requires: python2-multilib
|
||
|
|
diff --git a/pungi/phases/product_img.py b/pungi/phases/product_img.py
|
||
|
|
index 9a21eed..c956674 100644
|
||
|
|
--- a/pungi/phases/product_img.py
|
||
|
|
+++ b/pungi/phases/product_img.py
|
||
|
|
@@ -131,6 +131,9 @@ def create_product_img(compose, arch, variant):
|
||
|
|
|
||
|
|
shutil.rmtree(po_tmp)
|
||
|
|
|
||
|
|
+ ret, __ = run(["which", "guestmount"], can_fail=True)
|
||
|
|
+ guestmount_available = not bool(ret) # return code 0 means that guestmount is available
|
||
|
|
+
|
||
|
|
mount_tmp = compose.mkdtemp(prefix="product_img_mount_")
|
||
|
|
cmds = [
|
||
|
|
# allocate image
|
||
|
|
@@ -139,7 +142,8 @@ def create_product_img(compose, arch, variant):
|
||
|
|
"mke2fs -F %s" % shlex_quote(image),
|
||
|
|
# use guestmount to mount the image, which doesn't require root privileges
|
||
|
|
# LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt
|
||
|
|
- "LIBGUESTFS_BACKEND=direct guestmount -a %s -m /dev/sda %s" % (shlex_quote(image), shlex_quote(mount_tmp)),
|
||
|
|
+ "LIBGUESTFS_BACKEND=direct guestmount -a %s -m /dev/sda %s" % (shlex_quote(image), shlex_quote(mount_tmp)) if guestmount_available
|
||
|
|
+ else "mount -o loop %s %s" % (shlex_quote(image), shlex_quote(mount_tmp)),
|
||
|
|
"mkdir -p %s/run/install/product" % shlex_quote(mount_tmp),
|
||
|
|
"cp -rp %s/* %s/run/install/product/" % (shlex_quote(product_tmp), shlex_quote(mount_tmp)),
|
||
|
|
"mkdir -p %s/run/install/product/pyanaconda" % shlex_quote(mount_tmp),
|
||
|
|
@@ -149,7 +153,8 @@ def create_product_img(compose, arch, variant):
|
||
|
|
"ln -s run/install/product/locale %s" % shlex_quote(mount_tmp),
|
||
|
|
# compat symlink: run/install/product/pyanaconda/installclasses -> ../installclasses
|
||
|
|
"ln -s ../installclasses %s/run/install/product/pyanaconda/installclasses" % shlex_quote(mount_tmp),
|
||
|
|
- "fusermount -u %s" % shlex_quote(mount_tmp),
|
||
|
|
+ "fusermount -u %s" % shlex_quote(mount_tmp) if guestmount_available
|
||
|
|
+ else "umount %s" % shlex_quote(mount_tmp),
|
||
|
|
# tweak last mount path written in the image
|
||
|
|
"tune2fs -M /run/install/product %s" % shlex_quote(image),
|
||
|
|
]
|
||
|
|
diff --git a/pungi/util.py b/pungi/util.py
|
||
|
|
index 2d2df82..b74f280 100644
|
||
|
|
--- a/pungi/util.py
|
||
|
|
+++ b/pungi/util.py
|
||
|
|
@@ -36,6 +36,7 @@ from productmd.common import get_major_version
|
||
|
|
# Patterns that match all names of debuginfo packages
|
||
|
|
DEBUG_PATTERNS = ["*-debuginfo", "*-debuginfo-*", "*-debugsource"]
|
||
|
|
|
||
|
|
+
|
||
|
|
def _doRunCommand(command, logger, rundir='/tmp', output=subprocess.PIPE, error=subprocess.PIPE, env=None):
|
||
|
|
"""Run a command and log the output. Error out if we get something on stderr"""
|
||
|
|
|
||
|
|
@@ -609,11 +610,6 @@ def temp_dir(log=None, *args, **kwargs):
|
||
|
|
log.warning('Error removing %s: %s', dir, exc.strerror)
|
||
|
|
|
||
|
|
|
||
|
|
-def fusermount(path, **kwargs):
|
||
|
|
- """Run fusermount -u on a given path."""
|
||
|
|
- run_unmount_cmd(['fusermount', '-u', path], path=path, **kwargs)
|
||
|
|
-
|
||
|
|
-
|
||
|
|
def run_unmount_cmd(cmd, max_retries=10, path=None, logger=None):
|
||
|
|
"""Attempt to run the command to unmount an image.
|
||
|
|
|
||
|
|
diff --git a/pungi/wrappers/iso.py b/pungi/wrappers/iso.py
|
||
|
|
index 7e2c710..9ed3f24 100644
|
||
|
|
--- a/pungi/wrappers/iso.py
|
||
|
|
+++ b/pungi/wrappers/iso.py
|
||
|
|
@@ -385,11 +385,17 @@ def mount(image, logger=None):
|
||
|
|
The yielded path will only be valid in the with block and is removed once
|
||
|
|
the image is unmounted.
|
||
|
|
"""
|
||
|
|
- # use guestmount to mount the image, which doesn't require root privileges
|
||
|
|
- # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt
|
||
|
|
with util.temp_dir(prefix='iso-mount-') as mount_dir:
|
||
|
|
- env = {'LIBGUESTFS_BACKEND': 'direct', 'LIBGUESTFS_DEBUG': '1', 'LIBGUESTFS_TRACE': '1'}
|
||
|
|
- cmd = ["guestmount", "-a", image, "-m", "/dev/sda", mount_dir]
|
||
|
|
+ ret, __ = run(["which", "guestmount"], can_fail=True)
|
||
|
|
+ guestmount_available = not bool(ret) # return code 0 means that guestmount is available
|
||
|
|
+ if guestmount_available:
|
||
|
|
+ # use guestmount to mount the image, which doesn't require root privileges
|
||
|
|
+ # LIBGUESTFS_BACKEND=direct: running qemu directly without libvirt
|
||
|
|
+ env = {'LIBGUESTFS_BACKEND': 'direct', 'LIBGUESTFS_DEBUG': '1', 'LIBGUESTFS_TRACE': '1'}
|
||
|
|
+ cmd = ["guestmount", "-a", image, "-m", "/dev/sda", mount_dir]
|
||
|
|
+ else:
|
||
|
|
+ env = {}
|
||
|
|
+ cmd = ["mount", "-o", "loop", image, mount_dir]
|
||
|
|
ret, out = run(cmd, env=env, can_fail=True, universal_newlines=True)
|
||
|
|
if ret != 0:
|
||
|
|
# The mount command failed, something is wrong. Log the output and raise an exception.
|
||
|
|
@@ -400,4 +406,7 @@ def mount(image, logger=None):
|
||
|
|
try:
|
||
|
|
yield mount_dir
|
||
|
|
finally:
|
||
|
|
- util.fusermount(mount_dir, logger=logger)
|
||
|
|
+ if guestmount_available:
|
||
|
|
+ util.run_unmount_cmd(['fusermount', '-u', mount_dir], path=mount_dir)
|
||
|
|
+ else:
|
||
|
|
+ util.run_unmount_cmd(['umount', mount_dir], path=mount_dir)
|
||
|
|
diff --git a/tests/test_iso_wrapper.py b/tests/test_iso_wrapper.py
|
||
|
|
index a4a00a1..7ebc2c3 100644
|
||
|
|
--- a/tests/test_iso_wrapper.py
|
||
|
|
+++ b/tests/test_iso_wrapper.py
|
||
|
|
@@ -20,6 +20,7 @@ INCORRECT_OUTPUT = '''This should never happen: File not found'''
|
||
|
|
|
||
|
|
|
||
|
|
class TestIsoUtils(unittest.TestCase):
|
||
|
|
+
|
||
|
|
@mock.patch('pungi.wrappers.iso.run')
|
||
|
|
def test_get_implanted_md5_correct(self, mock_run):
|
||
|
|
mock_run.return_value = (0, CORRECT_OUTPUT)
|
||
|
|
@@ -44,24 +45,48 @@ class TestIsoUtils(unittest.TestCase):
|
||
|
|
@mock.patch('pungi.util.run_unmount_cmd')
|
||
|
|
@mock.patch('pungi.wrappers.iso.run')
|
||
|
|
def test_mount_iso(self, mock_run, mock_unmount):
|
||
|
|
- mock_run.return_value = (0, '')
|
||
|
|
+ # first tuple is return value for command 'which guestmount'
|
||
|
|
+ # value determines type of the mount/unmount command ('1' - guestmount is not available)
|
||
|
|
+ # for approach as a root, pair commands mount-umount are used
|
||
|
|
+ mock_run.side_effect = [(1, ''), (0, '')]
|
||
|
|
+ with iso.mount('dummy') as temp_dir:
|
||
|
|
+ self.assertTrue(os.path.isdir(temp_dir))
|
||
|
|
+ self.assertEqual(len(mock_run.call_args_list), 2)
|
||
|
|
+ mount_call_str = str(mock_run.call_args_list[1])
|
||
|
|
+ self.assertTrue(mount_call_str.startswith("call(['mount'"))
|
||
|
|
+ self.assertEqual(len(mock_unmount.call_args_list), 1)
|
||
|
|
+ unmount_call_str = str(mock_unmount.call_args_list[0])
|
||
|
|
+ self.assertTrue(unmount_call_str.startswith("call(['umount'"))
|
||
|
|
+ self.assertFalse(os.path.isdir(temp_dir))
|
||
|
|
+
|
||
|
|
+ @mock.patch('pungi.util.run_unmount_cmd')
|
||
|
|
+ @mock.patch('pungi.wrappers.iso.run')
|
||
|
|
+ def test_guestmount(self, mock_run, mock_unmount):
|
||
|
|
+ # first tuple is return value for command 'which guestmount'
|
||
|
|
+ # value determines type of the mount/unmount command ('0' - guestmount is available)
|
||
|
|
+ # for approach as a non-root, pair commands guestmount-fusermount are used
|
||
|
|
+ mock_run.side_effect = [(0, ''), (0, '')]
|
||
|
|
with iso.mount('dummy') as temp_dir:
|
||
|
|
self.assertTrue(os.path.isdir(temp_dir))
|
||
|
|
- self.assertEqual(len(mock_run.call_args_list), 1)
|
||
|
|
+ self.assertEqual(len(mock_run.call_args_list), 2)
|
||
|
|
+ mount_call_str = str(mock_run.call_args_list[1])
|
||
|
|
+ self.assertTrue(mount_call_str.startswith("call(['guestmount'"))
|
||
|
|
self.assertEqual(len(mock_unmount.call_args_list), 1)
|
||
|
|
+ unmount_call_str = str(mock_unmount.call_args_list[0])
|
||
|
|
+ self.assertTrue(unmount_call_str.startswith("call(['fusermount'"))
|
||
|
|
self.assertFalse(os.path.isdir(temp_dir))
|
||
|
|
|
||
|
|
@mock.patch('pungi.util.run_unmount_cmd')
|
||
|
|
@mock.patch('pungi.wrappers.iso.run')
|
||
|
|
def test_mount_iso_always_unmounts(self, mock_run, mock_unmount):
|
||
|
|
- mock_run.return_value = (0, '')
|
||
|
|
+ mock_run.side_effect = [(1, ''), (0, '')]
|
||
|
|
try:
|
||
|
|
with iso.mount('dummy') as temp_dir:
|
||
|
|
self.assertTrue(os.path.isdir(temp_dir))
|
||
|
|
raise RuntimeError()
|
||
|
|
except RuntimeError:
|
||
|
|
pass
|
||
|
|
- self.assertEqual(len(mock_run.call_args_list), 1)
|
||
|
|
+ self.assertEqual(len(mock_run.call_args_list), 2)
|
||
|
|
self.assertEqual(len(mock_unmount.call_args_list), 1)
|
||
|
|
self.assertFalse(os.path.isdir(temp_dir))
|
||
|
|
|
||
|
|
@@ -69,11 +94,11 @@ class TestIsoUtils(unittest.TestCase):
|
||
|
|
@mock.patch('pungi.wrappers.iso.run')
|
||
|
|
def test_mount_iso_raises_on_error(self, mock_run, mock_unmount):
|
||
|
|
log = mock.Mock()
|
||
|
|
- mock_run.return_value = (1, 'Boom')
|
||
|
|
+ mock_run.side_effect = [(1, ''), (1, 'Boom')]
|
||
|
|
with self.assertRaises(RuntimeError):
|
||
|
|
with iso.mount('dummy', logger=log) as temp_dir:
|
||
|
|
self.assertTrue(os.path.isdir(temp_dir))
|
||
|
|
- self.assertEqual(len(mock_run.call_args_list), 1)
|
||
|
|
+ self.assertEqual(len(mock_run.call_args_list), 2)
|
||
|
|
self.assertEqual(len(mock_unmount.call_args_list), 0)
|
||
|
|
self.assertEqual(len(log.mock_calls), 1)
|
||
|
|
|
||
|
|
diff --git a/tests/test_util.py b/tests/test_util.py
|
||
|
|
index e0dd2b1..7529a25 100644
|
||
|
|
--- a/tests/test_util.py
|
||
|
|
+++ b/tests/test_util.py
|
||
|
|
@@ -446,7 +446,7 @@ class TestUnmountCmd(unittest.TestCase):
|
||
|
|
self._fakeProc(0, out='It is very busy'),
|
||
|
|
self._fakeProc(1, out='lsof output')]
|
||
|
|
with self.assertRaises(RuntimeError) as ctx:
|
||
|
|
- util.fusermount('/path', max_retries=3, logger=logger)
|
||
|
|
+ util.run_unmount_cmd(['fusermount', '-u', '/path'], path='/path', max_retries=3, logger=logger)
|
||
|
|
cmd = ['fusermount', '-u', '/path']
|
||
|
|
expected = [mock.call(cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE,
|
||
|
|
universal_newlines=True),
|
||
|
|
--
|
||
|
|
2.13.6
|
||
|
|
|