Skip to content

fix(controller): fail rollouts with invalid Job templates - #657

Open
Chen Yufeiyang (feiiiiii5) wants to merge 1 commit into
microsoft:mainfrom
feiiiiii5:agent-lightning-C-212
Open

Chen Yufeiyang (feiiiiii5) wants to merge 1 commit into
microsoft:mainfrom
feiiiiii5:agent-lightning-C-212

Conversation

@feiiiiii5

Copy link
Copy Markdown

A rollout whose config.k8s.job_template cannot render or parse never reaches a terminal state. _create_job() calls build_job_spec() inside the Kubernetes API try/except, so a deterministic template/manifest error (TemplateSyntaxError, a YAML parse error, metadata: null) is classified as transient unless its message happens to contain 422, unprocessable, or invalid. The rollout stays QUEUING, the controller logs “will retry” on every poll, and AglRolloutManager.enqueue_and_wait_until_completed() waits forever.

Manifest construction happens before any Kubernetes request and is deterministic, so retrying the same rollout cannot succeed. This separates the build step from the API call: a build exception marks the rollout FAILED with the validation error, while K8s API failures keep the existing retry classification.

Test: the new regression covers a Jinja syntax error and a structurally invalid Job (metadata: null); both fail on 05c7fe1 with Job creation failed — will retry and no status patch, and pass here. The controller/server/client/package suites are 75 passed; ruff check/format --check are clean.

Details

The regression mocks _patch_status but exercises the real build_job_spec() failures. The trainer-side hang is not executed end-to-end because it requires the optional verl/torch group; it follows from the manager polling until every rollout is terminal. No live Kubernetes cluster is used.

build_job_spec ran inside the Kubernetes API try/except, so a
deterministic template or manifest error without a 422/unprocessable/
invalid substring was treated as retryable and the rollout stayed
QUEUING forever. Build errors now mark the rollout FAILED; API failures
keep the existing retry classification.
Copilot AI balanced review requested due to automatic review settings October 26, 2026 07:15

Copilot AI left a comment

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.

🟢 Approval recommended

The focused change addresses the rollout hang, preserves API retry behavior, and includes regression coverage with no identified blocking issues.

No open issues 🎉

What changed in this PR

This PR prevents rollouts with invalid Kubernetes Job templates from remaining in QUEUING indefinitely by treating manifest-build errors as terminal failures.

Changes:

  • Separates manifest construction from Kubernetes API error handling while preserving existing API retry behavior.
  • Centralizes invalid Job failure reporting.
  • Adds regression tests for Jinja syntax errors and null Job metadata.
File Description
tests/​controller/​test_k8s_reconciler_errors.py Verifies invalid templates mark rollouts as failed.
agentlightning/​controller/​k8s_reconciler.py Handles manifest-build failures separately and shares failure reporting.

Review effort: Balanced

This branch has not been deployed

No deployments
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