Skip to content

Add NICo NVSwitch firmware query - #573

Open
osu wants to merge 2 commits into
NVIDIA:mainfrom
osu:issue-214-nv-switch-firmware
Open

Add NICo NVSwitch firmware query#573
osu wants to merge 2 commits into
NVIDIA:mainfrom
osu:issue-214-nv-switch-firmware

Conversation

@osu

@osu osu commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

  • replace the BFX03-02 NICo gap stub with the read-only tray inventory API
  • query every paginated tray and filter NVSwitch types client-side so both deployed and current API enum spellings are supported
  • map every NVSwitch tray to the provider-neutral tray_id / firmware_version contract
  • preserve missing firmware values so incomplete inventory fails visibly
  • emit a structured BFX03-02 runtime skip only for the exact NICo 412 Site does not have NICo Flow enabled response

Why

NICo exposes NVSwitch tray firmware through its Flow-backed tray inventory. Available DSX staging sites currently have Flow disabled, so the provider must distinguish an unavailable site capability from an implementation failure without reporting false success.

Closes #214.

Validation

  • uv run pytest -q isvctl/tests/providers/nico/test_nico_provider.py isvtest/tests/test_breakfix.py (191 passed)
  • uv run isvctl test validate -f isvctl/configs/providers/nico/config/bare_metal.yaml
  • applicable pre-commit hooks passed
  • live read-only NICo execution on az51-dev3 and az51-dev4-dh1 returned the expected structured BFX03-02 runtime skip because both sites report Flow disabled
  • no cluster state was changed

A live positive result still requires a Flow-enabled staging site with NVSwitch trays.

Summary by CodeRabbit

  • New Features

    • Added automated NVSwitch firmware discovery across site trays.
    • Reports firmware versions in a standardized format and recognizes supported NVSwitch component types.
    • Provides clear skipped results when inventory is unavailable or the required service is disabled.
  • Bug Fixes

    • Replaced the previous placeholder break-fix behavior with the actual firmware query.
    • Correctly handles missing firmware versions and relevant API or response errors.

Signed-off-by: Hasan Khan <hasank@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 9, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

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: 55059894-c66b-419d-9ad3-603c801fdde1

📥 Commits

Reviewing files that changed from the base of the PR and between d1e4b34 and d4d7461.

📒 Files selected for processing (5)
  • isvctl/configs/providers/nico/config/bare_metal.yaml
  • isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py
  • isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py
  • isvctl/tests/providers/nico/test_nico_provider.py
  • isvtest/tests/test_breakfix.py

📝 Walkthrough

Walkthrough

The NICo break-fix flow now queries NVSwitch tray firmware through a standalone provider script. It filters and normalizes inventory data, handles skips and API failures, and validates complete firmware evidence.

Changes

NICo NVSwitch firmware inspection

Layer / File(s) Summary
Query implementation and break-fix wiring
isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py, isvctl/configs/providers/nico/config/bare_metal.yaml, isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py
The configuration invokes the new firmware query script. The script retrieves NICo trays, filters and deduplicates NVSwitch components, normalizes firmware values, and emits structured JSON. The obsolete gap-stub entry is removed.
Provider query coverage
isvctl/tests/providers/nico/test_nico_provider.py
Tests cover tray requests, supported NVSwitch type spellings, missing firmware values, empty inventory skips, Flow-disabled skips, and unrelated HTTP failures.
Firmware validation coverage
isvtest/tests/test_breakfix.py
Tests cover Flow-disabled skips, successful validation with complete firmware data, and failure when firmware data is incomplete.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to d4d74

The PR adds read-only NVSwitch firmware inventory handling and preserves the documented Flow-disabled skip behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: abegnoche

Sequence Diagram(s)

sequenceDiagram
  participant query_switch_firmware.py
  participant NICo authentication
  participant NICo trays API
  participant NvSwitchFirmwareCheck
  query_switch_firmware.py->>NICo authentication: authenticate
  query_switch_firmware.py->>NICo trays API: request site trays
  NICo trays API-->>query_switch_firmware.py: return tray components
  query_switch_firmware.py-->>NvSwitchFirmwareCheck: return normalized firmware records
  NvSwitchFirmwareCheck->>NvSwitchFirmwareCheck: validate firmware for all NVSwitch trays
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding the NICo NVSwitch firmware query.
Linked Issues check ✅ Passed The PR implements BFX03-02 by querying NVSwitch tray firmware versions and adds coverage for success, missing data, and Flow-disabled cases [#214].
Out of Scope Changes check ✅ Passed The configuration, script changes, and tests directly support the BFX03-02 implementation and contain no unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@osu

osu commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Closing after live NICo validation. The read-only rack endpoint returned 412 Site does not have NICo Flow enabled on both az51-dev3 and az51-dev4-dh1; the tray endpoint with type=NVLSwitch returned the same result on dev3. Since the platform feature is not enabled in the available validation environments, we should retain the existing BFX03-02 gap stub rather than add an untestable ISV implementation. No cluster state was changed.

@osu osu closed this Aug 9, 2026
Signed-off-by: Hasan Khan <hasank@nvidia.com>
@osu osu reopened this Aug 9, 2026
@osu
osu marked this pull request as ready for review August 14, 2026 14:54
@osu
osu requested a review from a team as a code owner August 14, 2026 14:54
@osu

osu commented Aug 14, 2026

Copy link
Copy Markdown
Member Author

/ok to test d4d7461

@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-14 14:56:02 UTC | Commit: d4d7461

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.

BFX03-02: Inspect firmware versions of NV switch trays

1 participant