feat(site-explorer): create power shelves by default#3276
Merged
Conversation
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
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 (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Summary by CodeRabbit
WalkthroughSite Explorer defaults now enable machine, switch, and power-shelf creation plus static-IP power-shelf discovery. Exploration mode, equality checks, configuration tests, documentation, skip logging, and power-shelf latency documentation were updated. ChangesSite Explorer configuration and observability
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
🐇✨ ✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 4 minutes. |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
chet
changed the title
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
✅ Action performedFull review finished. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3276.docs.buildwithfern.com/infra-controller |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@book/src/configuration/configurability.md`:
- Line 1192: Update the quick-reference table entry for site explorer settings
to include the site-wide `explore_power_shelves_from_static_ip` option, or add a
dedicated row distinguishing static-IP discovery from switch/power-shelf
creation; use the exact repository configuration terminology and document its
behavior and default consistently.
In `@crates/api-core/src/cfg/README.md`:
- Line 306: The README description for `explore_power_shelves_from_static_ip`
should explicitly state that discovery uses the static IPs declared in
`expected_power_shelves`, rather than discovering arbitrary power shelves.
Update the option’s description to document this configuration prerequisite
accurately.
🪄 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: 6cea4b35-9eaf-4fb2-9178-58e491750cfb
📒 Files selected for processing (6)
book/src/configuration/configurability.mdcrates/api-core/src/cfg/README.mdcrates/api-core/src/cfg/file.rscrates/site-explorer/src/config.rscrates/site-explorer/src/lib.rscrates/site-explorer/src/switch_creator.rs
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
(ᵔᴥᵔ) ✅ Action performedFull review finished. |
nick-vetro
approved these changes
Jul 10, 2026
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
✅ Action performedFull review finished. |
Contributor
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 `@crates/site-explorer/src/lib.rs`:
- Around line 890-895: Update the skip log in the exploration handling branch to
mention both prerequisites: set [site_explorer] create_power_shelves=true and
provide matching expected_power_shelves records. Adjust the tracing::info!
message so operators understand that shelves are also skipped when no
expected-hardware record exists.
🪄 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: b75a2916-a42f-4669-8739-31027973c1c0
📒 Files selected for processing (7)
book/src/configuration/configurability.mdcrates/api-core/src/cfg/README.mdcrates/api-core/src/cfg/file.rscrates/site-explorer/src/config.rscrates/site-explorer/src/lib.rscrates/site-explorer/src/switch_creator.rsdocs/observability/core_metrics.md
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
✅ Action performedFull review finished. |
polarweasel
requested changes
Jul 11, 2026
| plus thresholds for DPU agent compliance. Operators flip this from | ||
| `MonitorOnly` to `Enforce` once their fleet's baseline is clean. | ||
|
|
||
| ### Site Explorer |
Contributor
There was a problem hiding this comment.
This whole section (and doubtless a bunch of others around the docs) could be a bulleted list, or dictionary term entries. If you want to do that, the ParamField component is a fun one to play with: https://buildwithfern.com/learn/docs/writing-content/components/parameter-fields
| | SPDM | siteConfig | `[spdm].enabled` | off | Hardware attestation via NRAS. | | ||
| | Rack Management | siteConfig | `rack_management_enabled = true` | off | Standalone infrastructure manager mode (GB200/GB300/VR144). | | ||
| | Site Explorer machine auto-creation | siteConfig | `[site_explorer].create_machines` | on | Disable for manual-onboarding environments. | | ||
| | Site Explorer switch / power shelf auto-creation | siteConfig | `[site_explorer].create_switches` / `[site_explorer].create_power_shelves` | on | Ingests only declared hardware (`expected_switches` / `expected_power_shelves` records). Disable to hold switch or power shelf ingestion back site-wide. | |
Contributor
There was a problem hiding this comment.
Either use "pause" here, or "hold back", but don't split "hold back" if you use it.
Suggested change
| | Site Explorer switch / power shelf auto-creation | siteConfig | `[site_explorer].create_switches` / `[site_explorer].create_power_shelves` | on | Ingests only declared hardware (`expected_switches` / `expected_power_shelves` records). Disable to hold switch or power shelf ingestion back site-wide. | | |
| | Site Explorer switch / power shelf auto-creation | siteConfig | `[site_explorer].create_switches` / `[site_explorer].create_power_shelves` | on | Ingests only declared hardware (`expected_switches` / `expected_power_shelves` records). Disable to pause switch or power shelf ingestion site-wide. | |
Completes what NVIDIA#3280 started for switches: explored power shelves that match an `expected_power_shelves` record are now ingested out of the box too -- `create_power_shelves` and `explore_power_shelves_from_static_ip` default to true instead of false. The expected-hardware records remain the per-device gate, so these defaults only ever ingest hardware an operator has declared, and the flags stay available as site-wide kill switches. Primary callouts are: - The power shelf creation defaults flip to true, matching `create_machines` (true all along) and `create_switches` (true since NVIDIA#3280). - The two default paths now agree by construction: `Default::default()` delegates to the serde default fns, and `SiteExplorerConfig`'s `PartialEq` compares every field, destructuring `Self` without `..` so a future field is a compile error rather than a silent comparison gap. Previously a config with no `[site_explorer]` section at all ran with these flags on while a config with the section got false -- and the parity test couldn't see it because `PartialEq` skipped the creation flags. - The now-exhaustive comparison made three full-config fixture tests assert their `site_explorer` fields for real, surfacing stale values that predate the `nv-redfish` default (`explore_mode: LibRedfish`, one old `machines_created_per_run`); the fixtures now match the parsed config. - Skipped-creation visibility rounds out: the power shelf skip log mirrors the switch one from NVIDIA#3280 (`num_power_shelves`, same actionable wording), both skip logs now name the full prerequisite (the creation flag plus a matching expected-hardware record), and an explored switch with no `expected_switches` record now logs per endpoint instead of silently vanishing (power shelves and machines already logged this case). - Docs follow: the configurability book page records all three creation defaults and the expected-record gating, and the config reference README gets fuller row descriptions plus a stale `explore_mode` default fix (it said `libredfish`; the code default is `nv-redfish`). The auto-generated metrics reference gains `site_explorer_create_power_shelves_latency_seconds`, which the now-default-on shelf creation phase emits. Upgrade note: a site that declares `expected_power_shelves` records but never set the creation flags starts ingesting those shelves on upgrade (switches got this behavior in NVIDIA#3280). Setting the flags to false keeps ingestion off, exactly as before. Tests added! This supports NVIDIA#3152 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
polarweasel
approved these changes
Jul 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes what #3280 started for switches: explored power shelves that match an
expected_power_shelvesrecord are now ingested out of the box too --create_power_shelvesandexplore_power_shelves_from_static_ipdefault to true instead of false. The expected-hardware records remain the per-device gate, so these defaults only ever ingest hardware an operator has declared, and the flags stay available as site-wide kill switches.Primary callouts are:
create_machines(true all along) andcreate_switches(true since feat(site-explorer): default create_switches to true and log when skipping #3280).Default::default()delegates to the serde default fns, andSiteExplorerConfig'sPartialEqcompares every field, destructuringSelfwithout..so a future field is a compile error rather than a silent comparison gap. Previously a config with no[site_explorer]section at all ran with these flags on while a config with the section got false -- and the parity test couldn't see it becausePartialEqskipped the creation flags.site_explorerfields for real, surfacing stale values that predate thenv-redfishdefault (explore_mode: LibRedfish, one oldmachines_created_per_run); the fixtures now match the parsed config.num_power_shelves, same actionable wording), both skip logs now name the full prerequisite (the creation flag plus a matching expected-hardware record), and an explored switch with noexpected_switchesrecord now logs per endpoint instead of silently vanishing (power shelves and machines already logged this case).explore_modedefault fix (it saidlibredfish; the code default isnv-redfish). The auto-generated metrics reference gainssite_explorer_create_power_shelves_latency_seconds, which the now-default-on shelf creation phase emits.Upgrade note: a site that declares
expected_power_shelvesrecords but never set the creation flags starts ingesting those shelves on upgrade (switches got this behavior in #3280). Setting the flags to false keeps ingestion off, exactly as before.Tests added!
This supports #3152
Closes #3152