]> git.ipfire.org Git - thirdparty/mkosi.git/commitdiff
Run image builds in a fork again
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 7 Dec 2023 10:21:30 +0000 (11:21 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Thu, 7 Dec 2023 17:55:58 +0000 (18:55 +0100)
This solves two problems:
- When not using a tools tree, we can run qemu outside of the user
namespace which means that we don't need to pass fds to /dev/kvm and
/dev/vhost-vsock to keep things working unprivileged
- The vmspawn verb we're about to introduce will not be able to run
properly inside a user namespace, so we need to make sure we're not
inside a user namespace after the image build.

Compared to our original implementation of this way back with exception
propagation, this time we opt to do things differently by doing all
exception handling and logging inside the fork to avoid having to
propagate exceptions. This makes the overall implementation a lot
simpler.

We can also run the other verbs outside of the user namespace as long
as we're not using a tools tree. Because we want to keep support for
using a tools tree with all verbs, we keep support for running them
inside a user namespace as well. Because we already use INVOKING_USER
everywhere, this actually turns out to require very little changes. We
only need to make sure when starting virtiofsd that we unshare the user
namespace ourselves if we're not uid mapping.

mkosi/__init__.py
mkosi/__main__.py
mkosi/qemu.py
mkosi/run.py

index 1ae70f872daa5488e39e44b292730fdb3b08ef59..a0deedcfa4b3d94a077145aed482f9ea7b387848 100644 (file)
@@ -56,6 +56,7 @@ from mkosi.run import (
     bwrap,
     chroot_cmd,
     find_binary,
+    fork_and_wait,
     init_mount_namespace,
     run,
 )
@@ -2832,15 +2833,6 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None:
         if args.verb == Verb.qemu and d.feature(last) != ConfigFeature.disabled and d.available(log=True)
     }
 
-    # Get the user UID/GID either on the host or in the user namespace running the build
-    become_root()
-    init_mount_namespace()
-
-    # For extra safety when running as root, remount a bunch of stuff read-only.
-    for d in ("/usr", "/etc", "/opt", "/srv", "/boot", "/efi", "/media", "/mnt"):
-        if Path(d).exists():
-            run(["mount", "--rbind", d, d, "--options", "ro"])
-
     # First, process all directory removals because otherwise if different images share directories a later
     # image build could end up deleting the output generated by an earlier image build.
 
@@ -2848,7 +2840,11 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None:
         if not needs_build(args, config) and args.verb != Verb.clean:
             continue
 
-        unlink_output(args, config)
+        def target() -> None:
+            become_root()
+            unlink_output(args, config)
+
+        fork_and_wait(target)
 
     if args.verb == Verb.clean:
         return
@@ -2861,29 +2857,40 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None:
         if not needs_build(args, config):
             continue
 
-        with (
-            complete_step(f"Building {config.name()} image"),
-            mount_tools(config.tools_tree),
-            hide_host_directories(),
-            prepend_to_environ_path(config),
-        ):
-            # After tools have been mounted, check if we have what we need
-            check_tools(args, config)
-
-            # Create these as the invoking user to make sure they're owned by the user running mkosi.
-            for p in (
-                config.output_dir,
-                config.cache_dir,
-                config.build_dir,
-                config.workspace_dir,
+        def target() -> None:
+            become_root()
+            init_mount_namespace()
+
+            # For extra safety when running as root, remount a bunch of stuff read-only.
+            for d in ("/usr", "/etc", "/opt", "/srv", "/boot", "/efi", "/media", "/mnt"):
+                if Path(d).exists():
+                    run(["mount", "--rbind", d, d, "--options", "ro"])
+
+            with (
+                complete_step(f"Building {config.name()} image"),
+                mount_tools(config.tools_tree),
+                hide_host_directories(),
+                prepend_to_environ_path(config),
             ):
-                if p:
-                    run(["mkdir", "--parents", p], user=INVOKING_USER.uid, group=INVOKING_USER.gid)
+                # After tools have been mounted, check if we have what we need
+                check_tools(args, config)
+
+                # Create these as the invoking user to make sure they're owned by the user running mkosi.
+                for p in (
+                    config.output_dir,
+                    config.cache_dir,
+                    config.build_dir,
+                    config.workspace_dir,
+                ):
+                    if p:
+                        run(["mkdir", "--parents", p], user=INVOKING_USER.uid, group=INVOKING_USER.gid)
 
-            with acl_toggle_build(config, INVOKING_USER.uid):
-                build_image(args, config)
+                with acl_toggle_build(config, INVOKING_USER.uid):
+                    build_image(args, config)
 
-            build = True
+        fork_and_wait(target)
+
+        build = True
 
     if build and args.auto_bump:
         bump_image_version()
@@ -2891,11 +2898,17 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None:
     if args.verb == Verb.build:
         return
 
-    with (
-        mount_usr(last.tools_tree),
-        mount_passwd(),
-        prepend_to_environ_path(last),
-    ):
+    if last.tools_tree:
+        become_root()
+
+    with contextlib.ExitStack() as stack:
+        if os.getuid() == 0:
+            init_mount_namespace()
+            stack.enter_context(mount_usr(last.tools_tree))
+            stack.enter_context(mount_passwd())
+
+        stack.enter_context(prepend_to_environ_path(last))
+
         check_tools(args, last)
 
         with prepend_to_environ_path(last):
index 8239bd7855728ab02ddde504e40560f212680557..df25835437c01a32988a7966950d607cd7d0ea82 100644 (file)
@@ -1,47 +1,18 @@
 # SPDX-License-Identifier: LGPL-2.1+
 # PYTHON_ARGCOMPLETE_OK
 
-import contextlib
 import faulthandler
-import logging
 import shutil
-import subprocess
 import sys
-from collections.abc import Iterator
 
 from mkosi import run_verb
 from mkosi.config import parse_config
-from mkosi.log import ARG_DEBUG, log_setup
-from mkosi.run import ensure_exc_info, run
+from mkosi.log import log_setup
+from mkosi.run import run, uncaught_exception_handler
 from mkosi.util import INVOKING_USER
 
 
-@contextlib.contextmanager
-def propagate_failed_return() -> Iterator[None]:
-    try:
-        yield
-    except SystemExit as e:
-        if ARG_DEBUG.get():
-            sys.excepthook(*ensure_exc_info())
-
-        sys.exit(e.code)
-    except KeyboardInterrupt:
-        if ARG_DEBUG.get():
-            sys.excepthook(*ensure_exc_info())
-        else:
-            logging.error("Interrupted")
-
-        sys.exit(1)
-    except subprocess.CalledProcessError as e:
-        # Failures from qemu, ssh and systemd-nspawn are expected and we won't log stacktraces for those.
-        if ARG_DEBUG.get() and e.cmd and e.cmd[0] not in ("qemu", "ssh", "systemd-nspawn"):
-            sys.excepthook(*ensure_exc_info())
-
-        # We always log when subprocess.CalledProcessError is raised, so we don't log again here.
-        sys.exit(e.returncode)
-
-
-@propagate_failed_return()
+@uncaught_exception_handler(exit=sys.exit)
 def main() -> None:
     log_setup()
     # Ensure that the name and home of the user we are running as are resolved as early as possible.
index b7b409b9237e4d649504e3a3c1882d3d943312ba..07d20a33f38bad2e78ffd31a9a203075a7d27b93 100644 (file)
@@ -33,7 +33,7 @@ from mkosi.config import (
 )
 from mkosi.log import die
 from mkosi.partition import finalize_root, find_partitions
-from mkosi.run import MkosiAsyncioThread, find_binary, run, spawn
+from mkosi.run import MkosiAsyncioThread, become_root, find_binary, run, spawn
 from mkosi.tree import copy_tree, rmtree
 from mkosi.types import PathString
 from mkosi.util import INVOKING_USER, StrEnum
@@ -343,7 +343,8 @@ def start_virtiofsd(directory: Path, *, uidmap: bool) -> Iterator[Path]:
             cmdline,
             user=INVOKING_USER.uid if uidmap else None,
             group=INVOKING_USER.gid if uidmap else None,
-            pass_fds=(sock.fileno(),)
+            pass_fds=(sock.fileno(),),
+            preexec_fn=become_root if not uidmap and os.getuid() != 0 else None,
         ) as proc:
             try:
                 yield path
index a862c9e91be5ee353ef145bbaf8bbca10b3783bd..f8501acf1def190c4a904055fcb32c680252e6a2 100644 (file)
@@ -21,7 +21,7 @@ import threading
 from collections.abc import Awaitable, Collection, Iterator, Mapping, Sequence
 from pathlib import Path
 from types import TracebackType
-from typing import Any, Optional
+from typing import Any, Callable, NoReturn, Optional
 
 from mkosi.log import ARG_DEBUG, ARG_DEBUG_SHELL, die
 from mkosi.types import _FILE, CompletedProcess, PathString, Popen
@@ -161,6 +161,61 @@ def ensure_exc_info() -> tuple[type[BaseException], BaseException, TracebackType
     return (exctype, exc, tb)
 
 
+@contextlib.contextmanager
+def uncaught_exception_handler(exit: Callable[[int], NoReturn]) -> Iterator[None]:
+    rc = 0
+    try:
+        yield
+    except SystemExit as e:
+        if ARG_DEBUG.get():
+            sys.excepthook(*ensure_exc_info())
+
+        rc = e.code if isinstance(e.code, int) else 1
+    except KeyboardInterrupt:
+        if ARG_DEBUG.get():
+            sys.excepthook(*ensure_exc_info())
+        else:
+            logging.error("Interrupted")
+
+        rc = 1
+    except subprocess.CalledProcessError as e:
+        # Failures from qemu, ssh and systemd-nspawn are expected and we won't log stacktraces for those.
+        # Failures from self come from the forks we spawn to build images in a user namespace. We've already done all
+        # the logging for those failures so we don't log stacktraces for those either.
+        if ARG_DEBUG.get() and e.cmd and e.cmd[0] not in ("self", "qemu", "ssh", "systemd-nspawn"):
+            sys.excepthook(*ensure_exc_info())
+
+        # We always log when subprocess.CalledProcessError is raised, so we don't log again here.
+        rc = e.returncode
+    except BaseException:
+        sys.excepthook(*ensure_exc_info())
+        rc = 1
+    finally:
+        sys.stdout.flush()
+        sys.stderr.flush()
+        exit(rc)
+
+
+def fork_and_wait(target: Callable[[], None]) -> None:
+    pid = os.fork()
+    if pid == 0:
+        with uncaught_exception_handler(exit=os._exit):
+            make_foreground_process()
+            target()
+
+    try:
+        _, status = os.waitpid(pid, 0)
+    except BaseException:
+        os.kill(pid, signal.SIGTERM)
+        _, status = os.waitpid(pid, 0)
+    finally:
+        make_foreground_process(new_process_group=False)
+
+    rc = os.waitstatus_to_exitcode(status)
+
+    if rc != 0:
+        raise subprocess.CalledProcessError(rc, ["self"])
+
 def run(
     cmdline: Sequence[PathString],
     check: bool = True,
@@ -240,6 +295,7 @@ def spawn(
     env: Mapping[str, str] = {},
     log: bool = True,
     foreground: bool = False,
+    preexec_fn: Optional[Callable[[], None]] = None,
 ) -> Iterator[Popen]:
     cmdline = [os.fspath(x) for x in cmdline]
 
@@ -259,6 +315,12 @@ def spawn(
         **env,
     }
 
+    def preexec() -> None:
+        if foreground:
+            make_foreground_process()
+        if preexec_fn:
+            preexec_fn()
+
     try:
         with subprocess.Popen(
             cmdline,
@@ -270,7 +332,7 @@ def spawn(
             group=group,
             pass_fds=pass_fds,
             env=env,
-            preexec_fn=make_foreground_process if foreground else None,
+            preexec_fn=preexec,
         ) as proc:
             yield proc
     except FileNotFoundError as e: