docs: separate host DPU policy from observed device mode#3473
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (10)
Summary by CodeRabbit
WalkthroughThe documentation adopts ChangesDPU policy documentation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full_review, thanks! |
|
ʘ‿ʘ ✅ Action performedFull review finished. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3473.docs.buildwithfern.com/infra-controller |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/manuals/vpc/flat_vpcs_zero_dpu.md`:
- Around line 103-143: Update all listed documentation sites to use the existing
DpuMode/dpu_mode contract until the policy rename lands: replace dpu_policy,
HostDpuPolicy, and --dpu-policy guidance with the current dpu_mode/DpuMode names
and CLI values, while preserving legacy behavior descriptions. Apply the
corresponding updates in docs/manuals/vpc/flat_vpcs_zero_dpu.md (lines 103-143,
73-74, and 416-418), docs/glossary.md (lines 54-61 and 159),
docs/provisioning/ingesting-hosts.md (line 192),
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md
(lines 19-20, 117-131, and 164), and expected-machine-patch.md (lines 20-21,
123-137, and 170).
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 53-73: The guide must match the implemented DPU-policy contract:
document legacy dpu_mode and --dpu-mode names with their mappings, and use
accepted CLI flags and payload fields. Update
docs/provisioning/boot-interfaces-and-dpu-modes.md lines 53-73, 97, 115,
129-137, 148-152, 161-165, 174-181, and 197-200; retain
ExpectedMachine.dpu_mode, correct the add and patch flags, update all zero-DPU,
NIC-mode, and managed-DPU payloads, and correct the admin-CLI reference table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 25e4efc6-d359-4d69-8126-1474e6bbb303
📒 Files selected for processing (9)
docs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 63-67: Update the `manage` row in the JSON/TOML and CLI policy
table so it describes DPU lifecycle management without requiring DPU/Admin boot.
State that DPU/Admin is the default boot path only when no plain-NIC primary
interface is declared, while preserving the existing inheritance behavior for
per-host `manage` declarations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d30fb5b5-5753-4503-8729-5437ce1d358d
📒 Files selected for processing (9)
docs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
|
@coderabbitai full_review, thanks! |
|
ʘ‿ʘ ✅ Action performedFull review finished. |
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/provisioning/boot-interfaces-and-dpu-modes.md (1)
63-67: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not make
manageimply a DPU boot path.This row says every managed host boots through its primary DPU/Admin overlay, but Scenario 3.4 explicitly supports managed DPUs with an integrated NIC as the primary HostInband interface. Describe
manageas controlling DPU lifecycle management; make DPU/Admin boot the default only when no plain-NIC primary is declared.As per path instructions, Markdown must remain technically correct and operator-usable.
Suggested wording
-| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo and the host boots through its primary DPU on the Admin overlay. A per-host `manage` declaration inherits the site policy rather than overriding it. | +| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo. Boot-interface selection is independent; DPU/Admin is the default only when no plain-NIC primary is declared. A per-host `manage` declaration inherits the site policy rather than overriding it. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/provisioning/boot-interfaces-and-dpu-modes.md` around lines 63 - 67, Update the `manage` row in the JSON/TOML value table to describe DPU lifecycle management without requiring a DPU/Admin boot path. State that DPU/Admin boot is the default only when no plain-NIC primary interface is declared, while preserving the documented behavior for managed DPUs with an integrated NIC as the HostInband primary.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 53-57: The policy documentation does not consistently describe
rack-management mode. Update docs/provisioning/boot-interfaces-and-dpu-modes.md
lines 53-57 to qualify the unset dpu_policy default and Admin-network boot
claims when rack_management_enabled is enabled, and update docs/glossary.md line
159 to include rack-management mode or explicitly state its effective dpu_policy
mapping; keep both descriptions consistent with the policy model.
---
Duplicate comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 63-67: Update the `manage` row in the JSON/TOML value table to
describe DPU lifecycle management without requiring a DPU/Admin boot path. State
that DPU/Admin boot is the default only when no plain-NIC primary interface is
declared, while preserving the documented behavior for managed DPUs with an
integrated NIC as the HostInband primary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e93fec0a-c3d0-42b3-a1ae-33737f888e76
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
|
@coderabbitai full_review, thanks! |
|
(=^・ω・^=) ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/development/new_hardware_support.md`:
- Line 221: The benchmark example below the boot-MAC guidance must match the
documented requirement. Update that benchmark command to include --boot-mac with
the host boot interface MAC placeholder, unless the example is explicitly
restricted to platforms where selecting a boot interface is optional; keep the
repeated exploration behavior unchanged.
In
`@docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md`:
- Around line 117-122: Update the --dpu-policy documentation in
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md
(lines 117-122) and
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md
(lines 123-128) to explicitly map legacy --dpu-mode values: dpu-mode to manage,
nic-mode to use-as-nic, and no-dpu to ignore.
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Line 288: Update both AwaitingNic descriptions in the boot interface and DPU
modes documentation to replace the awkward “an ignore/use_as_nic host” phrasing
with “a host using ignore or use_as_nic,” while preserving the surrounding
explanation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0317193f-5043-4497-bf96-b2cb60740a4b
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/provisioning/boot-interfaces-and-dpu-modes.md (1)
70-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
managerow still couples DPU management with DPU boot — regression of a previously fixed issue.Line 72 states unconditionally that a
managehost "boots through its primary DPU on the Admin overlay," but Section 3.4 (lines 171-186) explicitly documents amanagehost that boots from an integrated NIC on HostInband instead. This is the exact wording previously flagged in review and marked "✅ Addressed in commits da30d63 to 421d8d3" — the fix (clarifying that DPU/Admin boot is only the default when no plain-NIC primary is declared) does not appear to be present in this row.♻️ Restore the previously-agreed wording
-| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo and the host boots through its primary DPU on the Admin overlay. A per-host `manage` declaration inherits the site policy rather than overriding it. | +| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo. The boot interface is resolved independently; when no plain-NIC primary is declared, the default is the DPU on the Admin overlay. A per-host `manage` declaration inherits the site policy rather than overriding it. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/provisioning/boot-interfaces-and-dpu-modes.md` around lines 70 - 73, Update the `manage` row in the JSON/TOML value table so DPU/Admin-overlay boot is described only as the default when no plain-NIC primary is declared; preserve that a `manage` host may instead boot through an integrated NIC on HostInband. Keep the existing policy-inheritance wording and align the row with the behavior documented in Section 3.4.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 70-73: Update the `manage` row in the JSON/TOML value table so
DPU/Admin-overlay boot is described only as the default when no plain-NIC
primary is declared; preserve that a `manage` host may instead boot through an
integrated NIC on HostInband. Keep the existing policy-inheritance wording and
align the row with the behavior documented in Section 3.4.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dc7e5a44-4a24-43e5-9fbe-607bcd12de44
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/observability/core_metrics.md
- docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md
- docs/index.yml
- docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md
- docs/provisioning/ingesting-hosts.md
- docs/development/new_hardware_support.md
- docs/glossary.md
- docs/manuals/vpc/flat_vpcs_zero_dpu.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/provisioning/boot-interfaces-and-dpu-modes.md (1)
70-74:⚠️ Potential issue | 🟠 MajorDo not make
manageimply DPU/Admin boot.The
managepolicy controls DPU lifecycle, not boot-interface selection. These lines contradict the independent-axis model and Scenario 3.4, where managed DPUs coexist with an integrated primary NIC on HostInband. Qualify DPU/Admin boot as the default only when no plain-NIC primary is declared.As per path instructions, Markdown under
docs/**must remain technically consistent with the documented operator workflow.Suggested wording
-| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo and the host boots through its primary DPU on the Admin overlay. A per-host `manage` declaration inherits the site policy rather than overriding it. | +| `manage` (site/effective default) | `manage` | As a site or resolved policy, DPUs are managed by NICo. Boot follows the host's primary interface; when no plain-NIC primary is declared, the default is the DPU on the Admin overlay. A per-host `manage` declaration inherits the site policy rather than overriding it. |Also applies to: 141-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/provisioning/boot-interfaces-and-dpu-modes.md` around lines 70 - 74, Update the `manage` row in the policy table so it describes DPU lifecycle management without making DPU/Admin boot mandatory; state that DPU/Admin is the default only when no plain-NIC primary is declared, while preserving HostInband for hosts with an integrated primary NIC. Apply the same correction to the additional occurrence referenced by the review.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 70-74: Update the `manage` row in the policy table so it describes
DPU lifecycle management without making DPU/Admin boot mandatory; state that
DPU/Admin is the default only when no plain-NIC primary is declared, while
preserving HostInband for hosts with an integrated primary NIC. Apply the same
correction to the additional occurrence referenced by the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8a70a241-3a8c-4bf6-a424-7d868e89794a
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
eb59f0d to
f0915f1
Compare
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Around line 53-56: Update the default-configuration statement to explicitly
limit the manage/primary-DPU boot behavior to hosts with DPU hardware, and
revise the corresponding manage row in section 3.2 to match. Ensure zero-DPU
hosts are directed to use dpu_policy ignore with a primary HostInband NIC,
preserving technically accurate operator guidance.
- Around line 288-289: Update the AwaitingNic/Missing troubleshooting guidance
in the boot-interface and DPU modes documentation to explicitly cover
managed-DPU hosts booting through an integrated HostInband NIC. Classify this
topology as AwaitingNic while the integrated NIC has not appeared, or explicitly
state that it remains Missing until the NIC appears, and apply the same
clarification to the related guidance near section 6.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 373ede48-c819-4798-ab59-58e88c6be7a9
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md`:
- Around line 117-123: Update the --dpu-policy documentation in
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md:117-123
and
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md:123-129
so manage explicitly means the host is managed by NICo, while an omitted option
is documented separately as deferring to the site default on add and preserving
the existing per-host value on patch. Keep the existing use-as-nic, ignore, and
legacy --dpu-mode mappings accurate.
- Around line 117-132: Align the expected-machine add and patch manuals with the
shipped CLI by documenting --dpu-mode and its dpu-mode, nic-mode, and no-dpu
values instead of --dpu-policy and manage, use-as-nic, and ignore. Update the
option descriptions and every related example at
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md:19,
117-132, 165 and
docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md:20,
123-138, 171; no CLI change is requested.
In `@docs/manuals/vpc/flat_vpcs_zero_dpu.md`:
- Line 74: Clarify the existing-host application behavior in the table entry for
ExpectedMachine.dpu_policy: distinguish updating the policy with nico-admin-cli
em patch, which requires no API restart, from applying it to an already-ingested
host, which requires force-deletion and re-ingestion. Keep the instructions
consistent with the boot-interfaces-and-dpu-modes provisioning guidance.
In `@docs/provisioning/boot-interfaces-and-dpu-modes.md`:
- Line 59: Update the Section 3.2 Markdown link in the surrounding documentation
text to use the heading-generated fragment `#32-zero-dpu-host-no-dpu-hardware`
instead of `#32-boot-a-host-with-no-dpus`, leaving the link text and surrounding
content unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c2931cf8-50c5-4ccd-a9ff-d4351826e296
📒 Files selected for processing (10)
crates/site-explorer/src/metrics.rsdocs/development/new_hardware_support.mddocs/glossary.mddocs/index.ymldocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.mddocs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.mddocs/manuals/vpc/flat_vpcs_zero_dpu.mddocs/observability/core_metrics.mddocs/provisioning/boot-interfaces-and-dpu-modes.mddocs/provisioning/ingesting-hosts.md
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
The provisioning, ingestion, Flat VPC, glossary, metrics, and admin CLI docs now use `HostDpuPolicy::{Manage, UseAsNic, Ignore}` for desired behavior and `BlueFieldOperatingMode::{Dpu, Nic}` for observed hardware state.
Existing `dpu_mode` fields, flags, values, and protobuf/Redfish names are called out as backwards-compatible boundaries rather than the canonical vocabulary.
This supports NVIDIA#3472
Signed-off-by: Chet Nichols III <chetn@nvidia.com>
|
@coderabbitai full_review, thanks! |
|
✅ Action performedFull review finished. |
The provisioning, ingestion, Flat VPC, glossary, metrics, and admin CLI docs now use
HostDpuPolicy::{Manage, UseAsNic, Ignore}for desired behavior andBlueFieldOperatingMode::{Dpu, Nic}for observed hardware state.Existing
dpu_modefields, flags, values, and protobuf/Redfish names are called out as backwards-compatible boundaries rather than the canonical vocabulary.This is the separately reviewed documentation companion to #3453 and should merge after that code PR.
Validation:
cargo make format-nightlycargo make clippycargo make carbide-lintsgit diff --checkThis supports #3472