build: use exec_run with dmux

Since upstream implemented `dmux` for exec_run it should be used instead of creating containers multiple times. Previously it wasn't possible to tell stderr apart from stdout, making it harder to parse things. Now with proper demuxing, recycle the container all three times, yey! Signed-off-by: Paul Spooren <[email protected]>
openwrt · Sep 24, 2024 · 64850b1 · 64850b1
1 parent 0f8b087
commit 64850b1
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 143 deletions.
diff --git a/asu/build.py b/asu/build.py
@@ -21,7 +21,7 @@
     get_request_hash,
     parse_manifest,
     report_error,
-    run_container,
+    run_cmd,
 )
 
 log = logging.getLogger("rq.worker")
@@ -63,49 +63,6 @@ def build(build_request: BuildRequest, job=None):
     podman.images.pull(image)
     log.info(f"Pulling {image}... done")
 
-    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
-        podman, image, ["make", "info"]
-    )
-
-    job.save_meta()
-
-    version_code = re.search('Current Revision: "(r.+)"', job.meta["stdout"]).group(1)
-
-    if requested := build_request.version_code:
-        if version_code != requested:
-            report_error(
-                job,
-                f"Received incorrect version {version_code} (requested {requested})",
-            )
-
-    default_packages = set(
-        re.search(r"Default Packages: (.*)\n", job.meta["stdout"]).group(1).split()
-    )
-    log.debug(f"Default packages: {default_packages}")
-
-    profile_packages = set(
-        re.search(
-            r"{}:\n    .+\n    Packages: (.*?)\n".format(build_request.profile),
-            job.meta["stdout"],
-            re.MULTILINE,
-        )
-        .group(1)
-        .split()
-    )
-
-    appy_package_changes(build_request)
-
-    build_cmd_packages = build_request.packages
-
-    if build_request.diff_packages:
-        build_cmd_packages: list[str] = diff_packages(
-            build_request.packages, default_packages | profile_packages
-        )
-        log.debug(f"Diffed packages: {build_cmd_packages}")
-
-    job.meta["imagebuilder_status"] = "calculate_packages_hash"
-    job.save_meta()
-
     mounts = []
 
     (bin_dir / "keys").mkdir(parents=True, exist_ok=True)
@@ -153,17 +110,86 @@ def build(build_request: BuildRequest, job=None):
             },
         )
 
-    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
-        podman,
+    if build_request.defaults:
+        log.debug("Found defaults")
+
+        defaults_file = bin_dir / "files/etc/uci-defaults/99-asu-defaults"
+        defaults_file.parent.mkdir(parents=True)
+        defaults_file.write_text(build_request.defaults)
+        mounts.append(
+            {
+                "type": "bind",
+                "source": str(bin_dir / "files"),
+                "target": str(bin_dir / "files"),
+                "read_only": True,
+            },
+        )
+
+    log.debug("Mounts: %s", mounts)
+
+    container = podman.containers.create(
         image,
+        command=["sleep", "600"],
+        mounts=mounts,
+        cap_drop=["all"],
+        no_new_privileges=True,
+        privileged=False,
+        auto_remove=True,
+    )
+    container.start()
+
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd(
+        container, ["make", "info"]
+    )
+
+    job.save_meta()
+
+    version_code = re.search('Current Revision: "(r.+)"', job.meta["stdout"]).group(1)
+
+    if requested := build_request.version_code:
+        if version_code != requested:
+            report_error(
+                job,
+                f"Received incorrect version {version_code} (requested {requested})",
+            )
+
+    default_packages = set(
+        re.search(r"Default Packages: (.*)\n", job.meta["stdout"]).group(1).split()
+    )
+    log.debug(f"Default packages: {default_packages}")
+
+    profile_packages = set(
+        re.search(
+            r"{}:\n    .+\n    Packages: (.*?)\n".format(build_request.profile),
+            job.meta["stdout"],
+            re.MULTILINE,
+        )
+        .group(1)
+        .split()
+    )
+
+    appy_package_changes(build_request)
+
+    build_cmd_packages = build_request.packages
+
+    if build_request.diff_packages:
+        build_cmd_packages: list[str] = diff_packages(
+            build_request.packages, default_packages | profile_packages
+        )
+        log.debug(f"Diffed packages: {build_cmd_packages}")
+
+    job.meta["imagebuilder_status"] = "calculate_packages_hash"
+    job.save_meta()
+
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd(
+        container,
         [
             "make",
             "manifest",
             f"PROFILE={build_request.profile}",
             f"PACKAGES={' '.join(build_cmd_packages)}",
             "STRIP_ABI=1",
         ],
-        mounts=mounts,
     )
 
     job.save_meta()
@@ -190,6 +216,9 @@ def build(build_request: BuildRequest, job=None):
         f"BIN_DIR=/builder/{request_hash}",
     ]
 
+    if build_request.defaults:
+        job.meta["build_cmd"].append(f"FILES={bin_dir}/files")
+
     # Check if custom rootfs size is requested
     if build_request.rootfs_size_mb:
         log.debug("Found custom rootfs size %d", build_request.rootfs_size_mb)
@@ -200,30 +229,14 @@ def build(build_request: BuildRequest, job=None):
     job.meta["imagebuilder_status"] = "building_image"
     job.save_meta()
 
-    if build_request.defaults:
-        log.debug("Found defaults")
-
-        defaults_file = bin_dir / "files/etc/uci-defaults/99-asu-defaults"
-        defaults_file.parent.mkdir(parents=True)
-        defaults_file.write_text(build_request.defaults)
-        job.meta["build_cmd"].append(f"FILES={bin_dir}/files")
-        mounts.append(
-            {
-                "type": "bind",
-                "source": str(bin_dir / "files"),
-                "target": str(bin_dir / "files"),
-                "read_only": True,
-            },
-        )
-
-    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
-        podman,
-        image,
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd(
+        container,
         job.meta["build_cmd"],
-        mounts=mounts,
         copy=["/builder/" + request_hash, bin_dir.parent],
     )
 
+    container.kill()
+
     job.save_meta()
 
     if any(err in job.meta["stderr"] for err in ["is too big", "out of space?"]):
@@ -264,24 +277,8 @@ def build(build_request: BuildRequest, job=None):
 
     if Path(build_key).is_file():
         log.info(f"Signing images with key {build_key}")
-        returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
-            podman,
+        container = podman.containers.create(
             image,
-            [
-                "bash",
-                "-c",
-                (
-                    "env;"
-                    "for IMAGE in $IMAGES_TO_SIGN; do "
-                    "touch ${IMAGE}.test;"
-                    'fwtool -t -s /dev/null "$IMAGE" && echo "sign entfern";'
-                    'cp "/builder/key-build.ucert" "$IMAGE.ucert" && echo "moved";'
-                    'usign -S -m "$IMAGE" -s "/builder/key-build" -x "$IMAGE.sig"  && echo "usign";'
-                    'ucert -A -c "$IMAGE.ucert" -x "$IMAGE.sig" && echo "ucert";'
-                    'fwtool -S "$IMAGE.ucert" "$IMAGE" && echo "fwtool";'
-                    "done"
-                ),
-            ],
             mounts=[
                 {
                     "type": "bind",
@@ -308,7 +305,27 @@ def build(build_request: BuildRequest, job=None):
                 "IMAGES_TO_SIGN": " ".join(images),
                 "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/builder/staging_dir/host/bin",
             },
+            auto_remove=True,
+        )
+        returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd(
+            container,
+            [
+                "bash",
+                "-c",
+                (
+                    "env;"
+                    "for IMAGE in $IMAGES_TO_SIGN; do "
+                    "touch ${IMAGE}.test;"
+                    'fwtool -t -s /dev/null "$IMAGE" && echo "sign entfern";'
+                    'cp "/builder/key-build.ucert" "$IMAGE.ucert" && echo "moved";'
+                    'usign -S -m "$IMAGE" -s "/builder/key-build" -x "$IMAGE.sig"  && echo "usign";'
+                    'ucert -A -c "$IMAGE.ucert" -x "$IMAGE.sig" && echo "ucert";'
+                    'fwtool -S "$IMAGE.ucert" "$IMAGE" && echo "fwtool";'
+                    "done"
+                ),
+            ],
         )
+        container.stop()
         job.save_meta()
     else:
         log.warning("No build key found, skipping signing")

diff --git a/asu/util.py b/asu/util.py
@@ -238,53 +238,17 @@ def diff_packages(requested_packages: list, default_packages: set) -> list[str]:
     )
 
 
-def run_container(
-    podman: PodmanClient,
-    image,
+def run_cmd(
+    container,
     command,
-    mounts=[],
     copy=[],
-    user=None,
     environment={},
     working_dir=None,
 ):
-    """Run a container and return the returncode, stdout and stderr
+    returncode, output = container.exec_run(command, demux=True, user="buildbot")
 
-    Args:
-        podman (PodmanClient): Podman client
-        image (str): Image to run
-        command (list): Command to run
-        mounts (list, optional): List of mounts. Defaults to [].
-
-    Returns:
-        tuple: (returncode, stdout, stderr)
-    """
-    logging.warning(
-        f"Running {image} {command} {mounts} {copy} {user} {environment} {working_dir}"
-    )
-    container = podman.containers.run(
-        image=image,
-        command=command,
-        detach=True,
-        mounts=mounts,
-        cap_drop=["all"],
-        no_new_privileges=True,
-        privileged=False,
-        user=user,
-        working_dir=working_dir,
-        environment=environment,
-    )
-
-    returncode = container.wait()
-
-    # Podman 4.x changed the way logs are returned
-    if podman.version()["Version"].startswith("3"):
-        delimiter = b"\n"
-    else:
-        delimiter = b""
-
-    stdout = delimiter.join(container.logs(stdout=True, stderr=False)).decode("utf-8")
-    stderr = delimiter.join(container.logs(stdout=False, stderr=True)).decode("utf-8")
+    stdout = output[0].decode("utf-8")
+    stderr = output[1].decode("utf-8")
 
     logging.debug(f"returncode: {returncode}")
     logging.debug(f"stdout: {stdout}")
@@ -308,12 +272,6 @@ def run_container(
         host_tar.close()
         logging.debug(f"Closed {host_tar}")
 
-    try:
-        container.remove(v=True)
-        podman.volumes.prune()  # TODO: remove once v=True works
-    except Exception as e:
-        logging.warning(f"Failed to remove container: {e}")
-
     return returncode, stdout, stderr
 
 

diff --git a/poetry.lock b/poetry.lock
diff --git a/pyproject.toml b/pyproject.toml
@@ -10,7 +10,7 @@ python = "^3.11"
 fastapi = { extras = ["standard"], version = "^0.115.0" }
 pynacl = "^1.5.0"
 requests = "^2.32.3"
-podman = "^5.0.0"
+podman = "^5.1.0"
 redis = "^5.0.8"
 pydantic-settings = "^2.4.0"
 rq = "^1.16.2"

diff --git a/tests/test_util.py b/tests/test_util.py
@@ -16,7 +16,7 @@
     get_request_hash,
     get_str_hash,
     parse_packages_versions,
-    run_container,
+    run_cmd,
     verify_usign,
 )
 
@@ -141,13 +141,19 @@ def test_get_podman():
     assert isinstance(podman, PodmanClient)
 
 
-def test_run_container():
+def test_run_cmd():
     podman = get_podman()
     podman.images.pull("ghcr.io/openwrt/imagebuilder:testtarget-testsubtarget-v1.2.3")
 
-    returncode, stdout, stderr = run_container(
-        podman,
+    container = podman.containers.create(
         "ghcr.io/openwrt/imagebuilder:testtarget-testsubtarget-v1.2.3",
+        command=["sleep", "1000"],
+        detach=True,
+    )
+    container.start()
+
+    returncode, stdout, stderr = run_cmd(
+        container,
         ["make", "info"],
     )
 

diff --git a/...arget/testsubtarget/openwrt-imagebuilder-testtarget-testsubtarget.Linux-x86_64/Dockerfile b/...arget/testsubtarget/openwrt-imagebuilder-testtarget-testsubtarget.Linux-x86_64/Dockerfile
@@ -2,11 +2,10 @@ FROM alpine
 
 RUN apk add make bash
 
-RUN adduser -D builder -h /builder/
+RUN adduser -D buildbot -h /builder/
 
-USER builder 
+USER buildbot
 
 ADD ./ /builder/
 
 WORKDIR /builder/
-