From c9bcc1753bae4eea4c1c42e9e4e66283951228e6 Mon Sep 17 00:00:00 2001 From: Hasan Khan Date: Sat, 8 Aug 2026 18:45:56 -0700 Subject: [PATCH 1/2] Add NICo NVSwitch firmware query Signed-off-by: Hasan Khan --- .../providers/nico/config/bare_metal.yaml | 4 +- .../nico/scripts/breakfix/gap_stub.py | 6 +- .../scripts/breakfix/query_switch_firmware.py | 110 ++++++++++++++++ .../providers/nico/test_nico_provider.py | 124 ++++++++++++++++++ 4 files changed, 236 insertions(+), 8 deletions(-) create mode 100755 isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py diff --git a/isvctl/configs/providers/nico/config/bare_metal.yaml b/isvctl/configs/providers/nico/config/bare_metal.yaml index 1b84b2baf..54882f16b 100644 --- a/isvctl/configs/providers/nico/config/bare_metal.yaml +++ b/isvctl/configs/providers/nico/config/bare_metal.yaml @@ -364,7 +364,7 @@ commands: - name: query_switch_firmware phase: test continue_on_failure: true - command: "python ../scripts/breakfix/gap_stub.py" + command: "python ../scripts/breakfix/query_switch_firmware.py" args: - "--org" - "{{org}}" @@ -372,8 +372,6 @@ commands: - "{{site_id}}" - "--api-base" - "{{nico_api_base}}" - - "--gap" - - "BFX03-02" timeout: 120 - name: query_bmc_kernel_logs diff --git a/isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py b/isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py index 55fdc447e..b5df988c8 100644 --- a/isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py +++ b/isvctl/configs/providers/nico/scripts/breakfix/gap_stub.py @@ -6,7 +6,7 @@ Several break-fix requirements have no NICo tenant REST surface to exercise: the mutating BFX01 workflows run through Maestro/repair fixtures, and the -BFX02-02/BFX03-02/BFX04-01/BFX05/BFX06 signals are not exposed at all. Each of +BFX02-02/BFX04-01/BFX05/BFX06 signals are not exposed at all. Each of those steps emits a structured skip naming the gap rather than a hard failure, so the suite reports "not available on this platform" instead of "broken". @@ -44,10 +44,6 @@ "NICo has no retirement-notice query API (BFX02-02 gap)", {"notices_queryable": False, "notices": []}, ), - "BFX03-02": ( - "NV switch tray firmware is not queryable via NICo tenant REST API (BFX03-02 gap)", - {"trays": []}, - ), "BFX04-01": ( "GPUd/Sentinel/Maestro node health agents are not observable via NICo REST (BFX04-01 gap)", {"agents_observable": False, "agents": []}, diff --git a/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py b/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py new file mode 100755 index 000000000..bb6230173 --- /dev/null +++ b/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Query NVSwitch tray firmware versions from NICo racks (BFX03-02). + +NICo's read-only rack list endpoint returns rack components when +``includeComponents`` is enabled. NVSwitch components expose their installed +firmware through ``firmwareVersion``. +""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path +from typing import Any +from urllib.error import URLError + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from breakfix._common import emit +from common.nico_client import NicoAuthError, forge_get_all, resolve_auth + + +def _is_nvswitch(component: dict[str, Any]) -> bool: + """Return whether a rack component is explicitly typed as an NVSwitch.""" + component_type = re.sub(r"[^a-z0-9]", "", str(component.get("type") or "").lower()) + return component_type in {"nvswitch", "componenttypenvswitch"} + + +def _tray_id(component: dict[str, Any]) -> str: + """Choose the first stable, human-useful identifier NICo provides.""" + for field in ("componentId", "id", "serialNumber", "name"): + value = component.get(field) + if value is not None and str(value).strip(): + return str(value) + return "" + + +def main() -> int: + """List NVSwitch tray firmware versions as provider-neutral JSON.""" + parser = argparse.ArgumentParser(description="Query NICo NVSwitch tray firmware versions") + parser.add_argument("--org", required=True) + parser.add_argument("--site-id", required=True) + parser.add_argument("--api-base", required=True) + args = parser.parse_args() + + result: dict[str, Any] = { + "success": False, + "platform": "nico", + "site_id": args.site_id, + "trays": [], + } + try: + auth = resolve_auth() + racks = forge_get_all( + args.org, + "rack", + auth.token, + base_url=args.api_base, + params={"siteId": args.site_id, "includeComponents": "true"}, + result_key="racks", + ) + except NicoAuthError as exc: + result.update(error_type="auth", error=str(exc)) + return emit(result) + except (URLError, ValueError) as exc: + result["error"] = f"{type(exc).__name__}: {exc}" + return emit(result) + + seen: set[str] = set() + trays: list[dict[str, Any]] = [] + for rack in racks: + rack_id = str(rack.get("id") or "") + components = rack.get("components") or [] + if not isinstance(components, list): + continue + for component in components: + if not isinstance(component, dict) or not _is_nvswitch(component): + continue + tray_id = _tray_id(component) + dedupe_key = tray_id or f"{rack_id}:{component.get('slotId')}:{component.get('trayIdx')}" + if dedupe_key in seen: + continue + seen.add(dedupe_key) + trays.append( + { + "tray_id": tray_id, + "firmware_version": str(component.get("firmwareVersion") or ""), + "rack_id": rack_id, + "slot_id": component.get("slotId"), + "tray_index": component.get("trayIdx"), + } + ) + + if not trays: + result.update( + success=True, + skipped=True, + skip_reason="No NVSwitch tray components were returned for this NICo site", + ) + return emit(result) + + result.update(success=True, trays=trays) + return emit(result) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/isvctl/tests/providers/nico/test_nico_provider.py b/isvctl/tests/providers/nico/test_nico_provider.py index 40c1cc708..640f51bba 100644 --- a/isvctl/tests/providers/nico/test_nico_provider.py +++ b/isvctl/tests/providers/nico/test_nico_provider.py @@ -141,6 +141,14 @@ def _load_nico_script(relative_path: str, module_name: str) -> ModuleType: return module +def _load_switch_firmware_script() -> ModuleType: + """Load the BFX03-02 NVSwitch firmware query script.""" + return _load_nico_script( + "breakfix/query_switch_firmware.py", + "test_query_switch_firmware", + ) + + def _load_governance_metrics_script() -> ModuleType: """Load the query_metrics (governance) script as a module for direct unit testing.""" script_path = NICO_SCRIPTS / "governance" / "query_metrics.py" @@ -1095,6 +1103,122 @@ def test_nico_scripts_require_api_base( assert "--api-base" in captured.err +def test_switch_firmware_queries_racks_and_maps_only_nvswitch_components( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """BFX03-02 should use NICo rack inventory and preserve missing firmware for validation.""" + module = _load_switch_firmware_script() + observed: dict[str, Any] = {} + + def fake_get_all(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: + observed["args"] = args + observed["kwargs"] = kwargs + return [ + { + "id": "rack-1", + "components": [ + { + "id": "internal-switch-1", + "componentId": "switch-1", + "type": "ComponentTypeNVSwitch", + "firmwareVersion": "1.2.3", + "slotId": 3, + "trayIdx": 0, + }, + { + "id": "switch-2", + "type": "nvswitch", + "firmwareVersion": None, + }, + { + "id": "compute-1", + "type": "ComponentTypeCompute", + "firmwareVersion": "9.9.9", + }, + ], + } + ] + + monkeypatch.setattr(module, "resolve_auth", lambda: SimpleNamespace(token="test-token")) + monkeypatch.setattr(module, "forge_get_all", fake_get_all) + monkeypatch.setattr( + sys, + "argv", + [ + "query_switch_firmware.py", + "--org", + "test-org", + "--site-id", + "site-1", + "--api-base", + "https://nico.example/v2/org", + ], + ) + + assert module.main() == 0 + + payload = json.loads(capsys.readouterr().out) + assert payload["success"] is True + assert payload["trays"] == [ + { + "tray_id": "switch-1", + "firmware_version": "1.2.3", + "rack_id": "rack-1", + "slot_id": 3, + "tray_index": 0, + }, + { + "tray_id": "switch-2", + "firmware_version": "", + "rack_id": "rack-1", + "slot_id": None, + "tray_index": None, + }, + ] + assert observed["args"] == ("test-org", "rack", "test-token") + assert observed["kwargs"] == { + "base_url": "https://nico.example/v2/org", + "params": {"siteId": "site-1", "includeComponents": "true"}, + "result_key": "racks", + } + + +def test_switch_firmware_skips_when_site_has_no_nvswitch_components( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """A site without NVSwitch inventory is inapplicable, not a false pass.""" + module = _load_switch_firmware_script() + monkeypatch.setattr(module, "resolve_auth", lambda: SimpleNamespace(token="test-token")) + monkeypatch.setattr( + module, + "forge_get_all", + lambda *args, **kwargs: [{"id": "rack-1", "components": [{"type": "ComponentTypeCompute"}]}], + ) + monkeypatch.setattr( + sys, + "argv", + [ + "query_switch_firmware.py", + "--org", + "test-org", + "--site-id", + "site-1", + "--api-base", + "https://nico.example/v2/org", + ], + ) + + assert module.main() == 0 + + payload = json.loads(capsys.readouterr().out) + assert payload["success"] is True + assert payload["skipped"] is True + assert payload["trays"] == [] + assert "No NVSwitch tray components" in payload["skip_reason"] + + def test_dpu_health_script_treats_nullable_machine_lists_as_empty( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], From d4d746196f684ed1227387f20323ff295033050a Mon Sep 17 00:00:00 2001 From: Hasan Khan Date: Sat, 8 Aug 2026 21:40:56 -0700 Subject: [PATCH 2/2] Handle Flow-disabled NVSwitch inventory Signed-off-by: Hasan Khan --- .../scripts/breakfix/query_switch_firmware.py | 76 +++++---- .../providers/nico/test_nico_provider.py | 146 ++++++++++++++---- isvtest/tests/test_breakfix.py | 43 ++++++ 3 files changed, 198 insertions(+), 67 deletions(-) diff --git a/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py b/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py index bb6230173..c40b01259 100755 --- a/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py +++ b/isvctl/configs/providers/nico/scripts/breakfix/query_switch_firmware.py @@ -2,11 +2,11 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""Query NVSwitch tray firmware versions from NICo racks (BFX03-02). +"""Query NVSwitch tray firmware versions from NICo Flow (BFX03-02). -NICo's read-only rack list endpoint returns rack components when -``includeComponents`` is enabled. NVSwitch components expose their installed -firmware through ``firmwareVersion``. +NICo's read-only tray list endpoint returns every tray at a Flow-enabled site. +The provider filters the version-specific tray type values client-side, and +NVSwitch trays expose their installed firmware through ``firmwareVersion``. """ from __future__ import annotations @@ -16,17 +16,19 @@ import sys from pathlib import Path from typing import Any -from urllib.error import URLError +from urllib.error import HTTPError, URLError sys.path.insert(0, str(Path(__file__).resolve().parents[1])) -from breakfix._common import emit +from breakfix._common import emit, skip_result from common.nico_client import NicoAuthError, forge_get_all, resolve_auth +_FLOW_DISABLED_MESSAGE = "site does not have nico flow enabled" + def _is_nvswitch(component: dict[str, Any]) -> bool: - """Return whether a rack component is explicitly typed as an NVSwitch.""" + """Return whether a tray is explicitly typed as an NVSwitch.""" component_type = re.sub(r"[^a-z0-9]", "", str(component.get("type") or "").lower()) - return component_type in {"nvswitch", "componenttypenvswitch"} + return component_type in {"switch", "nvswitch", "componenttypenvswitch"} def _tray_id(component: dict[str, Any]) -> str: @@ -38,6 +40,11 @@ def _tray_id(component: dict[str, Any]) -> str: return "" +def _flow_disabled(exc: HTTPError) -> bool: + """Return whether NICo rejected the query because Flow is disabled.""" + return exc.code == 412 and _FLOW_DISABLED_MESSAGE in str(exc).lower() + + def main() -> int: """List NVSwitch tray firmware versions as provider-neutral JSON.""" parser = argparse.ArgumentParser(description="Query NICo NVSwitch tray firmware versions") @@ -54,45 +61,48 @@ def main() -> int: } try: auth = resolve_auth() - racks = forge_get_all( + components = forge_get_all( args.org, - "rack", + "tray", auth.token, base_url=args.api_base, - params={"siteId": args.site_id, "includeComponents": "true"}, - result_key="racks", + params={"siteId": args.site_id}, + result_key="trays", ) except NicoAuthError as exc: result.update(error_type="auth", error=str(exc)) return emit(result) + except HTTPError as exc: + if _flow_disabled(exc): + skip = skip_result( + args.site_id, + "NICo Flow is not enabled for this site; NVSwitch tray firmware is unavailable (BFX03-02 gap)", + gap="BFX03-02", + ) + skip["trays"] = [] + return emit(skip) + result.update(error_type="api", error=f"NICo tray query failed (HTTP {exc.code})") + return emit(result) except (URLError, ValueError) as exc: result["error"] = f"{type(exc).__name__}: {exc}" return emit(result) seen: set[str] = set() trays: list[dict[str, Any]] = [] - for rack in racks: - rack_id = str(rack.get("id") or "") - components = rack.get("components") or [] - if not isinstance(components, list): + for component in components: + if not isinstance(component, dict) or not _is_nvswitch(component): continue - for component in components: - if not isinstance(component, dict) or not _is_nvswitch(component): - continue - tray_id = _tray_id(component) - dedupe_key = tray_id or f"{rack_id}:{component.get('slotId')}:{component.get('trayIdx')}" - if dedupe_key in seen: - continue - seen.add(dedupe_key) - trays.append( - { - "tray_id": tray_id, - "firmware_version": str(component.get("firmwareVersion") or ""), - "rack_id": rack_id, - "slot_id": component.get("slotId"), - "tray_index": component.get("trayIdx"), - } - ) + tray_id = _tray_id(component) + if tray_id and tray_id in seen: + continue + if tray_id: + seen.add(tray_id) + trays.append( + { + "tray_id": tray_id, + "firmware_version": str(component.get("firmwareVersion") or ""), + } + ) if not trays: result.update( diff --git a/isvctl/tests/providers/nico/test_nico_provider.py b/isvctl/tests/providers/nico/test_nico_provider.py index 640f51bba..2c00560b1 100644 --- a/isvctl/tests/providers/nico/test_nico_provider.py +++ b/isvctl/tests/providers/nico/test_nico_provider.py @@ -1103,11 +1103,11 @@ def test_nico_scripts_require_api_base( assert "--api-base" in captured.err -def test_switch_firmware_queries_racks_and_maps_only_nvswitch_components( +def test_switch_firmware_queries_all_trays_and_filters_nvswitches( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], ) -> None: - """BFX03-02 should use NICo rack inventory and preserve missing firmware for validation.""" + """BFX03-02 should query all trays and accept each deployed NVSwitch type spelling.""" module = _load_switch_firmware_script() observed: dict[str, Any] = {} @@ -1116,28 +1116,32 @@ def fake_get_all(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: observed["kwargs"] = kwargs return [ { - "id": "rack-1", - "components": [ - { - "id": "internal-switch-1", - "componentId": "switch-1", - "type": "ComponentTypeNVSwitch", - "firmwareVersion": "1.2.3", - "slotId": 3, - "trayIdx": 0, - }, - { - "id": "switch-2", - "type": "nvswitch", - "firmwareVersion": None, - }, - { - "id": "compute-1", - "type": "ComponentTypeCompute", - "firmwareVersion": "9.9.9", - }, - ], - } + "id": "internal-switch-1", + "componentId": "switch-1", + "type": "switch", + "firmwareVersion": "1.2.3", + "rackId": "rack-1", + "position": {"slotId": 3, "trayIdx": 0}, + }, + { + "id": "switch-2", + "type": "NVSwitch", + "firmwareVersion": None, + "rackId": "rack-2", + "position": {"slotId": 5, "trayIdx": 1}, + }, + { + "id": "switch-3", + "type": "ComponentTypeNVSwitch", + "firmwareVersion": "3.4.5", + "rackId": "rack-2", + }, + { + "id": "compute-1", + "type": "compute", + "firmwareVersion": "9.9.9", + "rackId": "rack-2", + }, ] monkeypatch.setattr(module, "resolve_auth", lambda: SimpleNamespace(token="test-token")) @@ -1164,23 +1168,21 @@ def fake_get_all(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: { "tray_id": "switch-1", "firmware_version": "1.2.3", - "rack_id": "rack-1", - "slot_id": 3, - "tray_index": 0, }, { "tray_id": "switch-2", "firmware_version": "", - "rack_id": "rack-1", - "slot_id": None, - "tray_index": None, + }, + { + "tray_id": "switch-3", + "firmware_version": "3.4.5", }, ] - assert observed["args"] == ("test-org", "rack", "test-token") + assert observed["args"] == ("test-org", "tray", "test-token") assert observed["kwargs"] == { "base_url": "https://nico.example/v2/org", - "params": {"siteId": "site-1", "includeComponents": "true"}, - "result_key": "racks", + "params": {"siteId": "site-1"}, + "result_key": "trays", } @@ -1194,7 +1196,7 @@ def test_switch_firmware_skips_when_site_has_no_nvswitch_components( monkeypatch.setattr( module, "forge_get_all", - lambda *args, **kwargs: [{"id": "rack-1", "components": [{"type": "ComponentTypeCompute"}]}], + lambda *args, **kwargs: [{"id": "compute-1", "type": "compute"}], ) monkeypatch.setattr( sys, @@ -1219,6 +1221,82 @@ def test_switch_firmware_skips_when_site_has_no_nvswitch_components( assert "No NVSwitch tray components" in payload["skip_reason"] +def test_switch_firmware_skips_when_nico_flow_is_disabled( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """NICo's Flow-disabled precondition is a documented runtime gap, not a pass.""" + module = _load_switch_firmware_script() + + def flow_disabled(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: + raise HTTPError( + "https://nico.example/v2/org/test-org/nico/tray", + 412, + "Site does not have NICo Flow enabled", + None, + None, + ) + + monkeypatch.setattr(module, "resolve_auth", lambda: SimpleNamespace(token="test-token")) + monkeypatch.setattr(module, "forge_get_all", flow_disabled) + monkeypatch.setattr( + sys, + "argv", + [ + "query_switch_firmware.py", + "--org", + "test-org", + "--site-id", + "site-1", + "--api-base", + "https://nico.example/v2/org", + ], + ) + + assert module.main() == 0 + + payload = json.loads(capsys.readouterr().out) + assert payload["success"] is True + assert payload["skipped"] is True + assert payload["gap"] == "BFX03-02" + assert payload["trays"] == [] + assert "Flow is not enabled" in payload["skip_reason"] + + +def test_switch_firmware_does_not_skip_other_http_errors( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """Authentication and API authorization failures must remain failures.""" + module = _load_switch_firmware_script() + + def forbidden(*args: Any, **kwargs: Any) -> list[dict[str, Any]]: + raise HTTPError("https://nico.example/tray", 403, "Forbidden", None, None) + + monkeypatch.setattr(module, "resolve_auth", lambda: SimpleNamespace(token="test-token")) + monkeypatch.setattr(module, "forge_get_all", forbidden) + monkeypatch.setattr( + sys, + "argv", + [ + "query_switch_firmware.py", + "--org", + "test-org", + "--site-id", + "site-1", + "--api-base", + "https://nico.example/v2/org", + ], + ) + + assert module.main() == 1 + + payload = json.loads(capsys.readouterr().out) + assert payload["success"] is False + assert payload["error_type"] == "api" + assert "skipped" not in payload + + def test_dpu_health_script_treats_nullable_machine_lists_as_empty( monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], diff --git a/isvtest/tests/test_breakfix.py b/isvtest/tests/test_breakfix.py index 485c8cbcb..c4465815c 100644 --- a/isvtest/tests/test_breakfix.py +++ b/isvtest/tests/test_breakfix.py @@ -17,6 +17,7 @@ HostReplacementCheck, MaintenanceEventsCheck, NodeHealthAgentCheck, + NvSwitchFirmwareCheck, PlannedMaintenanceNotificationCheck, RepairHistoryCheck, RetirementNoticesCheck, @@ -146,6 +147,48 @@ def test_node_maintenance_reports_mode(self) -> None: assert "maintenance_mode=hw" in check.message +class TestNvSwitchFirmwareCheck: + """Cover BFX03-02 firmware evidence for every returned switch tray.""" + + def test_propagates_flow_disabled_runtime_skip(self) -> None: + """A Flow-disabled site remains skipped instead of becoming a pass.""" + step_output = { + "success": True, + "skipped": True, + "skip_reason": "NICo Flow is not enabled for this site", + "gap": "BFX03-02", + "trays": [], + } + with pytest.raises(pytest.skip.Exception): + _run(NvSwitchFirmwareCheck, step_output) + + def test_passes_when_every_tray_has_firmware(self) -> None: + """Every discovered NVSwitch tray must report a non-empty version.""" + step_output = { + "success": True, + "trays": [ + {"tray_id": "switch-1", "firmware_version": "1.2.3"}, + {"tray_id": "switch-2", "firmware_version": "2.0.0"}, + ], + } + check = _run(NvSwitchFirmwareCheck, step_output) + assert check.passed + assert "2 NV switch tray(s)" in check.message + + def test_fails_when_any_tray_has_no_firmware(self) -> None: + """One missing version keeps partial inventory from passing BFX03-02.""" + step_output = { + "success": True, + "trays": [ + {"tray_id": "switch-1", "firmware_version": "1.2.3"}, + {"tray_id": "switch-2", "firmware_version": ""}, + ], + } + check = _run(NvSwitchFirmwareCheck, step_output) + assert not check.passed + assert "1 switch tray(s) missing firmware_version" in check.message + + class TestNodeHealthAgentCheck: """Cover the BFX04-01 GPUd/Sentinel health-agent check."""