more cleanups

Signed-off-by: Paul Spooren <[email protected]>

asu/api.py

@@ -158,9 +158,7 @@ def return_job_v1(job):
         response.update(job.meta)
 
     if job.is_failed:
-        response.update(
-            {"status": 500, "error": job.latest_result().exc_string}
-        )
+        response.update({"status": 500, "error": job.latest_result().exc_string})
 
     elif job.is_queued:
         response.update(

asu/build.py

@@ -8,10 +8,12 @@
 from rq import get_current_job
 
 from asu.common import (
+    check_manifest,
     diff_packages,
     fingerprint_pubkey_usign,
     get_container_version_tag,
     get_packages_hash,
+    parse_manifest,
     report_error,
     run_container,
 )
@@ -54,13 +56,13 @@ def build(req: dict, job=None):
     podman.images.pull(image)
     log.info(f"Pulling {image}... done")
 
-    returncode, stdout, stderr = run_container(image, ["make", "info"])
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
+        image, ["make", "info"]
+    )
 
-    job.meta["stdout"] = stdout
-    job.meta["stderr"] = stderr
     job.save_meta()
 
-    version_code = re.search('Current Revision: "(r.+)"', stdout).group(1)
+    version_code = re.search('Current Revision: "(r.+)"', job.meta["stdout"]).group(1)
 
     if "version_code" in req:
         if version_code != req.get("version_code"):
@@ -70,20 +72,19 @@ def build(req: dict, job=None):
             )
 
     default_packages = set(
-        re.search(r"Default Packages: (.*)\n", stdout).group(1).split()
+        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(req["profile"]),
-            stdout,
+            job.meta["stdout"],
             re.MULTILINE,
         )
         .group(1)
         .split()
     )
-    log.debug(f"Profile packages: {profile_packages}")
 
     if req.get("diff_packages"):
         req["packages"] = diff_packages(
@@ -142,7 +143,7 @@ def build(req: dict, job=None):
             },
         )
 
-    returncode, stdout, stderr = run_container(
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
         image,
         [
             "make",
@@ -154,48 +155,35 @@ def build(req: dict, job=None):
         mounts=mounts,
     )
 
-    job.meta["stdout"] = stdout
-    job.meta["stderr"] = stderr
     job.save_meta()
 
     if returncode:
         report_error(job, "Impossible package selection")
 
-    manifest = dict(map(lambda pv: pv.split(" - "), stdout.splitlines()))
-
+    manifest = parse_manifest(job.meta["stdout"])
     log.debug(f"Manifest: {manifest}")
 
-    for package, version in req.get("packages_versions", {}).items():
-        if package not in manifest:
-            report_error(
-                job, f"Impossible package selection: {package} not in manifest"
-            )
-        if version != manifest[package]:
-            report_error(
-                job,
-                f"Impossible package selection: {package} version not as requested: "
-                f"{version} vs. {manifest[package]}",
-            )
+    # Check if all requested packages are in the manifest
+    if err := check_manifest(manifest, req.get("packages_versions", {})):
+        report_error(job, err)
 
-    manifest_packages = manifest.keys()
+    packages_hash = get_packages_hash(manifest.keys())
+    log.debug(f"Packages Hash: {packages_hash}")
 
-    log.debug(f"Manifest Packages: {manifest_packages}")
-
-    packages_hash = get_packages_hash(manifest_packages)
-    log.debug(f"Packages Hash {packages_hash}")
-
-    build_cmd = [
+    job.meta["build_cmd"] = [
         "make",
         "image",
         f"PROFILE={req['profile']}",
         f"PACKAGES={' '.join(sorted(req.get('packages', [])))}",
         f"EXTRA_IMAGE_NAME={packages_hash}",
         f"BIN_DIR=/builder/{bin_dir}",
     ]
+
+    # Check if custom rootfs size is requested
     if rootfs_size_mb := req.get("rootfs_size_mb"):
-        build_cmd.append(f"ROOTFS_PARTSIZE={rootfs_size_mb}")
+        job.meta["build_cmd"].append(f"ROOTFS_PARTSIZE={rootfs_size_mb}")
 
-    log.debug("Build command: %s", build_cmd)
+    log.debug("Build command: %s", job.meta["build_cmd"])
 
     job.meta["imagebuilder_status"] = "building_image"
     job.save_meta()
@@ -207,7 +195,7 @@ def build(req: dict, job=None):
         )
         defaults_file.parent.mkdir(parents=True)
         defaults_file.write_text(req["defaults"])
-        build_cmd.append(f"FILES={req['store_path'] / bin_dir / 'files'}")
+        job.meta["build_cmd"].append(f"FILES={req['store_path'] / bin_dir / 'files'}")
         mounts.append(
             {
                 "type": "bind",
@@ -217,19 +205,19 @@ def build(req: dict, job=None):
             },
         )
 
-    returncode, stdout, stderr = run_container(
-        image, build_cmd, mounts=mounts, copy=["/builder/" + bin_dir, req["store_path"]]
+    returncode, job.meta["stdout"], job.meta["stderr"] = run_container(
+        image,
+        job.meta["build_cmd"],
+        mounts=mounts,
+        copy=["/builder/" + bin_dir, req["store_path"]],
     )
 
-    job.meta["stdout"] = stdout
-    job.meta["stderr"] = stderr
-    job.meta["build_cmd"] = build_cmd
     job.save_meta()
 
     if returncode:
         report_error(job, "Error while building firmware. See stdout/stderr")
 
-    if "is too big" in stderr:
+    if "is too big" in job.meta["stderr"]:
         report_error(job, "Selected packages exceed device storage")
 
     json_file = Path(req["store_path"] / bin_dir / "profiles.json")
@@ -239,11 +227,10 @@ def build(req: dict, job=None):
 
     json_content = json.loads(json_file.read_text())
 
+    # Check if profile is in JSON file
     if req["profile"] not in json_content["profiles"]:
         report_error(job, "Profile not found in JSON file")
 
-    now_timestamp = int(datetime.now().timestamp())
-
     json_content.update({"manifest": manifest})
     json_content.update(json_content["profiles"][req["profile"]])
     json_content["id"] = req["profile"]
@@ -258,6 +245,7 @@ def build(req: dict, job=None):
 
     job.connection.sadd(f"builds:{version_code}:{req['target']}", req["request_hash"])
 
+    # Increment stats
     job.connection.hincrby(
         "stats:builds",
         "#".join([req["version"], req["target"], req["profile"]]),

asu/common.py

@@ -285,3 +285,35 @@ def report_error(job, msg):
     job.meta["detail"] = f"Error: {msg}"
     job.save_meta()
     raise
+
+
+def parse_manifest(manifest_content: str):
+    """Parse a manifest file and return a dictionary
+
+    Args:
+        manifest (str): Manifest file content
+
+    Returns:
+        dict: Dictionary of packages and versions
+    """
+    return dict(map(lambda pv: pv.split(" - "), manifest_content.splitlines()))
+
+
+def check_manifest(manifest, packages_versions):
+    """Validate a manifest file
+
+    Args:
+        manifest (str): Manifest file content
+        packages_versions (dict): Dictionary of packages and versions
+
+    Returns:
+        str: Error message or None
+    """
+    for package, version in packages_versions.items():
+        if package not in manifest:
+            return f"Impossible package selection: {package} not in manifest"
+        if version != manifest[package]:
+            return (
+                f"Impossible package selection: {package} version not as requested: "
+                f"{version} vs. {manifest[package]}"
+            )

tests/conftest.py

@@ -1,6 +1,5 @@
 import shutil
 import tempfile
-from pathlib import Path
 
 import prometheus_client
 import pytest
@@ -200,19 +199,6 @@ def client(app):
     return app.test_client()
 
 
-@pytest.fixture
-def runner(app):
-    return app.test_cli_runner()
-
-
 @pytest.fixture(scope="session")
 def httpserver_listen_address():
     return ("127.0.0.1", 8001)
-
-
-@pytest.fixture
-def upstream(httpserver):
-    base_url = "/snapshots/targets/testtarget/testsubtarget"
-    upstream_path = Path("./tests/upstream/snapshots/targets/testtarget/testsubtarget/")
-
-    httpserver.check_assertions()

tests/test_api.py

@@ -156,6 +156,7 @@ def test_api_build_packages_versions(client):
     assert response.status == "200 OK"
     assert response.json.get("request_hash") == request_hash
 
+
 def test_api_build_packages_versions_bad(client):
     response = client.post(
         "/api/v1/build",
@@ -169,7 +170,11 @@ def test_api_build_packages_versions_bad(client):
     request_hash = response.json["request_hash"]
     response = client.get(f"/api/v1/build/{request_hash}")
     assert response.status == "500 INTERNAL SERVER ERROR"
-    assert response.json.get("detail") == "Error: Impossible package selection: test1 version not as requested: 0.0 vs. 1.0"
+    assert (
+        response.json.get("detail")
+        == "Error: Impossible package selection: test1 version not as requested: 0.0 vs. 1.0"
+    )
+
 
 def test_api_build_packages_duplicate(client):
     response = client.post(

tests/test_common.py

@@ -71,3 +71,15 @@ def test_get_version_container_tag():
     assert get_container_version_tag("1.0.0") == "v1.0.0"
     assert get_container_version_tag("SNAPSHOT") == "master"
     assert get_container_version_tag("1.0.0-SNAPSHOT") == "openwrt-1.0.0"
+
+
+def test_check_manifest():
+    assert check_manifest({"test": "1.0"}, {"test": "1.0"}) == None
+    assert (
+        check_manifest({"test": "1.0"}, {"test": "2.0"})
+        == "Impossible package selection: test version not as requested: 2.0 vs. 1.0"
+    )
+    assert (
+        check_manifest({"test": "1.0"}, {"test2": "1.0"})
+        == "Impossible package selection: test2 not in manifest"
+    )

tests/test_janitor.py

@@ -1,9 +1,7 @@
 from pathlib import Path
 
 import pytest
-from pytest_httpserver import HTTPServer
 
-from asu.build import build
 from asu.janitor import *