Skip to content

feat(dpf): report DPU conditions and agent status in the host snapshot - #5374

Open
abvarshney-nv wants to merge 1 commit into
NVIDIA:mainfrom
abvarshney-nv:feat/dpf-snapshot-dpu-status-fields
Open

feat(dpf): report DPU conditions and agent status in the host snapshot#5374
abvarshney-nv wants to merge 1 commit into
NVIDIA:mainfrom
abvarshney-nv:feat/dpf-snapshot-dpu-status-fields

Conversation

@abvarshney-nv

Copy link
Copy Markdown
Contributor

dpf snapshot reported only phase and bfbFile from a DPU's status, so it answered where provisioning had reached but never why it had stopped there. DPF has since grown three status surfaces that carry the reason.

This adds them to the snapshot payload, verbatim rather than flattened:

  • conditions — the reason and message explaining the phase
  • operationalConditions — the DPU's health once provisioned, a separate axis from provisioning progress
  • agentStatus — what the DPU-side agent reports about itself

Passing the CR types through unchanged keeps the payload lossless and means a field DPF adds later shows up without another change here. The trade is that keys inside these three are the CRD's own camelCase while the wrapper keys stay snake_case:

"status_phase": "Ready",
"status_conditions": [
  { "type": "Ready", "status": "True", "reason": "DPUReady", "observedGeneration": 3 }
],
"status_agent_status": { "kubeletVersion": "v1.31.4", "rebootSequenceCount": 2 }

HostDpfSnapshot is documented as "not a stable wire format", and the only other consumer (machine-controller/src/dpf.rs) reads just dpu_node and dpu_devices, so nothing downstream is affected.

Related issues

None filed.

Type of Change

  • Add - New feature or capability

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

snapshot_host had no test coverage before this. New crates/dpf/src/test/sdk_host_snapshot.rs covers all three surfaces reported together, partial status not dragging absent fields into existence, and a DPU with no status at all.

Unrelated and pre-existing on main: cargo check -p carbide-api-core fails on method 'key' is never used at crates/api-core/src/api.rs:3706. Confirmed present without this change.

@copy-pr-bot

copy-pr-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 16f2a9a5-4630-479d-91ad-f240e822ff7d

📥 Commits

Reviewing files that changed from the base of the PR and between 0702bd9 and 87be6dc.

📒 Files selected for processing (5)
  • crates/dpf/src/sdk.rs
  • crates/dpf/src/test/mod.rs
  • crates/dpf/src/test/sdk_host_snapshot.rs
  • crates/dpf/src/types.rs
  • docs/manuals/dpf.md

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


Summary by CodeRabbit

  • New Features

    • DPU snapshots now include status conditions, operational health, and agent status when available.
    • Snapshot results retain DPU status details, including provisioning phase and installed BFB information.
    • DPUs without populated status remain visible, with unavailable fields shown as null.
  • Documentation

    • Updated snapshot documentation to describe the available DPU status fields and null values.

Walkthrough

The SDK host snapshot now includes DPU conditions, operational conditions, and agent status. New tests cover populated and absent status fields. The DPF documentation describes these fields and their null representation.

Changes

DPU snapshot status

Layer / File(s) Summary
DPU status summary contract
crates/dpf/src/types.rs
DpuSummary now exposes optional condition, operational-condition, and agent-status fields.
Host snapshot status mapping
crates/dpf/src/sdk.rs
snapshot_host copies the DPU status fields into each DPU summary.
Snapshot status validation and documentation
crates/dpf/src/test/sdk_host_snapshot.rs, crates/dpf/src/test/mod.rs, docs/manuals/dpf.md
Tests validate populated, partially absent, and entirely absent status fields. The documentation describes the fields and their null representation.

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

Merge Risk: ⚪ Minimal · up to 87be6

This change adds DPU condition, operational-condition, and agent-status details to the host snapshot without changing existing fields or downstream behavior; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the addition of DPU conditions, operational conditions, and agent status to the host snapshot, including testing details.
Title check ✅ Passed The title clearly and concisely identifies the main change: reporting DPU conditions and agent status in the host snapshot.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@abvarshney-nv
abvarshney-nv marked this pull request as ready for review August 26, 2026 10:44
@abvarshney-nv
abvarshney-nv requested review from a team and polarweasel as code owners August 26, 2026 10:44
@github-actions

Copy link
Copy Markdown

`dpf snapshot` reported only `phase` and `bfbFile` from a DPU's status, so
it answered where provisioning had reached but never why it had stopped
there. DPF has since grown three status surfaces that carry the reason.

Adds them to `DpuSummary`, verbatim rather than flattened:

  - `conditions`, whose reason and message explain the phase
  - `operationalConditions`, the DPU's health once provisioned, which is a
    separate axis from provisioning progress
  - `agentStatus`, what the DPU-side agent reports about itself

Passing the CR types through unchanged keeps the payload lossless and
means a field DPF adds later shows up without another change here. The
keys inside each are therefore the CRD's own camelCase, not this
payload's snake_case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@abvarshney-nv
abvarshney-nv force-pushed the feat/dpf-snapshot-dpu-status-fields branch from 87be6dc to 1366aca Compare August 26, 2026 10:51
@abvarshney-nv
abvarshney-nv removed the request for review from polarweasel August 26, 2026 15:08
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