Skip to content

feat(nico): add NICo governance metrics validation (CAP01-01) - #451

Merged
abegnoche merged 3 commits into
mainfrom
cursor/cap01-01-governance-metrics-035e
Jun 9, 2026
Merged

feat(nico): add NICo governance metrics validation (CAP01-01)#451
abegnoche merged 3 commits into
mainfrom
cursor/cap01-01-governance-metrics-035e

Conversation

@abegnoche

@abegnoche abegnoche commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

Adds the M6 CAP01-01 governance metrics validation as a follow-up to the NICo work in #450. A Cloud Governance API must expose Delivered, Healthy, Reserved, and Active counts for nodes and GPUs; this PR validates that contract end-to-end on the NICo provider.

  • GovernanceMetricsCheck (isvtest/.../validations/governance.py): provider-agnostic check that the four buckets exist with non-negative integer (nodes, gpus) counts and that Healthy/Reserved ⊆ Delivered and Active ⊆ Reserved. Optional min_delivered_{nodes,gpus} thresholds.
  • NICo query_metrics.py (providers/nico/scripts/governance/): pages the site's machines via the shared NICo client and maps each MachineStatus into a bucket (Decommissioned/Unknown excluded). GPU counts sum MachineCapability.count.
  • Wiring: bare_metal.yaml gets a governance_metrics validation on a new query_governance_metrics step (opt-in, so providers that don't wire it skip automatically); the NICo config wires the script with the existing org/site_id/api_base settings.
  • Docs: configs/suites/README.md lists the new step and JSON fields.

NICo MachineStatus → bucket: Delivered = not {Decommissioned, Unknown}; Healthy = Delivered with no health.alerts; Reserved = {InUse, Maintenance}; Active = {InUse}.

Ships unreleased (released_tests.json untouched); run with ISVTEST_INCLUDE_UNRELEASED=1.

Testing

  • make test — 925 pass (17 new validation cases + 6 new NICo script cases, incl. an end-to-end test piping the script's JSON through the validation).
  • make lint / uvx pre-commit run -a — clean.
  • make demo-testmy-isv configs pass; new check skips with step_not_configured where the step isn't wired.
  • Live NICo run (ISVTEST_INCLUDE_UNRELEASED=1 isvctl test run -f providers/nico/config/bare_metal.yaml): GovernanceMetricsCheck PASSED — delivered: nodes=20, gpus=160 across the 20-machine site.

Closes #251

Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features

    • Added governance metrics validation for bare-metal systems to query and aggregate machine states (delivered, healthy, reserved, active) and enforce minimum delivery thresholds.
  • Documentation

    • Updated validation suite documentation to describe the new governance metrics validation step and its expected output fields.
  • Tests

    • Added comprehensive test coverage for governance metrics aggregation, validation logic, error handling, and API failures.

Adds a provider-agnostic GovernanceMetricsCheck plus a NICo-side
aggregator so the bare_metal suite verifies that the cloud governance
API surfaces the canonical Delivered, Healthy, Reserved, and Active
buckets for both nodes and GPUs (CAP01-01).

- GovernanceMetricsCheck (isvtest): asserts the four canonical metric
  buckets exist with non-negative integer (nodes, gpus) counts and that
  the inter-metric invariants hold (Healthy / Reserved \u2286 Delivered,
  Active \u2286 Reserved). Optional min_delivered_{nodes,gpus} thresholds.
- NICo query_metrics.py: reuses the shared NICo client to page through
  the site's machines and classify each by MachineStatus into the
  governance buckets (Decommissioned/Unknown excluded so they cannot
  leak into Reserved/Active). GPU counts sum MachineCapability.count.
- bare_metal suite: new query_governance_metrics step + GovernanceMetrics
  validation; the step is omitted by providers that have not wired it
  yet so it skips automatically (matches the ingestion/dpu pattern).
- Provider config: wires the NICo provider to invoke the new script.
- Documentation: suites/README.md lists the new step and JSON fields.

Tests cover the validation's bucket-shape, value-type, threshold and
invariant paths, plus the NICo script's status-bucket mapping,
nullable-field tolerance, error propagation, and an end-to-end contract
test that pipes the script's JSON through the validation.

Ships unreleased (released_tests.json is unchanged); exercise via
ISVTEST_INCLUDE_UNRELEASED=1.

Closes #251

Signed-off-by: Cursor Agent <cursoragent@cursor.com>

Co-authored-by: Alexandre Begnoche <abegnoche@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 8, 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.

@abegnoche abegnoche changed the title feat(governance): add NICo governance metrics validation (CAP01-01) feat(nico): add NICo governance metrics validation (CAP01-01) Jun 8, 2026
@abegnoche
abegnoche marked this pull request as ready for review June 8, 2026 17:57
@abegnoche
abegnoche requested a review from a team as a code owner June 8, 2026 17:57
@abegnoche

Copy link
Copy Markdown
Member Author

/ok to test 63aac42

@github-actions

github-actions Bot commented Jun 8, 2026

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-06-08 17:59:59 UTC | Commit: 63aac42

Collapse the per-bucket helpers in query_metrics.py: drop the
_EMPTY_BUCKET constant and its copies for a name-tuple + dict
comprehension, remove the trivial _add() wrapper, and fold the four
near-identical conditionals into a single membership map. Behavior is
unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
@abegnoche
abegnoche force-pushed the cursor/cap01-01-governance-metrics-035e branch from 63aac42 to 8c9a573 Compare June 8, 2026 18:03
@abegnoche

Copy link
Copy Markdown
Member Author

/ok to test 63aac42

@copy-pr-bot

copy-pr-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

/ok to test 63aac42

@abegnoche, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@abegnoche

Copy link
Copy Markdown
Member Author

/ok to test dfa76f4

@abegnoche

abegnoche commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@NVIDIA NVIDIA deleted a comment from coderabbitai Bot Jun 8, 2026
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: a19470c6-2910-46c2-a7be-e67343d12691

📥 Commits

Reviewing files that changed from the base of the PR and between 0bcbb81 and dfa76f4.

📒 Files selected for processing (8)
  • isvctl/configs/providers/nico/config/bare_metal.yaml
  • isvctl/configs/providers/nico/scripts/governance/query_metrics.py
  • isvctl/configs/suites/README.md
  • isvctl/configs/suites/bare_metal.yaml
  • isvctl/tests/test_nico_provider.py
  • isvtest/src/isvtest/validations/__init__.py
  • isvtest/src/isvtest/validations/governance.py
  • isvtest/tests/test_governance.py

Walkthrough

This PR introduces a complete governance metrics capability for bare-metal NICo validation. A new query_metrics.py script queries the NICo API for machines at a site, classifies them into delivered/healthy/reserved/active buckets based on status rules, and outputs aggregated node and GPU counts. A new GovernanceMetricsCheck validation reads this output, enforces schema correctness, rejects invalid types and negative values, validates bucket subset relationships, and optionally enforces minimum delivered thresholds. Comprehensive tests cover the script's classification logic and edge cases, plus full validation unit coverage.

Changes

Governance Metrics Query and Validation

Layer / File(s) Summary
Governance Metrics Query Script
isvctl/configs/providers/nico/scripts/governance/query_metrics.py, isvctl/configs/providers/nico/config/bare_metal.yaml, isvctl/configs/suites/README.md
New query_metrics.py CLI script parses --org, --site-id, and --api-base arguments, resolves NICo authentication, fetches all machines for the site, classifies each by status into delivered/healthy/reserved/active buckets with GPU tallies, and outputs a JSON contract containing machine count and per-bucket node/GPU metrics. Script is wired into bare_metal.yaml as a timed command step, and configuration comments are updated to reflect the newly implemented governance-metrics lifecycle component. README documents the step and its expected JSON output fields.
Governance Validation Framework
isvtest/src/isvtest/validations/governance.py, isvtest/src/isvtest/validations/__init__.py, isvctl/configs/suites/bare_metal.yaml
New GovernanceMetricsCheck validation class reads step output, validates the metrics schema (required delivered/healthy/reserved/active buckets with nodes and gpus resource counts), explicitly rejects boolean and non-integer count values, emits per-bucket subtest results, applies optional min_delivered_nodes and min_delivered_gpus config thresholds (defaulting to 0), and enforces invariants: healthy ≤ delivered, reserved ≤ delivered, active ≤ reserved. Validation is exported via the validations package and integrated into bare_metal.yaml validation contract with min_delivered_nodes: 1.
Query Metrics Script Testing
isvctl/tests/test_nico_provider.py
New _load_governance_metrics_script loader dynamically imports the governance script. Bare-metal config and script requirements tests are parametrized to include query_governance_metrics step. Governance script test suite mocks NICo auth and API calls, verifies bucket classification by machine status (handling delivered, healthy, reserved, active statuses and excluding decommissioned/unknown), ensures empty-site output zeroes all buckets, tolerates missing/nullable optional fields, validates complete end-to-end output against the GovernanceMetricsCheck contract, and confirms API/auth exceptions are returned as JSON error payloads with exit code 1.
Governance Validation Unit Tests
isvtest/tests/test_governance.py
New test module with _metrics_output() helper that constructs governance step output dicts with default metrics and per-test overrides, supporting mutation-safe independent dict instances. TestGovernanceMetricsCheck suite validates well-formed metrics pass, propagates step-level failures, fails on missing metrics object or required buckets, fails on missing per-bucket fields, rejects negative counts and boolean values, enforces non-integer type rejection, asserts bucket subset invariants across both resources, verifies configurable min_delivered_nodes/min_delivered_gpus thresholds with defaults and invalid type handling, and confirms helper dict isolation.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • mresvanis

🐰 A script hops through machines, counting each hop—
Delivered, healthy, reserved, and active!
The validation checks every bucket true,
Ensuring none violate their subset's due.

🚥 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 'feat(nico): add NICo governance metrics validation (CAP01-01)' clearly and specifically describes the main change—adding governance metrics validation for the NICo provider tied to the CAP01-01 requirement.
Linked Issues check ✅ Passed The PR fully implements CAP01-01 requirements: governance metrics validation with four buckets (Delivered, Healthy, Reserved, Active) for nodes/GPUs, NICo aggregator script, validation logic with invariant checks, and comprehensive testing.
Out of Scope Changes check ✅ Passed All changes are in-scope: governance validation module, NICo aggregator script, configuration updates, test suites, and documentation. No unrelated refactoring, dependency upgrades, or extraneous modifications present.
Docstring Coverage ✅ Passed Docstring coverage is 97.44% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/cap01-01-governance-metrics-035e

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

@abegnoche
abegnoche merged commit 2db4916 into main Jun 9, 2026
7 checks passed
@abegnoche
abegnoche deleted the cursor/cap01-01-governance-metrics-035e branch June 9, 2026 12:35
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.

CAP01-01: Query and verify governance API returns Delivered, Healthy, Reserved, and Active metrics for nodes/GPUs

3 participants