Skip to content

docs: clarify retained address lifetime - #4842

Merged
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-4836
Aug 11, 2026
Merged

docs: clarify retained address lifetime#4842
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-4836

Conversation

@chet

@chet chet commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

A Retained BMC address becomes Static for the lifetime of its machine-interface record. Calling it non-expiring made it sound like the address was saved for reuse after that interface was deleted and re-ingested, which it is not.

So, this updates the nico-admin-cli expected-machine {add,patch} help, the model Rustdoc, and the protobuf comments to name the actual lifetime. The checked-in REST protobuf mirrors are regenerated from the updated source. CLI arguments, protobuf wire values, and allocation behavior all stay the same.

Related issues

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

  • This PR contains breaking changes

Testing

  • Unit tests added/updated

  • Integration tests added/updated

  • Manual testing performed

  • No testing required (docs, internal refactor, etc.)

  • cargo test -p nico-admin-cli (427 passed)

  • Rendered and inspected nico-admin-cli expected-machine add --help

  • Rendered and inspected nico-admin-cli expected-machine patch --help

  • Regenerated the REST Core protobuf snapshots and Go bindings, then verified the worktree stayed clean

  • cargo make format-nightly

  • cargo make clippy

  • Cached Carbide-lints workflow

Additional Notes

The generated nico-admin-cli reference under docs/ stays in the separate docs-only #4151 / #4418 so the code and documentation review paths remain independent.

A Retained address becomes Static for the lifetime of its machine-interface record. Calling it non-expiring made it sound like we saved the address for reuse after that interface was deleted and re-ingested, which we do not.

So, update the CLI help, model Rustdoc, and protobuf comments to name the actual lifetime. The CLI arguments, protobuf wire format, and allocation behavior stay the same.

This supports NVIDIA#4836

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet
chet requested a review from a team as a code owner August 11, 2026 21:25
@coderabbitai

coderabbitai Bot commented Aug 11, 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: 21d13b66-053b-4194-a1aa-f29b90ffe0c4

📥 Commits

Reviewing files that changed from the base of the PR and between 8076fa9 and f491260.

⛔ Files ignored due to path filters (1)
  • rest-api/proto/core/gen/v1/nico_nico.pb.go is excluded by !**/*.pb.go, !**/gen/**, !rest-api/**/*.pb.go
📒 Files selected for processing (5)
  • crates/admin-cli/src/expected_machines/add/args.rs
  • crates/admin-cli/src/expected_machines/patch/args.rs
  • crates/api-model/src/expected_machine.rs
  • crates/rpc/proto/forge.proto
  • rest-api/proto/core/src/v1/nico_nico.proto

Summary by CodeRabbit

  • Documentation
    • Clarified that retained BMC IP addresses remain static for the lifetime of the associated machine-interface record.
    • Updated the relevant CLI help text and API documentation for consistent wording.

Walkthrough

The pull request updates retained BMC IP allocation documentation in the admin CLI, API model, and protobuf definitions. The text now limits address retention to the lifetime of the machine-interface record.

Changes

Retained BMC IP documentation

Layer / File(s) Summary
Update retained BMC IP descriptions
crates/admin-cli/src/expected_machines/..., crates/api-model/src/expected_machine.rs, crates/rpc/proto/forge.proto, rest-api/proto/core/src/v1/nico_nico.proto
The documentation now describes retained BMC addresses as auto-allocated DHCP addresses that remain static for the lifetime of the machine-interface record.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

  • NVIDIA/infra-controller issue 4836 — The changes directly implement its objective to clarify the lifetime of retained BMC IP allocation.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the documentation clarification for retained address lifetime.
Description check ✅ Passed The description directly explains the documentation changes, rationale, affected areas, and validation performed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@chet

chet commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full_review, thanks!

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@chet I will perform a complete review of the pull request.

✅ Action performed

Full review finished.

@github-actions

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-08-11 21:28:18 UTC | Commit: f491260

@chet
chet enabled auto-merge (squash) August 11, 2026 22:11
@chet
chet merged commit 1612bdb into NVIDIA:main Aug 11, 2026
121 checks passed
@chet
chet deleted the gh-issue-4836 branch August 12, 2026 23:46
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