Skip to content

Commit

Permalink
builder: add more consistency and finer granularity to build status
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
efahl authored and aparcar committed Oct 17, 2024
1 parent def4b33 commit cdf2a5d
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 44 deletions.
10 changes: 9 additions & 1 deletion asu/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -377,4 +382,7 @@ def build(build_request: BuildRequest, job=None):
},
)

job.meta["imagebuilder_status"] = "done"
job.save_meta()

return json_content
67 changes: 26 additions & 41 deletions asu/routers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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}",
Expand Down
3 changes: 2 additions & 1 deletion asu/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
27 changes: 26 additions & 1 deletion tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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):
Expand Down

0 comments on commit cdf2a5d

Please sign in to comment.