docs(admin-cli): document selected boot interface commands - #4229
Conversation
Summary by CodeRabbit
WalkthroughAdds documentation for ChangesCLI documentation updates
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full_review, thanks! |
|
🐇🔍 ✅ Action performedFull review finished. |
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/manuals/nico-admin-cli/commands/boot-interface/boot-interface.md`:
- Around line 23-25: Correct the repeated measured-boot sentence in the
documentation generator’s source/template from “This used by measured boot” to
“This is used by measured boot,” then regenerate the affected Markdown pages.
Apply and verify the correction in
docs/manuals/nico-admin-cli/commands/boot-interface/boot-interface.md#L23-L25,
boot-interface-show.md#L31-L33, boot-interface-candidates.md#L34-L36,
boot-interface-set.md#L44-L46, and
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md#L33-L35,
reviewing the generated Markdown for spelling and grammar.
🪄 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: 87a7ba94-efa1-4d1e-baf4-69b9861b892c
📒 Files selected for processing (8)
docs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-candidates.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-show.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host.mddocs/manuals/nico-admin-cli/hardware.md
The selected boot-interface workflow commits the primary interface and desired target together. `machine-controller` then reconciles Redfish when the host is eligible. This adds the generated pages for `boot-interface show`, `candidates`, and `set`, plus the `managed-host` compatibility commands. The reference covers reconciliation progress, `--force-reconcile`, and the deprecated `--reboot` alias. It also distinguishes the lowest-MAC managed fallback from the pre-first-lease prediction rule, which refuses to guess among multiple predictions without a declared primary. This supports NVIDIA#4227 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
c43e113 to
2ad2628
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-4229.docs.buildwithfern.com/infra-controller |
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/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.md`:
- Around line 32-34: Update the `--reboot` option documentation to state that it
sets both `reboot: true` and `force_reconcile: true`, while `--force-reconcile`
only enables reconciliation and does not request a reboot; retain the note that
`--reboot` is the compatibility path for older servers.
In
`@docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md`:
- Around line 7-19: Update the shared Clap source for the deprecated
set-primary-dpu command so its migration notice links to the
set-primary-interface documentation and explicitly states that the replacement
requires an INTERFACE_ID (machine-interface ID), not a DPU_MACHINE_ID.
Regenerate the managed-host documentation page from that source rather than
editing only the generated Markdown.
In
`@docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md`:
- Around line 27-29: Clarify the --reboot compatibility contract in the Clap
help definitions for all three commands: managed-host-set-primary-interface,
managed-host-set-primary-dpu, and boot-interface-set. State that --reboot sets
both reboot=true and force_reconcile=true, while --force-reconcile sets only
force_reconcile, and that neither flag directly restarts hardware; replace “with
current servers” with the authoritative support boundary or compatibility
reference. Regenerate managed-host-set-primary-interface.md,
managed-host-set-primary-dpu.md, and boot-interface-set.md with the updated help
text.
🪄 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: 6bd1e07b-8ad8-460b-93a4-7d8619662642
📒 Files selected for processing (9)
crates/admin-cli/src/boot_interface/mod.rsdocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-candidates.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-show.mddocs/manuals/nico-admin-cli/commands/boot-interface/boot-interface.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.mddocs/manuals/nico-admin-cli/commands/managed-host/managed-host.mddocs/manuals/nico-admin-cli/hardware.md
| **--reboot** | ||
| Deprecated compatibility alias; use --force-reconcile with current | ||
| servers |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline crates/admin-cli/src/boot_interface --items all --type struct,enum,function
rg -n -C 8 '\b(force_reconcile|reboot)\b' crates/admin-cli/src/boot_interfaceRepository: NVIDIA/infra-controller
Length of output: 5949
Document the distinct --reboot behavior.
--reboot sets both reboot: true and force_reconcile: true; --force-reconcile does not request a reboot. State this compatibility behavior and retain the older-server boundary.
🤖 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/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.md`
around lines 32 - 34, Update the `--reboot` option documentation to state that
it sets both `reboot: true` and `force_reconcile: true`, while
`--force-reconcile` only enables reconciliation and does not request a reboot;
retain the note that `--reboot` is the compatibility path for older servers.
Source: Path instructions
| nico-admin-cli-managed-host-set-primary-dpu - Deprecated: use | ||
| set-primary-interface. Sets the primary DPU. | ||
|
|
||
| ## SYNOPSIS | ||
|
|
||
| **nico-admin-cli managed-host set-primary-dpu** \[**--reboot**\] | ||
| \[**--extended**\] \[**--sort-by**\] \[**-h**\|**--help**\] | ||
| \<*HOST_MACHINE_ID*\> \<*DPU_MACHINE_ID*\> | ||
| **nico-admin-cli managed-host set-primary-dpu** | ||
| \[**--force-reconcile**\] \[**--reboot**\] \[**--extended**\] | ||
| \[**--sort-by**\] \[**-h**\|**--help**\] \<*HOST_MACHINE_ID*\> | ||
| \<*DPU_MACHINE_ID*\> | ||
|
|
||
| ## DESCRIPTION | ||
|
|
||
| Set the primary DPU for the managed host | ||
| Deprecated: use set-primary-interface. Sets the primary DPU. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the deprecated-command migration explicit.
set-primary-dpu accepts a DPU_MACHINE_ID, while set-primary-interface accepts an INTERFACE_ID. The current notice only names the replacement and does not explain this target change. Link the replacement page and state that it requires a machine-interface ID.
As per path instructions, documentation must be technically correct and operator-usable. Based on learnings, generated CLI wording should be updated in the shared Clap source and regenerated instead of being edited only in the generated page.
Proposed wording
-Deprecated: use set-primary-interface. Sets the primary DPU.
+Deprecated: use [`set-primary-interface`](./managed-host-set-primary-interface.md) with an `INTERFACE_ID`. This command accepts a `DPU_MACHINE_ID`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| nico-admin-cli-managed-host-set-primary-dpu - Deprecated: use | |
| set-primary-interface. Sets the primary DPU. | |
| ## SYNOPSIS | |
| **nico-admin-cli managed-host set-primary-dpu** \[**--reboot**\] | |
| \[**--extended**\] \[**--sort-by**\] \[**-h**\|**--help**\] | |
| \<*HOST_MACHINE_ID*\> \<*DPU_MACHINE_ID*\> | |
| **nico-admin-cli managed-host set-primary-dpu** | |
| \[**--force-reconcile**\] \[**--reboot**\] \[**--extended**\] | |
| \[**--sort-by**\] \[**-h**\|**--help**\] \<*HOST_MACHINE_ID*\> | |
| \<*DPU_MACHINE_ID*\> | |
| ## DESCRIPTION | |
| Set the primary DPU for the managed host | |
| Deprecated: use set-primary-interface. Sets the primary DPU. | |
| nico-admin-cli-managed-host-set-primary-dpu - Deprecated: use [`set-primary-interface`](./managed-host-set-primary-interface.md) with an `INTERFACE_ID`. This command accepts a `DPU_MACHINE_ID`. | |
| ## SYNOPSIS | |
| **nico-admin-cli managed-host set-primary-dpu** | |
| \[**--force-reconcile**\] \[**--reboot**\] \[**--extended**\] | |
| \[**--sort-by**\] \[**-h**\|**--help**\] \<*HOST_MACHINE_ID*\> | |
| \<*DPU_MACHINE_ID*\> | |
| ## DESCRIPTION | |
| Deprecated: use [`set-primary-interface`](./managed-host-set-primary-interface.md) with an `INTERFACE_ID`. This command accepts a `DPU_MACHINE_ID`. |
🤖 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/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md`
around lines 7 - 19, Update the shared Clap source for the deprecated
set-primary-dpu command so its migration notice links to the
set-primary-interface documentation and explicitly states that the replacement
requires an INTERFACE_ID (machine-interface ID), not a DPU_MACHINE_ID.
Regenerate the managed-host documentation page from that source rather than
editing only the generated Markdown.
Sources: Path instructions, Learnings
| **--reboot** | ||
| Deprecated compatibility alias; use --force-reconcile with current | ||
| servers |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- targeted documentation files ---'
for f in \
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md \
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md
do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
sed -n '1,100p' "$f"
else
printf 'MISSING: %s\n' "$f"
fi
done
printf '%s\n' '--- occurrences of relevant options and request fields ---'
rg -n -S --glob '!target/**' --glob '!node_modules/**' \
'force-reconcile|force_reconcile|--reboot|reboot|SetPrimaryDpuRequest|SetPrimary.*InterfaceRequest' .
printf '%s\n' '--- likely Clap/help source files ---'
git ls-files | rg -i '(managed.?host|primary|clap|cli|command).*\.(rs|md|yml|yaml)$' | head -200Repository: NVIDIA/infra-controller
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- documentation pages ---'
for f in \
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md \
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md
do
printf '\n### %s\n' "$f"
sed -n '1,80p' "$f"
done
printf '%s\n' '--- source and test matches ---'
rg -n -S \
--glob '*.rs' \
--glob '!target/**' \
--glob '!docs/**' \
--glob '!**/generated/**' \
--glob '!**/fixtures/**' \
'force_reconcile|force-reconcile|SetPrimaryDpuRequest|SetPrimaryInterfaceRequest|reboot' \
. | head -400
printf '%s\n' '--- documentation generation metadata ---'
rg -n -S \
--glob '*.rs' --glob '*.toml' --glob '*.yml' --glob '*.yaml' --glob '*.md' \
--glob '!target/**' \
'managed-host-set-primary-interface|managed-host-set-primary-dpu|nico-admin-cli|generate.*doc|clap' \
. | head -300
printf '%s\n' '--- candidate source files ---'
git ls-files '*.rs' | rg -i '(cli|command|managed.?host|primary|request)' | head -250Repository: NVIDIA/infra-controller
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- tracked files related to the CLI and commands ---'
git ls-files | rg -i \
'(^|/)(nico|.*cli.*|.*command.*|managed.?host|primary|admin)(/|[-_.]).*\.(rs|toml|yaml|yml|md)$|nico-admin-cli|managed-host-set-primary'
printf '%s\n' '--- files containing exact option or field names ---'
for term in force_reconcile force-reconcile SetPrimaryDpuRequest SetPrimaryInterfaceRequest; do
printf '\n### %s\n' "$term"
git grep -l -F "$term" -- ':!docs' ':!target' ':!**/generated/**' | head -100 || true
done
printf '%s\n' '--- all tracked files containing the command names ---'
git grep -l -E 'set-primary-interface|set-primary-dpu' -- ':!target' | head -100 || trueRepository: NVIDIA/infra-controller
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -eu
files=(
crates/admin-cli/src/managed_host/set_primary_interface/args.rs
crates/admin-cli/src/managed_host/set_primary_interface/cmd.rs
crates/admin-cli/src/managed_host/set_primary_interface/mod.rs
crates/admin-cli/src/managed_host/set_primary_dpu/args.rs
crates/admin-cli/src/managed_host/set_primary_dpu/cmd.rs
crates/admin-cli/src/managed_host/set_primary_dpu/mod.rs
crates/admin-cli/src/managed_host/tests.rs
crates/admin-cli/build.rs
crates/admin-cli/cli_domains.yaml
crates/admin-cli/src/generate_docs/cmd.rs
)
for f in "${files[@]}"; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
wc -l "$f"
cat -n "$f" | sed -n '1,260p'
else
printf '\nMISSING: %s\n' "$f"
fi
done
printf '\n--- exact option references in admin-cli ---\n'
rg -n -S \
'force_reconcile|force-reconcile|SetPrimaryDpuRequest|SetPrimaryInterfaceRequest|reboot' \
crates/admin-cli/src/managed_host crates/admin-cli/src/generate_docs crates/admin-cli/build.rs crates/admin-cli/cli_domains.yamlRepository: NVIDIA/infra-controller
Length of output: 41734
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- reconciliation tests ---'
sed -n '235,315p' crates/admin-cli/src/managed_host/tests.rs
printf '%s\n' '--- request type and API metadata references ---'
git grep -n -E \
'SetPrimary(Dpu|Interface)Request|force[_-]?reconcile|forceReconcile|reboot.*deprecated|deprecated.*reboot|reboot.*compat' \
-- \
':!target' \
':!docs/manuals/nico-admin-cli/**' \
':!**/generated/**' \
':!**/*.svg' \
':!**/*.html' | head -500
printf '%s\n' '--- likely RPC and protocol files ---'
git ls-files | rg -i \
'(^|/)(rpc|proto|openapi|api|forge|carbide).*\.(rs|proto|yaml|yml|json)$|Cargo.toml$' | head -400
printf '%s\n' '--- dependency declarations for RPC crates ---'
rg -n -S \
'rpc|forge|carbide-rpc|carbide_rpc' \
crates/admin-cli/Cargo.toml Cargo.toml Cargo.lockRepository: NVIDIA/infra-controller
Length of output: 40718
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- boot-interface implementation and documentation ---'
for f in \
crates/admin-cli/src/boot_interface/set/args.rs \
crates/admin-cli/src/boot_interface/set/cmd.rs \
docs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.md
do
printf '\n### %s\n' "$f"
if [ -f "$f" ]; then
cat -n "$f" | sed -n '1,180p'
else
printf 'MISSING\n'
fi
done
printf '%s\n' '--- protocol request definitions ---'
sed -n '8525,8575p' crates/rpc/proto/forge.proto
sed -n '8542,8575p' rest-api/proto/core/src/v1/nico_nico.proto
printf '%s\n' '--- server handler ---'
sed -n '45,120p' crates/api-core/src/handlers/managed_host.rs
printf '%s\n' '--- server compatibility tests ---'
for f in \
crates/api-core/src/tests/set_primary_interface.rs \
crates/api-core/src/tests/set_primary_dpu.rs
do
printf '\n### %s\n' "$f"
rg -n -B 12 -A 35 \
'compatibility|deprecated reboot|reboot alias|force_reconcile|reboot' "$f" | head -300
doneRepository: NVIDIA/infra-controller
Length of output: 37710
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
cases = [
(
Path("crates/admin-cli/src/managed_host/set_primary_interface/args.rs"),
Path("docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md"),
),
(
Path("crates/admin-cli/src/managed_host/set_primary_dpu/args.rs"),
Path("docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md"),
),
(
Path("crates/admin-cli/src/boot_interface/set/args.rs"),
Path("docs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.md"),
),
]
for source, doc in cases:
src = source.read_text()
md = doc.read_text()
assert 'long,\n help = "Deprecated compatibility alias; use --force-reconcile with current servers"' in src
assert re.search(r"reboot:\s*args\.reboot,\s*\n\s*force_reconcile:\s*args\.force_reconcile \|\| args\.reboot", src)
assert "Deprecated compatibility alias; use --force-reconcile with current\nservers" in md
print(f"{source}: reboot -> reboot=true and force_reconcile=true; force-only leaves reboot=false")
print(f"{doc}: contains the same compatibility wording")
PYRepository: NVIDIA/infra-controller
Length of output: 784
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
cases = [
(
Path("crates/admin-cli/src/managed_host/set_primary_interface/args.rs"),
Path("docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md"),
),
(
Path("crates/admin-cli/src/managed_host/set_primary_dpu/args.rs"),
Path("docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md"),
),
(
Path("crates/admin-cli/src/boot_interface/set/cmd.rs"),
Path("docs/manuals/nico-admin-cli/commands/boot-interface/boot-interface-set.md"),
),
]
for source, doc in cases:
src = source.read_text()
md = doc.read_text()
assert "Deprecated compatibility alias; use --force-reconcile with current servers" in src or \
"Deprecated compatibility alias; use --force-reconcile with current\nservers" in md
assert re.search(r"reboot:\s*args\.reboot,\s*\n\s*force_reconcile:\s*args\.force_reconcile \|\| args\.reboot", src)
assert "Deprecated compatibility alias; use --force-reconcile with current\nservers" in md
print(f"{source}: reboot -> reboot=true and force_reconcile=true; force-only leaves reboot=false")
print(f"{doc}: contains the same compatibility wording")
PYRepository: NVIDIA/infra-controller
Length of output: 937
Clarify the --reboot compatibility contract.
In all three commands, --reboot sets reboot=true and force_reconcile=true; --force-reconcile sets only force_reconcile. Neither flag directly restarts hardware. Replace “with current servers” with a concrete support boundary or authoritative compatibility reference. Update the three Clap help definitions and regenerate the affected pages: managed-host-set-primary-interface.md, managed-host-set-primary-dpu.md, and boot-interface-set.md.
📍 Affects 2 files
docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md#L27-L29(this comment)docs/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-dpu.md#L27-L29
🤖 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/manuals/nico-admin-cli/commands/managed-host/managed-host-set-primary-interface.md`
around lines 27 - 29, Clarify the --reboot compatibility contract in the Clap
help definitions for all three commands: managed-host-set-primary-interface,
managed-host-set-primary-dpu, and boot-interface-set. State that --reboot sets
both reboot=true and force_reconcile=true, while --force-reconcile sets only
force_reconcile, and that neither flag directly restarts hardware; replace “with
current servers” with the authoritative support boundary or compatibility
reference. Regenerate managed-host-set-primary-interface.md,
managed-host-set-primary-dpu.md, and boot-interface-set.md with the updated help
text.
Sources: Coding guidelines, Path instructions, Learnings
The selected boot-interface workflow now commits the primary interface and its
desired Redfish target together.
machine-controllerthen reconciles thatpersisted intent when the host is eligible, while
boot-interface showreportsthe effective interface and reconciliation progress.
This adds the generated references for
boot-interface show,candidates, andset, plusmanaged-host set-primary-interface. The pages document the managedand pre-first-lease selection rules,
--force-reconcilefor requesting a freshcontroller pass, and
--rebootas a deprecated compatibility alias. Theaffected Hardware and
managed-hostindexes and the deprecatedset-primary-dpupage are refreshed too.Related issues
This supports #4227.
This documents #4193.
Type of Change
Add - New feature or capability
Change - Changes in existing functionality
Fix - Bug fixes
Remove - Removed features or deprecated functionality
Internal - Internal changes (refactoring, tests, docs, etc.)
Breaking Changes
Testing
Unit tests added/updated
Integration tests added/updated
Manual testing performed
No testing required (docs, internal refactor, etc.)
Additional Notes
The repository-wide CLI-doc generator currently rewrites unrelated pages from
current
main, so the generated diff stays limited to the eight pages for#4227. Their command content was checked against Pandoc 2.17.1.1 output—the
Debian Bookworm version used by the docs tooling—and all seven affected live
--helpsurfaces.cargo test -p nico-admin-cli cfg::cli_options::tests— 5 passed.cargo make format-nightlycargo make clippycargo make carbide-lints