Skip to content

fix(operator): fail a graph while a component crash-loops - #16426

Merged
julienmancuso merged 8 commits into
ai-dynamo:mainfrom
dinhxuanvu:vdinh/operator-crashloop-failed
Nov 5, 2026
Merged

julienmancuso merged 8 commits into
ai-dynamo:mainfrom
dinhxuanvu:vdinh/operator-crashloop-failed

Conversation

@dinhxuanvu

@dinhxuanvu dinhxuanvu commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Overview:

A graph whose component keeps crashing on start reports pending forever. Anything waiting on its state (Argo CD health checks, CI waits) can't tell a crash loop from a slow start and only gives up at its own timeout. This reports failed instead, naming the crashing Pod, container and exit code.

Details:

  • internal/pods decides whether a Pod is crash-looping: a not-Ready Pod, neither finished nor being deleted, with a container that restarted at least twice and is down after a non-zero exit. CrashLoopChanged compares two observations of a Pod.
  • internal/controller/pods finds a workload's Pods through indexes on the operator's Pod cache and decides which Pod events wake a reconcile.
  • Deployment-backed components: the DCD sets Available=False with reason ContainerCrashLooping from the Pods its Deployment selects. The DGD reports failed with reason component_crash_looping.
  • Grove components: the Grove program checks each component's Pods by graph and component labels. It fails the graph even while Grove reports an update in progress.
  • DisaggregatedSet: components it runs as DCDs, such as the frontend, are covered as above.
  • The Pod cache keeps restart counts and last exit codes for all containers.

Not covered: DisaggregatedSet workers and LeaderWorkerSet-backed DCDs. LeaderWorkerSet recreates the whole group at the first container restart, so no container reaches two restarts. Detecting repeated group recreation is a follow-up that depends on LeaderWorkerSet's recovery accounting.

Validation:

  • Unit tests and envtests pass for internal/pods, internal/controller/pods, internal/controller (including the DisaggregatedSet envtests) and internal/podcache. golangci-lint v2.15.0 reports no issues.

  • Informer-cache tests on an envtest API server: a crash-looping Pod created before the cache starts is reported once the cache syncs, and again by a rebuilt cache. Back-to-back status updates converge to the latest state.

  • Benchmark through a real informer cache, one namespace, unrelated Pods spread over many components (Apple M4 Pro):

    Unrelated Pods Component index Deployment index Label-selector scan
    1,000 4.2 µs, 16.6 KB, 44 allocs 5.3 µs, 21.5 KB, 53 allocs 37.7 µs, 48 KB
    5,000 3.5 µs, 16.6 KB, 44 allocs 4.6 µs, 21.2 KB, 53 allocs 191.7 µs, 179 KB
    10,000 3.5 µs, 16.6 KB, 44 allocs 4.6 µs, 21.2 KB, 53 allocs 434.8 µs, 343 KB
  • Kind cluster with Grove v0.1.0-alpha.15 and LWS v0.10.0: Grove, DisaggregatedSet-frontend and Deployment-backed graphs report failed within 15s of a crash loop and recover within 5s of the fix. The Grove graph stays failed across an operator restart, and its message follows the restart count.

Where should the reviewer start?

deploy/operator/internal/pods/crashloop.go, then deploy/operator/internal/controller/pods/.

Related Issues

Summary by CodeRabbit

  • New Features
    • Deployment status now identifies containers repeatedly restarting and failing in the current revision, with details about the affected containers.
    • Graph deployments report a failed state when a component is crash-looping, including during a rolling update.
  • Documentation
    • Updated Kubernetes API references to describe crash-loop status and its effect on graph deployment state.

@dinhxuanvu
dinhxuanvu requested review from a team as code owners October 14, 2026 19:21
@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 14, 2026 19:21 — with GitHub Actions Inactive
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 14, 2026 19:21 — with GitHub Actions Inactive
@github-actions github-actions Bot added the fix label Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot added external-contribution Pull request is from an external contributor documentation Improvements or additions to documentation deployment::k8s Relates to dynamo deployment in kubernetes labels Oct 14, 2026
@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 64139322-b2d8-412c-8d6e-d21d8e248ed8
📥 Commits

Reviewing files that changed from the base of the PR and between 2db5333 and 76eb464.

📒 Files selected for processing: 14
  • deploy/helm/charts/platform/components/operator/files/role.yaml
  • deploy/operator/internal/controller/component_crash_loop.go
  • deploy/operator/internal/controller/component_crash_loop_test.go
  • deploy/operator/internal/controller/dgd_component_workloads_reconciler.go
  • deploy/operator/internal/controller/dgd_workload_program_test.go
  • deploy/operator/internal/controller/dgd_workload_result.go
  • deploy/operator/internal/controller/dgd_workload_result_test.go
  • deploy/operator/internal/controller/dynamocomponentdeployment_controller.go
  • deploy/operator/internal/controller/dynamocomponentdeployment_controller_test.go
  • deploy/operator/internal/controller/dynamographdeployment_controller.go
  • deploy/operator/internal/podcache/transform.go
  • deploy/operator/internal/podcache/transform_test.go
  • docs/fern/pages/reference/kubernetes-api/dynamo-component-deployment.mdx
  • docs/fern/pages/reference/kubernetes-api/dynamo-graph-deployment.mdx

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The operator now detects crash-looping containers in Pods from a Deployment’s current template. It reports the condition in DCD status and projects it into DGD failure status, including during rolling updates.

Changes

Deployment crash-loop detection

Layer / File(s) Summary
Pod status and current-template detection
deploy/operator/internal/podcache/transform.go, deploy/operator/internal/podcache/transform_test.go, deploy/operator/internal/controller/component_crash_loop.go, deploy/operator/internal/controller/component_crash_loop_test.go
Pod status projection retains restart counts and last-termination exit code and reason for regular and init containers. Detection checks readiness, phase, restart count, termination state, and the current Deployment template hash.
DCD detection and reconciliation
deploy/helm/charts/platform/components/operator/files/role.yaml, deploy/operator/internal/controller/dynamocomponentdeployment_controller.go, deploy/operator/internal/controller/dynamocomponentdeployment_controller_test.go, deploy/operator/internal/controller/component_crash_loop.go, deploy/operator/internal/controller/component_crash_loop_test.go
ReplicaSet read, list, and watch permissions are added. DCD reconciliation reports qualifying crash loops instead of the generic not-ready reason, and a Pod watch maps qualifying events to DCD requests. Tests cover current and older Deployment revisions and Pod event predicates.
DGD failure projection
deploy/operator/internal/controller/dgd_component_workloads_reconciler.go, deploy/operator/internal/controller/dgd_workload_result.go, deploy/operator/internal/controller/dgd_workload_result_test.go, deploy/operator/internal/controller/dgd_workload_program_test.go, deploy/operator/internal/controller/dynamographdeployment_controller.go, docs/fern/pages/reference/kubernetes-api/dynamo-component-deployment.mdx, docs/fern/pages/reference/kubernetes-api/dynamo-graph-deployment.mdx
Current-generation DCD crash-loop conditions set the workload result to failed with sorted component details. Tests cover the projection and a rolling update. The API references describe the DCD condition and resulting DGD state.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 76eb4

Crash-loop status updates reach both deployment controllers. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Issue #16425 requires failure reporting for crash-looping Deployment-backed components. The implementation resolves the current Deployment revision, filters to non-terminal and not-Ready Pods, and req…
Out of Scope Changes check Passed The changes stay within issue #16425. The ReplicaSet RBAC, pod-cache retention, event predicates, controller tests, DGD tests, and API documentation support crash-loop detection or status reporting. T…
Title check Passed The title clearly and concisely describes the main change: failing a graph when a component enters a crash loop.
Description check Passed The description includes the required Overview, Details, reviewer guidance, and Related Issues sections. It explains the behavior, scope, validation, and links issue #16425.
Full details: Linked Issues check

Explanation

Issue #16425 requires failure reporting for crash-looping Deployment-backed components. The implementation resolves the current Deployment revision, filters to non-terminal and not-Ready Pods, and requires at least two restarts after a non-zero exit. The DCD message includes the Pod, container, restart count, and exit code. The DGD projects a current-generation DCD condition with reason ContainerCrashLooping to state failed with reason component_crash_looping. Tests cover detection, revision selection, DCD reconciliation, DGD projection, and pod-cache status retention.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #16425. The ReplicaSet RBAC, pod-cache retention, event predicates, controller tests, DGD tests, and API documentation support crash-loop detection or status reporting. The reviewed summary reports no Grove or LWS behavior changes. No unrelated change is demonstrated.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 14, 2026 19:36 — with GitHub Actions Inactive
@devin-ai-integration

Copy link
Copy Markdown
Contributor

External PR triage

Blocker (ci-approval-needed): Trusted CI has never run: no /ok to test has been posted on this PR. The code-owner review request has had no response for ~5 days.

Next action: Comment /ok to test 73506ab65afcd4997491db32f8d63d646f39a5e4, then review, paying particular attention to the RBAC change below. Owner: @ai-dynamo/dynamo-operator-codeowners @ai-dynamo/dynamo-docs-codeowners.

Supply-chain audit of 73506ab6: Safe to approve CI, with one item for the reviewer. The diff changes Go controller and podcache logic plus tests and docs under deploy/operator/internal/. It also grows the operator ClusterRole in deploy/helm/charts/platform/components/operator/files/role.yaml: replicasets is added to the existing read-only apps rule (get/list/watch). That is a small read-only privilege increase, consistent with detecting component crash-loops, but an operator owner should confirm it. New imports are only existing k8s.io/controller-runtime/testify modules, and go.mod is unchanged. No workflow/CI files, no dependency manifest changes (Cargo.toml/Cargo.lock/pyproject.toml/go.mod/requirements), no new download URLs, scripts, secret/credential access, or binary blobs. (Devin has not posted /ok to test or approved anything; a maintainer still has to vet and run CI.)

Automated triage, 2026-10-19. This comment is edited in place when the PR changes.

Written by Devin

@sttts sttts 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.

Three inline comments on workload hierarchy dependencies, shared Pod diagnostics, and validating the design with Grove and DisaggSet before merging the DCD implementation.

@dinhxuanvu
dinhxuanvu force-pushed the vdinh/operator-crashloop-failed branch from 73506ab to d891155 Compare October 26, 2026 13:43
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 26, 2026 13:43 — with GitHub Actions Inactive
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 26, 2026 14:42 — with GitHub Actions Inactive

@sttts sttts 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.

Two inline comments on completing the package boundary: shared Pod controller integration in internal/controller/pods, and pure diagnosis-change detection in internal/pods. Workload ordering and graph-status policy stay with the workload programs.

@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 27, 2026 14:08 — with GitHub Actions Inactive
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 27, 2026 15:09 — with GitHub Actions Inactive
@dinhxuanvu
dinhxuanvu force-pushed the vdinh/operator-crashloop-failed branch from 183dca2 to 664e516 Compare October 27, 2026 15:14
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 27, 2026 15:14 — with GitHub Actions Inactive

@sttts sttts 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.

Three inline simplification suggestions: reuse controller-runtime's mapped event handler, avoid an unconstrained generic solely for status formatting, and select the first failing Pod without sorting the whole collection. These are maintainability suggestions, not additional functional bug findings.

@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator October 29, 2026 15:34 — with GitHub Actions Inactive
@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@sttts Addressed your feedback. PTAL. Thanks

@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@sttts Hey Stefan, code freeze for next release is tomorrow so would love to have this PR merged if possible. PTAL. Thanks.

@sttts

sttts commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

/ok to test 7700e29

@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator November 3, 2026 16:31 — with GitHub Actions Inactive
@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@sttts Hey Stefan, would you mind re-trigger the tests? Some tests were failing due to the branch is too old and missed out on a PR that fixed CI (#17285).

@sttts

sttts commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

/ok to test cbd72c6

@devin-ai-integration

devin-ai-integration Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 41905812821 (attempt 1) on dd34515ad7

Gate checks: ✅ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Posted automatically by Devin for run 41905812821. Updated on every full-CI run of this PR.

@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@sttts The two failing required checks on cbd72c6a34 are caused by CI infrastructure:

  • deploy-status-check: vLLM Snapshot Operator Setup times out because the vcluster-hpm-hostpath-mapper pods on aks-a100b-21135548-vmss000001 and aks-a100a-56471985-vmss000002 never start after being scheduled. Those two nodes failed this job in both runs here and both runs of feat(vllm): wire runtime LoRA into worker requests #15032. Could someone with cluster access look at them?
  • backend-status-check: the vllm-runtime build failed on a temporary NVIDIA package server error (File has unexpected size ... Mirror sync in progress?). Could you rerun it?

@julienmancuso

Copy link
Copy Markdown
Contributor

@dinhxuanvu could you rebase please ?

A DynamoGraphDeployment whose worker crash-loops stays pending indefinitely,
so tools that wait on its state, such as an Argo CD health check, cannot tell
a crash loop from a slow start and wait out their own timeout.

The DCD controller sets Available=False with reason ContainerCrashLooping when
a not-Ready Pod of the current template has a container that restarted at
least twice and is down after a non-zero exit. The DGD projects that reason
into state failed with reason component_crash_looping, also during a rolling
update. Reading Pods and ReplicaSets needs a new RBAC grant on replicasets.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
Resolving the Deployment's current ReplicaSet needed extra RBAC and
missed crash loops while that ReplicaSet was not yet cached. Select Pods
by the Deployment's labels instead. Pod-level diagnosis is in
internal/pods so other workload programs can share it. Terminating Pods
are ignored; Pods of an older template still count during a rollout.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
Signed-off-by: Vu Dinh <vudinh@outlook.com>
…tends

A crash-looping Grove component left the graph pending. So did a
frontend that DisaggregatedSet lowers to a DCD. The Grove program now
diagnoses each component's labeled Pods, woken by a Pod watch. The
DisaggregatedSet program applies the DCD crash-loop condition.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
…lookups

The Pod index, the queries that use it and the crash-loop watch were spread
over three controller files. The Deployment lookup scanned every cached Pod
in the namespace. They now live in internal/controller/pods, and both lookups
go through an index. A relabeled Pod wakes its old and new owner.
internal/pods gains CrashLoopChanged so the watch and the query agree on
when a diagnosis changes.

The reference pages state that DisaggregatedSet workers and
LeaderWorkerSet-backed components are not covered.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
Deleting a healthy Pod, relabeling an unowned one or a Grove Pod's crash
loop woke a reconcile that could not change any diagnosis. Grove Pods carry
the DCD selector label, so their changes enqueued DCDs that do not exist.
The crash-loop diagnosis now formats directly into the graph status. The
new code and tests follow the operator's comment and test-story rules.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
EnqueueOwners wraps controller-runtime's mapped handler, which already
enqueues both the old and new owner of an updated Pod. FindCrashLoop finds
the crash-looping Pod with the smallest name in one pass without sorting
its input. failCrashLoopingComponents takes diagnosis strings.

Signed-off-by: Vu Dinh <vudinh@outlook.com>
@dinhxuanvu
dinhxuanvu force-pushed the vdinh/operator-crashloop-failed branch from cbd72c6 to dd34515 Compare November 4, 2026 15:22
@dinhxuanvu
dinhxuanvu temporarily deployed to external_collaborator November 4, 2026 15:22 — with GitHub Actions Inactive
@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@julienmancuso Done!

@dinhxuanvu

Copy link
Copy Markdown
Contributor Author

@julienmancuso Would you mind retriggering the test suite? Thanks!

@julienmancuso

Copy link
Copy Markdown
Contributor

/ok to test dd34515

@julienmancuso
julienmancuso merged commit 20c5f11 into ai-dynamo:main Nov 5, 2026
112 checks passed
ishandhanani added a commit to ishandhanani/dynamo that referenced this pull request Nov 5, 2026
Fourth upstream merge for the split v2 PR: the five commits past
02331f0. Four conflicts, all from ai-dynamo#18009, placed in the owner crates;
upstream behavior whole, the stack's moves intact, no shims.

- ai-dynamo#18009 keep prepared rows when ranks are past the reject thresholds:
  is_worker_full(dp_size)/skips_full_workers in the kernel's filter.rs;
  the actor keeps prepared rows and the kernel's select_with_policy drops
  the rows of workers whose every rank is full before the policy adopts
  them (PreparedRows::retain in worker-selection; the policy recounts the
  best cached prefix from the rows it adopts and reports whether any is
  left); prepared rows carry dp_size (u16, so the 128-byte row holds) set
  in the kernel's CandidatePreparerExt::prepare; the three new kernel
  queue tests and the relocated test helpers updated.
- ai-dynamo#18108 CRTC decode stores appended at the leaf tail under one lock:
  lib/router/kv-indexer concurrent_radix_tree_compressed (auto-merged).
- ai-dynamo#16258 vLLM backend defaults for omitted token limits: lib/llm
  preprocessor, lib/sidecar/vllm, components, docs, tests (auto-merged).
- ai-dynamo#18152 planner ARIMA forecasts, ai-dynamo#16426 operator crash-loop: outside the
  router (auto-merged).

Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>

This branch was previously deployed

1 inactive deployment
external_collaborator — dd34515a Deployed Nov 4, 2026 by dinhxuanvu via ok-to-test #45451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment::k8s Relates to dynamo deployment in kubernetes documentation Improvements or additions to documentation external-contribution Pull request is from an external contributor fix size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: DGD stays pending while a worker crash-loops

3 participants