Skip to content

feat(k8s-launcher): explain empty logs when a pod was deleted before capture - #326

Closed
morgan-wowk wants to merge 1 commit into
masterfrom
08-12-feat_k8s-launcher_explain_empty_logs_when_a_pod_was_deleted_before_capture
Closed

feat(k8s-launcher): explain empty logs when a pod was deleted before capture#326
morgan-wowk wants to merge 1 commit into
masterfrom
08-12-feat_k8s-launcher_explain_empty_logs_when_a_pod_was_deleted_before_capture

Conversation

@morgan-wowk

@morgan-wowk morgan-wowk commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #325.

What this closes

#325 stops a deleted pod from turning FAILED into SYSTEM_ERROR. But it trades a loud-wrong-status for a silent blank: with the pod gone, _get_all_logs() recovers nothing, upload_log() persists an empty log, and the runs v2 page renders the bare text "No logs available" — indistinguishable from a task that simply produced no output. The Job launcher also can't help: launcher_error_message is an unimplemented return None, so no orchestration_error_message is recorded either.

New rules

  • A vanished pod is reported as deleted, not as "no log yet". _get_log_by_pod_key now returns a _POD_DELETED sentinel on a 404, distinct from None (pod exists, HTTP 400, still initializing). _get_all_logs returns (logs, deleted_pod_keys) so callers can tell the two apart.

  • When a deleted pod is the only reason there are no logs, the persisted log is an explanatory notice instead of an empty string:

    Logs are unavailable: the pod was deleted before its logs could be captured (for example, autoscaler eviction or Kubernetes garbage collection). The task's final status still reflects what happened; only the logs were lost.

    This flows straight into the existing log pane (the runs v2 page reads the persisted artifact for terminal executions) with no frontend change.

  • The notice never masks a real result. It is substituted only when the merged log is empty and at least one pod 404'd. A pod that still exists and printed nothing stays empty; any recovered logs (even partial, when only some pods vanished) are persisted verbatim.

Why a pod vanishes before capture

Two independent causes, both seen in practice: the cluster-autoscaler evicting the node mid-run, and Kubernetes garbage-collecting a finished pod before the orchestrator reads it. Both surface identically as a 404 on read_namespaced_pod_log.

Tests

  • tests/test_kubernetes_launcher_error_classification.py:
    • _get_log_by_pod_key returns the _POD_DELETED sentinel on 404 (still raises on 403/500).
    • upload_log/get_log persist the notice when the pod was deleted, keep an existing pod's empty output empty, and leave recovered (partial) logs untouched.
  • Full suite green (uv run pytest).

Downstream

Shopify/oasis-backend pins this repo as the backend submodule; picking this up needs a submodule pointer bump (together with #325).

morgan-wowk commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator Author

@morgan-wowk
morgan-wowk force-pushed the 08-12-fix_orchestrator_losing_pod_logs_must_not_fail_an_execution branch from 9802c98 to b2bdd00 Compare August 12, 2026 20:29
@morgan-wowk
morgan-wowk force-pushed the 08-12-feat_k8s-launcher_explain_empty_logs_when_a_pod_was_deleted_before_capture branch 2 times, most recently from 520f54f to f702c27 Compare August 12, 2026 20:49
@morgan-wowk
morgan-wowk marked this pull request as ready for review August 12, 2026 20:49
@morgan-wowk
morgan-wowk requested a review from Ark-kun as a code owner August 12, 2026 20:49
@morgan-wowk
morgan-wowk requested a review from a team August 12, 2026 20:50
@morgan-wowk
morgan-wowk changed the base branch from 08-12-fix_orchestrator_losing_pod_logs_must_not_fail_an_execution to graphite-base/326 August 13, 2026 01:45
@morgan-wowk
morgan-wowk force-pushed the 08-12-feat_k8s-launcher_explain_empty_logs_when_a_pod_was_deleted_before_capture branch from f702c27 to 3c14f2f Compare August 13, 2026 01:45
@graphite-app
graphite-app Bot changed the base branch from graphite-base/326 to master August 13, 2026 01:45
@graphite-app
graphite-app Bot requested a review from a team August 13, 2026 01:45
…capture

Signed-off-by: Morgan Wowk <morgan.wowk@shopify.com>
@morgan-wowk
morgan-wowk force-pushed the 08-12-feat_k8s-launcher_explain_empty_logs_when_a_pod_was_deleted_before_capture branch from 3c14f2f to a6b2d5b Compare August 13, 2026 01:45
# Persisted in place of an empty log when the only reason no logs were captured
# is that the Pod(s) were deleted before we could read them -- e.g. the
# cluster-autoscaler evicting the node, or Kubernetes garbage-collecting a
# finished Pod. Without this the UI shows a blank pane indistinguishable from a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The distinction between "empty log" and "missing log" can/should be done on the UI side.
The APi already returns log_text: "" vs log_text: undefined. If get_log returns None, the log_text is undefined. See

log_text: str | None = None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants