Mcp get log job fallback - #314
Conversation
|
please rebase |
12a0c90 to
f2ba72c
Compare
A job that fails before producing results never reaches the dashboard, so get_log could not fetch its log at all: get_log and get_build both return "not found", while Maestro holds it as an incomplete job with the LAVA callback attached. Fall back to that callback when the dashboard has no test for the id. The id is accepted in either form, since Maestro takes the bare hex and the dashboard takes it origin-prefixed. The callback is a JSON document rather than a log file, so it cannot be streamed into a bounded buffer. It is capped at MAX_CALLBACK_BYTES on both the download and the decompressed size, and the extracted log is then bounded as before. The response reports which source was used. Reported-by: Yogesh Lal <yogesh.lal@oss.qualcomm.com> Signed-off-by: Ben Copeland <ben.copeland@linaro.org>
f2ba72c to
2b8625d
Compare
| raise KciDevError("Log gzip stream is incomplete or malformed") | ||
|
|
||
|
|
||
| def _bare_node_id(node_id): |
There was a problem hiding this comment.
Fallback can return a Maestro log for a non-Maestro ID as any Dashboard "not found" result triggers _job_callback_log()
If the Dashboard does not contain other-origin:abcdef, but Maestro happens to contain node abcdef, get_log() could return an unrelated Maestro job's log instead of reporting that the original test was not found.
| raw = bytes(body) | ||
| if raw[:2] == b"\x1f\x8b": | ||
| expanded = bytearray() | ||
| for piece in _gunzip_iter(iter([raw])): |
There was a problem hiding this comment.
malformed gzip can leak zlib.error
| "kind": "job", | ||
| "result": "incomplete", | ||
| "data": {"error_code": "Infrastructure", "error_msg": "Unable to flash"}, | ||
| "artifacts": {"callback_data": callback_url} if callback_url else {}, |
There was a problem hiding this comment.
The current KernelCI pipeline says
# Temporarily we dont remove log field
# data.pop('log', None)
Maybe consider lava_log -> lava_logs -> callback_data["log"] or otherwise explain/document why depending on the temporary callback log field is intentional.
|
kcidev/api.py now imports gzip, but the production implementation does not use it. The tests import gzip locally instead. |
|
Please restrict the Maestro fallback to bare IDs or IDs explicitly prefixed with maestro:. Currently _bare_node_id() strips any origin prefix, so a missing other-origin: Dashboard test could accidentally resolve and return an unrelated Maestro log. Please preserve the Dashboard error for non-Maestro origins and add a regression test for this case. |
|
_job_callback_log() can call _gunzip_iter(), but unlike the normal Dashboard log path it does not catch zlib.error. A malformed gzip callback can therefore leak a raw exception from KernelCIClient.get_log() instead of KciDevError. Please catch zlib.error here and add a malformed-gzip regression test. |
2 participants
A job that fails before producing results never reaches the dashboard, so get_log could not fetch its log at all. Reported by @yogeshlal while testing #306 against a triage agent: a node whose device failed to flash returns "not found" from both get_log and get_build, while Maestro holds it as an incomplete job with the LAVA callback attached.
get_log now falls back to that callback when the dashboard has no test for the id, and accepts the id in either form since Maestro takes the bare hex and the dashboard takes it origin-prefixed. The callback is JSON rather than a log file, so it cannot be streamed into a bounded buffer; it is capped on both the download and the decompressed size, with the same deadline as the dashboard path. The response reports which source was used.
get_node already returns the infra diagnosis for these nodes and is usually the better answer; this is for when the raw log is wanted.
Based on #306, which needs to merge first.