From f40c8d1410ee6ea25fa2a18efd90dbda1ea261ce Mon Sep 17 00:00:00 2001 From: Eric Fahlgren Date: Tue, 15 Oct 2024 08:38:56 -0700 Subject: [PATCH] builder: add more consistency and finer granularity to build status Ensure that 'imagebuilder_status' is reported at all phases of build, and add a final "done" status. Ensure that all GET and HEAD requests to '/build' receive all x-headers regardless of build phase. After reading through all the FastAPI docs, it turns out you can put '@get' and '@head' on the same function and it works just as you would expect. Reduces code size a bit. Extended the 'build_get' test to also include the HEAD request and confirm that sequential HEAD and GET requests receive the same data. Signed-off-by: Eric Fahlgren --- asu/build.py | 10 ++++++- asu/routers/api.py | 67 ++++++++++++++++++---------------------------- asu/util.py | 3 ++- tests/test_api.py | 27 ++++++++++++++++++- 4 files changed, 63 insertions(+), 44 deletions(-) diff --git a/asu/build.py b/asu/build.py index d021ecfb..41c3d791 100644 --- a/asu/build.py +++ b/asu/build.py @@ -45,6 +45,7 @@ def build(build_request: BuildRequest, job=None): job = job or get_current_job() job.meta["detail"] = "init" + job.meta["imagebuilder_status"] = "init" job.meta["request"] = build_request job.save_meta() @@ -77,6 +78,9 @@ def build(build_request: BuildRequest, job=None): } ) + job.meta["imagebuilder_status"] = "container_setup" + job.save_meta() + log.info(f"Pulling {image}...") podman.images.pull(image) log.info(f"Pulling {image}... done") @@ -166,6 +170,7 @@ def build(build_request: BuildRequest, job=None): container, ["make", "info"] ) + job.meta["imagebuilder_status"] = "validate_revision" job.save_meta() version_code = re.search('Current Revision: "(r.+)"', job.meta["stdout"]).group(1) @@ -202,7 +207,7 @@ def build(build_request: BuildRequest, job=None): ) log.debug(f"Diffed packages: {build_cmd_packages}") - job.meta["imagebuilder_status"] = "calculate_packages_hash" + job.meta["imagebuilder_status"] = "validate_manifest" job.save_meta() returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd( @@ -377,4 +382,7 @@ def build(build_request: BuildRequest, job=None): }, ) + job.meta["imagebuilder_status"] = "done" + job.save_meta() + return json_content diff --git a/asu/routers/api.py b/asu/routers/api.py index 68fba5a7..15448d0b 100644 --- a/asu/routers/api.py +++ b/asu/routers/api.py @@ -3,6 +3,7 @@ from fastapi import APIRouter, Header from fastapi.responses import RedirectResponse, Response +from rq.job import Job from asu.build import build from asu.build_request import BuildRequest @@ -112,40 +113,37 @@ def validate_request(build_request: BuildRequest): return ({}, None) -def return_job_v1(job): - response = job.get_meta() - headers = {} +def return_job_v1(job: Job) -> tuple[dict, int, dict]: + response: dict = job.get_meta() + imagebuilder_status: str = "done" + queue_position: int = 0 + if job.meta: 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) + imagebuilder_status = "failed" elif job.is_queued: - response.update( - { - "status": 202, - "detail": "queued", - "queue_position": job.get_position() or 0, - } - ) - headers["X-Queue-Position"] = str(response["queue_position"]) - headers["X-Imagebuilder-Status"] = "queued" + queue_position = job.get_position() or 0 + response.update(status=202, detail="queued", queue_position=queue_position) + imagebuilder_status = "queued" elif job.is_started: - response.update( - { - "status": 202, - "detail": "started", - } - ) - headers["X-Imagebuilder-Status"] = response.get("imagebuilder_status", "init") + response.update(status=202, detail="started") + imagebuilder_status = response.get("imagebuilder_status", "init") elif job.is_finished: - response.update({"status": 200, **job.return_value()}) + response.update(status=200, **job.return_value()) + imagebuilder_status = "done" + + headers = { + "X-Imagebuilder-Status": imagebuilder_status, + "X-Queue-Position": str(queue_position), + } - response["enqueued_at"] = job.enqueued_at - response["request_hash"] = job.id + response.update(enqueued_at=job.enqueued_at, request_hash=job.id) logging.debug(response) return response, response["status"], headers @@ -175,21 +173,9 @@ def api_v1_update( @router.head("/build/{request_hash}") -def api_v1_build_head(request_hash: str, response: Response): - job = get_queue().fetch_job(request_hash) - if not job: - response.status_code = 404 - return None - - _, status, headers = return_job_v1(job) - response.headers.update(headers) - response.status_code = status - return None - - @router.get("/build/{request_hash}") -def api_v1_build_get(request_hash: str, response: Response): - job = get_queue().fetch_job(request_hash) +def api_v1_build_get(request_hash: str, response: Response) -> dict: + job: Job = get_queue().fetch_job(request_hash) if not job: response.status_code = 404 return { @@ -221,11 +207,10 @@ def api_v1_build_post( if build_request.client: client = build_request.client + elif user_agent.startswith("auc"): + client = user_agent.replace(" (", "/").replace(")", "") else: - if user_agent.startswith("auc"): - client = user_agent.replace(" (", "/").replace(")", "") - else: - client = "unknown/0" + client = "unknown/0" add_timestamp( f"stats:clients:{client}", diff --git a/asu/util.py b/asu/util.py index 655dbb12..e165d846 100644 --- a/asu/util.py +++ b/asu/util.py @@ -286,8 +286,9 @@ def run_cmd( def report_error(job: Job, msg: str) -> None: logging.warning(f"Error: {msg}") job.meta["detail"] = f"Error: {msg}" + job.meta["imagebuilder_status"] = "failed" job.save_meta() - raise + raise RuntimeError(msg) def parse_manifest(manifest_content: str) -> dict[str, str]: diff --git a/tests/test_api.py b/tests/test_api.py index 9aecb998..3a7a7c60 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -276,7 +276,7 @@ def test_api_build_bad_target(client): assert data.get("detail") == "Unsupported target: testtarget/testsubtargetbad" -def test_api_build_get(client): +def test_api_build_head_get(client): response = client.post( "/api/v1/build", json=dict( @@ -288,10 +288,35 @@ def test_api_build_get(client): ) data = response.json() request_hash = data["request_hash"] + + # verify HEAD response and that it has no payload + response = client.head(f"/api/v1/build/{request_hash}") + assert response.status_code == 200 + + headers = response.headers + assert headers["x-imagebuilder-status"] == "done" + assert headers["x-queue-position"] == "0" + + assert response.num_bytes_downloaded == 0 + data = response.text + assert data == "" + + # verify GET response and its JSON payload response = client.get(f"/api/v1/build/{request_hash}") assert response.status_code == 200 + + headers = response.headers + assert headers["x-imagebuilder-status"] == "done" + assert headers["x-queue-position"] == "0" + + assert response.num_bytes_downloaded > 0 data = response.json() assert data["request_hash"] == request_hash + assert data["imagebuilder_status"] == "done" + request = data["request"] + assert request["version"] == "1.2.3" + assert request["target"] == "testtarget/testsubtarget" + assert request["profile"] == "testprofile" def test_api_build_packages_versions(client):