feat(site-explorer): default create_switches to true and log when skipping by mehrdadmoradi-crusoe · Pull Request #3280 · NVIDIA/infra-controller

mehrdadmoradi-crusoe · 2026-07-08T20:52:48Z

Defaults [site_explorer] create_switches to true (matching create_machines) and adds an info-level log when switches are identified during exploration but skipped because create_switches = false. Also updates the config reference default in api-core.

Related issues

Implements #3152

Type of Change

Change - Changes in existing functionality

Breaking Changes

This PR contains breaking changes

Note: this changes a default — explored switches now auto-create by default. This was explicitly requested in #3152 by @ajf ("we should probably default to create_switches true") and @chet ("we can probably just change the default").

Testing

Manual testing performed
cargo check -p site-explorer passes.
With create_switches = true (now the default), explored NVOS switches ingest and appear in switch show. Previously the default false silently skipped Switch creation with no log line, while create_machines defaulted to true — the asymmetry is what made it look like a bug.
The new log fires once per explore cycle, only when switches were identified and creation was skipped.

Additional Notes

Per @ajf in #3152: "feel free to implement both things.. we should probably default to create_switches true as well as emit a log if it isn't creating them because of the config at info level." Context: evaluating NICo for DPU/bare-metal provisioning at Crusoe (NVIDIA Cloud Partner).

copy-pr-bot · 2026-07-08T20:52:52Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

coderabbitai · 2026-07-08T20:53:48Z

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: 4ce9afde-7963-47ad-8105-03972df83d67

📥 Commits

Reviewing files that changed from the base of the PR and between 728681e and a74a2a9.

📒 Files selected for processing (4)

crates/api-core/src/cfg/README.md
crates/site-explorer/src/config.rs
crates/site-explorer/src/lib.rs
docs/observability/core_metrics.md

✅ Files skipped from review due to trivial changes (2)

crates/api-core/src/cfg/README.md
docs/observability/core_metrics.md

🚧 Files skipped from review as they are similar to previous changes (2)

crates/site-explorer/src/lib.rs
crates/site-explorer/src/config.rs

Summary by CodeRabbit

New Features
- Site exploration now creates switch state machines by default when the switch-creation setting isn’t specified.
- Added an informational log when switches are discovered but switch creation is disabled.
Bug Fixes
- Updated configuration documentation to reflect the correct default for switch creation.
Documentation
- Added a new core metric: site_explorer_create_switches_latency_seconds (histogram, duration of switch creation).

Walkthrough

create_switches now defaults to true in SiteExplorerConfig. When switch discovery is skipped because it is false, explore_site emits an informational log. The configuration and observability documentation are updated to match.

Changes

Default create_switches Behavior Change

Layer / File(s)	Summary
Default value change and skip logging `crates/site-explorer/src/config.rs`, `crates/site-explorer/src/lib.rs`	`default_create_switches()` now returns `true`, and `explore_site` logs an info message when switches are discovered but skipped due to `create_switches=false`.
Documentation updates `crates/api-core/src/cfg/README.md`, `docs/observability/core_metrics.md`	The `create_switches` default is documented as `true`, and the switch-creation latency metric entry is added to the observability table.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: defaulting create_switches to true and logging when switches are skipped.
Description check	✅ Passed	The description is directly aligned with the changeset and clearly explains the default change, logging, related issue, and testing.
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 · 2026-07-08T20:59:00Z

/ok to test db3965e

chet · 2026-07-08T21:00:09Z

Thanks @mehrdadmoradi-crusoe! I missed the fact @ajf assigned it off to you. I'm actually working on an iterative PR that does a few more things beyond this, BUT, I want to pull in your tweak first, and then I'll base my additional stuff on top of it. Appreciate the help here!

mehrdadmoradi-crusoe · 2026-07-08T21:07:31Z

@chet Sure. Sounds good to me. Thanks

chet · 2026-07-08T22:02:38Z

@mehrdadmoradi-crusoe There's a core_metrics.md diff here. You can either just self-apply the diff locally and force-push it back up, or if you run test_integration on your side (if your build environment is functional for it), it will also dump it out.

github-actions · 2026-07-08T22:28:02Z

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Low	Other
boot-artifacts-aarch64	3	0	0	3	0	0
boot-artifacts-x86_64	3	0	0	3	0	0
forge-admin-cli-x86_64	271	13	34	91	7	126
machine-validation-runner	776	37	223	281	40	195
machine_validation	776	37	223	281	40	195
machine_validation-aarch64	776	37	223	281	40	195
nvmetal-carbide	776	37	223	281	40	195
TOTAL	3381	161	926	1221	167	906

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

…pping create_switches defaulted to false while create_machines defaults to true, so with a default config explored switches are identified but never created as Switch objects, and nothing in the logs explains why (NVIDIA#3152). - Default create_switches to true (align with create_machines). - Log at info level when switches are identified but create_switches=false causes them to be skipped. - Update the config reference default in api-core README. Implements the change requested in NVIDIA#3152. Signed-off-by: Mehrdad Moradi <imehrdad2012@gmail.com>

mehrdadmoradi-crusoe · 2026-07-09T04:30:26Z

Thanks @chet ..applied the core_metrics.md diff and force-pushed. Head is now a74a2a9. Ready for a re-run whenever you are, and happy for you to base your iterative PR on top.

chet · 2026-07-09T05:07:39Z

/ok to test a74a2a9

chet

Ahh this needs @polarweasel also due to the docs/ update. I can ping him in the AM!

github-actions · 2026-07-09T05:10:25Z

🌿 Preview your docs: https://nvidia-preview-pull-request-3280.docs.buildwithfern.com/infra-controller

mehrdadmoradi-crusoe requested a review from a team as a code owner July 8, 2026 20:52

chet mentioned this pull request Jul 8, 2026

feat(site-explorer): create switches and power shelves by default #3276

Open

mehrdadmoradi-crusoe force-pushed the fix/site-explorer-create-switches-default branch from db3965e to 728681e Compare July 9, 2026 03:41

mehrdadmoradi-crusoe requested a review from polarweasel as a code owner July 9, 2026 03:41

mehrdadmoradi-crusoe force-pushed the fix/site-explorer-create-switches-default branch from 728681e to a74a2a9 Compare July 9, 2026 04:17

chet approved these changes Jul 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(site-explorer): default create_switches to true and log when skipping#3280

feat(site-explorer): default create_switches to true and log when skipping#3280
mehrdadmoradi-crusoe wants to merge 1 commit into
NVIDIA:mainfrom
mehrdadmoradi-crusoe:fix/site-explorer-create-switches-default

mehrdadmoradi-crusoe commented Jul 8, 2026

Uh oh!

copy-pr-bot Bot commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

chet commented Jul 8, 2026

Uh oh!

chet commented Jul 8, 2026

Uh oh!

mehrdadmoradi-crusoe commented Jul 8, 2026

Uh oh!

chet commented Jul 8, 2026

Uh oh!

github-actions Bot commented Jul 8, 2026

Uh oh!

mehrdadmoradi-crusoe commented Jul 9, 2026

Uh oh!

chet commented Jul 9, 2026

Uh oh!

chet left a comment •

edited

Loading

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mehrdadmoradi-crusoe commented Jul 8, 2026

Related issues

Type of Change

Breaking Changes

Testing

Additional Notes

Uh oh!

copy-pr-bot Bot commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chet commented Jul 8, 2026

Uh oh!

chet commented Jul 8, 2026

Uh oh!

mehrdadmoradi-crusoe commented Jul 8, 2026

Uh oh!

chet commented Jul 8, 2026

Uh oh!

github-actions Bot commented Jul 8, 2026

🔍 Container Scan Summary

Uh oh!

mehrdadmoradi-crusoe commented Jul 9, 2026

Uh oh!

chet commented Jul 9, 2026

Uh oh!

chet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

chet left a comment •

edited

Loading