Repository: osbuild/osbuild
Pull request #1579 (2024-02-08 08:54:31)

stages,testutil: add test/tweak for podman mount errror handling
I promised to write a small test for the podman mount error handling in https://github.com/osbuild/osbuild/pull/1571. This PR contains a new helper `testutil.mock_command` to create fake commands for things in PATH and a test that ensures that the error messages are handled correct from podman. This lead to a small tweak in deploy-container so that the code only `podman umounts` if the `podman mount` was successful.

supakeen: I'll approve but I think @bcl accidentally added in a merge commit here.
achilleas-k: Thanks for cleaning up @bcl.  I wish we could disable update with merge -_-

Patch:
diff --git a/stages/org.osbuild.container-deploy b/stages/org.osbuild.container-deploy
index 7f419da4..df914a3b 100755
--- a/stages/org.osbuild.container-deploy
+++ b/stages/org.osbuild.container-deploy
@@ -44,18 +44,18 @@ SCHEMA_2 = r"""
 
 @contextlib.contextmanager
 def mount_container(image_tag):
+    result = subprocess.run(
+        ["podman", "image", "mount", image_tag],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        encoding="utf-8",
+        check=False,
+    )
+    if result.returncode != 0:
+        code = result.returncode
+        msg = result.stderr.strip()
+        raise RuntimeError(f"Failed to mount image ({code}): {msg}")
     try:
-        result = subprocess.run(
-            ["podman", "image", "mount", image_tag],
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            encoding="utf-8",
-            check=False,
-        )
-        if result.returncode != 0:
-            code = result.returncode
-            msg = result.stderr.strip()
-            raise RuntimeError(f"Failed to mount image ({code}): {msg}")
         yield result.stdout.strip()
     finally:
         subprocess.run(
diff --git a/tools/osbuild-depsolve-dnf b/tools/osbuild-depsolve-dnf
index fdc34624..1938fe1e 100755
--- a/tools/osbuild-depsolve-dnf
+++ b/tools/osbuild-depsolve-dnf
@@ -206,13 +206,11 @@ class Solver():
     def depsolve(self, transactions):
         last_transaction = []
 
-        for idx, transaction in enumerate(transactions):
+        for transaction in transactions:
             self.base.reset(goal=True)
             self.base.sack.reset_excludes()
 
-            # don't install weak-deps for transactions after the 1st transaction
-            if idx > 0:
-                self.base.conf.install_weak_deps = False
+            self.base.conf.install_weak_deps = transaction.get("install_weak_deps", False)
 
             # set the packages from the last transaction as installed
             for installed_pkg in last_transaction:


Test patch:
diff --git a/osbuild/testutil/__init__.py b/osbuild/testutil/__init__.py
index 3d9e2bc0..00531972 100644
--- a/osbuild/testutil/__init__.py
+++ b/osbuild/testutil/__init__.py
@@ -1,10 +1,12 @@
 """
 Test related utilities
 """
+import contextlib
 import os
 import pathlib
 import re
 import shutil
+import tempfile
 
 
 def has_executable(executable: str) -> bool:
@@ -57,3 +59,21 @@ def assert_jsonschema_error_contains(res, expected_err, expected_num_errs=None):
         def finder(s): return expected_err in s  # pylint: disable=C0321
     assert any(finder(err_msg)
                for err_msg in err_msgs), f"{expected_err} not found in {err_msgs}"
+
+
+@contextlib.contextmanager
+def mock_command(cmd_name: str, script: str):
+    """
+    mock_command creates a mocked binary with the given :cmd_name: and :script:
+    content. This is useful to e.g. mock errors from binaries.
+    """
+    original_path = os.environ["PATH"]
+    with tempfile.TemporaryDirectory() as tmpdir:
+        cmd_path = pathlib.Path(tmpdir) / cmd_name
+        cmd_path.write_text(script, encoding="utf8")
+        cmd_path.chmod(0o755)
+        os.environ["PATH"] = f"{tmpdir}:{original_path}"
+        try:
+            yield
+        finally:
+            os.environ["PATH"] = original_path
diff --git a/stages/test/test_container_deploy.py b/stages/test/test_container_deploy.py
index 285e82a7..b280b002 100644
--- a/stages/test/test_container_deploy.py
+++ b/stages/test/test_container_deploy.py
@@ -5,10 +5,12 @@ import os.path
 import random
 import string
 import subprocess
+import textwrap
 from unittest.mock import call, patch
 
 import pytest
 
+import osbuild.testutil
 from osbuild.testutil import has_executable, make_fake_tree
 
 STAGE_NAME = "org.osbuild.container-deploy"
@@ -127,3 +129,18 @@ def test_container_deploy_exclude(tmp_path, stage_module):
     assert (output_dir / "dir1/file3").read_bytes() == b"dir1/file3 content"
     assert not (output_dir / "dir2/file4").exists()
     assert not (output_dir / "dir2").exists()
+
+
+def test_container_deploy_error(stage_module):
+    fake_podman = textwrap.dedent("""\
+    #!/bin/sh
+    echo "some msg on stdout"
+    echo "other error on stderr" >&2
+    exit 1
+    """)
+    with osbuild.testutil.mock_command("podman", fake_podman):
+        with pytest.raises(RuntimeError) as exp:
+            with stage_module.mount_container("some-image-tag"):
+                pass
+    assert "some msg on stdout" not in str(exp.value)
+    assert "other error on stderr" in str(exp.value)
diff --git a/test/mod/test_testutil_mock_command.py b/test/mod/test_testutil_mock_command.py
new file mode 100644
index 00000000..5e332512
--- /dev/null
+++ b/test/mod/test_testutil_mock_command.py
@@ -0,0 +1,29 @@
+#
+# Tests for the 'osbuild.util.testutil.mock_command' module.
+#
+import os
+import subprocess
+import textwrap
+
+from osbuild.testutil import mock_command
+
+
+def test_mock_command_integration():
+    output = subprocess.check_output(["echo", "hello"])
+    assert output == b"hello\n"
+    fake_echo = textwrap.dedent("""\
+    #!/bin/sh
+    echo i-am-not-echo
+    """)
+    with mock_command("echo", fake_echo):
+        output = subprocess.check_output(["echo", "hello"])
+        assert output == b"i-am-not-echo\n"
+    output = subprocess.check_output(["echo", "hello"])
+    assert output == b"hello\n"
+
+
+def test_mock_command_environ_is_modified_and_restored():
+    orig_path = os.environ["PATH"]
+    with mock_command("something", "#!/bin/sh\ntrue\n"):
+        assert os.environ["PATH"] != orig_path
+    assert os.environ["PATH"] == orig_path