feat: add SDN controller validation tests (BYOIP, stable IP, floating IP, DNS, VPC peering) - #144
Merged
Conversation
… IP, DNS, VPC peering) Add 5 new network validation test suites for M3 DGXC Min Requirements: - #115 BYOIP: verify non-conflicting custom CIDRs (e.g. 7.0.0.0/8) - #116 Stable Private IP: verify IP persists across instance stop/start - #117 Floating IP: verify atomic EIP switch between instances (<10s) - #118 Localized DNS: verify private hosted zone and internal domain resolution - #119 VPC Peering: verify peering lifecycle, routes, and active state For each test: AWS boto3 stub, provider-agnostic template stub, validation class, output schema, and config wiring. Network test suite goes from 6 to 11 tests. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
- Move get_amazon_linux_ami() to common/ec2.py to deduplicate - Fix dns_test.py: compute storage_record from --domain arg instead of module-level constant (was ignoring custom domain) - Fix peering_test.py: initialize status=None before polling loop to avoid unbound variable if loop doesn't execute - Fix floating_ip_test.py: add route table cleanup in finally block - Add 5 new SDN validation classes to __init__.py exports - Add 16 unit tests for ByoipCheck, StablePrivateIpCheck, FloatingIpCheck, LocalizedDnsCheck, VpcPeeringCheck Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
- Disassociate route table before deleting it in floating_ip_test.py cleanup (AWS rejects delete_route_table with active associations) - Pass args.max_switch_seconds to reassociate_eip_timed() so the --max-switch-seconds CLI arg is respected by the script check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
daluz
approved these changes
Mar 23, 2026
This was referenced Mar 23, 2026
abegnoche
added a commit
to abegnoche/ai-cloud-validation
that referenced
this pull request
Jun 16, 2026
Verified via the introducing PR -> closed issue -> plan entry chain (with description disambiguation for multi-check PRs): - ByoipCheck NET03-01 (PR NVIDIA#144 / issue NVIDIA#115, Bring-Your-Own-IP) - DhcpIpManagementCheck CP-XX-01, VpcIpConfigCheck CP-XX-02 (PR NVIDIA#143 / NVIDIA#131,NVIDIA#132) - VpcCrudCheck SDN01-01..04, VpcIsolationCheck SDN04-02/-03, FloatingIpCheck SDN05-01, StablePrivateIpCheck SDN-XX-01, LocalizedDnsCheck SDN06-01, VpcPeeringCheck SDN07-01. SubnetConfigCheck, NetworkProvisionedCheck, NetworkConnectivityCheck, TrafficFlowCheck, and SecurityBlockingCheck are left unset: they originate from the foundational refactor commit with no issue link and no unambiguous plan entry. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com>
abegnoche
added a commit
that referenced
this pull request
Jun 17, 2026
* feat(test-plan): link validation classes to test-plan via test_ids metadata Establish test_id as the durable join key between docs/test-plan.yaml, the validation classes, and the release manifest, replacing brittle PR/commit archaeology. - Add `test_ids` ClassVar to BaseValidation (+ get_validation_test_ids helper) and surface it through the catalog. - Seed all 20 security.py validation classes with verified test_ids. - scripts/test_plan_coverage.py: offline join + report; `--check` fails when a class references a test_id absent from the plan (CI integrity guardrail). - scripts/sync_test_plan_status.py: refresh `#N (open/closed)` annotations from live GitHub (`--write`) or flag drift (`--check`). - make plan-coverage / make sync-plan targets; pytest guardrails under scripts/tests/. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * ci: add Test Plan Guardrails job Run the test-plan coverage integrity check (`make plan-coverage CHECK=1`) as a blocking gate so a validation class can never reference a test_id missing from docs/test-plan.yaml. Add the live GitHub issue-state drift check (`make sync-plan CHECK=1`) as an informational, non-blocking step since it depends on external issue state. Wire the new job into pipeline-status. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): derive issue open/closed state at render time Stop storing GitHub issue state in docs/test-plan.yaml - it duplicated the source of truth (GitHub) and went stale. github_issues entries are now bare "#N" references; open/closed status is resolved live by the renderer. - Strip "(open)"/"(closed)" from 239 github_issues entries, keeping the issue number (the stable plan->issue link). - test_plan_yaml_to_adoc.py: best-effort fetch issue state via gh and render status icons; make plan still works offline (plain links, no icons). validate_test_plan now rejects stored state to keep the invariant. - Retire scripts/sync_test_plan_status.py, the make sync-plan target, and the CI drift step; the plan-coverage integrity gate stays. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): seed test_ids for 37 more validation classes Map high-confidence classes to their test-plan IDs (verified against plan summaries / introducing commits), extending the code<->plan link beyond the security domain: - K8s: CSI suite (K8S23-04..07), node-pool CRUD (K8S06-01..03), conformance, metrics, network ACL, OIDC, control-plane logs, network policy, autoscaler. - SDN: security-group CRUD + scoping (SDN02-*), fault/perf/audit logging (SDN09-*), network fabric (NET01-01, NET02-02). - Observability/telemetry (OBS-XX-17..19, TELEM-XX-04) and assorted compute, serial-console, access-key, and service-account checks. Generic/reusable checks and classes without an unambiguous mapping are left unset on purpose; the plan-coverage integrity gate guards every added value. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): seed test_ids for VM/BM lifecycle checks Map the provider-agnostic instance lifecycle checks (wired in both the bare_metal and vm suites) to their BM and VM test-plan IDs: - reboot CNP01-05/-13, stop CNP01-07/-14, start CNP01-08/-15, power-cycle CNP01-06, create CNP01-09, list CNP01-11, tags CNP05-01/-02, cloud-init BOOT02-01/-02, stable id CNP08-01/-03, specified key AUTH-XX-02. InstanceStateCheck is left unset: it is a generic post-op state assertion reused across many steps, not tied to a single plan item. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): seed test_ids for network checks Verified via the introducing PR -> closed issue -> plan entry chain (with description disambiguation for multi-check PRs): - ByoipCheck NET03-01 (PR #144 / issue #115, Bring-Your-Own-IP) - DhcpIpManagementCheck CP-XX-01, VpcIpConfigCheck CP-XX-02 (PR #143 / #131,#132) - VpcCrudCheck SDN01-01..04, VpcIsolationCheck SDN04-02/-03, FloatingIpCheck SDN05-01, StablePrivateIpCheck SDN-XX-01, LocalizedDnsCheck SDN06-01, VpcPeeringCheck SDN07-01. SubnetConfigCheck, NetworkProvisionedCheck, NetworkConnectivityCheck, TrafficFlowCheck, and SecurityBlockingCheck are left unset: they originate from the foundational refactor commit with no issue link and no unambiguous plan entry. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): seed test_ids for host, bare-metal, slurm, NIM, and tenant checks Mapped by reading each domain's plan section (no issue links exist for these foundational checks, which originate from bulk rename/refactor commits): - Slurm: SLURM-XX-02..09 (plan entries name the classes 1:1). - Tenant/IAM: TenantCreated/Listed/Info -> CP-XX-07/08/09; access-key authenticate/reject -> CP-XX-05/06. - Bare-metal NVIDIA hardware (Bm* cuda/driver/gpu) -> BMAAS-XX-08. - Host/SSH node checks -> BMAAS/VMAAS node entries (connectivity, OS, GPU, software/driver, vcpu/pci, ethernet/IB/NVLink, NCCL, GPU stress, training). - NIM health/model/inference -> BMAAS-XX-10, VMAAS-XX-12. CpuInfoCheck, ContainerRuntimeCheck, InstanceStateCheck and the foundational network helpers remain unset (no unambiguous plan entry). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): seed test_ids for K8s workloads and GPU operator checks - K8s workloads: NIM inference K8S-XX-03, NIM Helm K8S-XX-04, NCCL (single + multi-node) K8S-XX-05. - GPU Operator presence (namespace/pods + cluster-level installed check) -> K8S25-01. K8s GPU/node/pod-health checks (nvidia-smi, GPU pod access, capacity, labels, MIG, node-ready/count, expected-nodes, no-error/no-pending pods, pod health) are left unset: the plan's K8s section has no corresponding entry for them. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): drop min_req-specific coverage metrics The min_req label currently lives only in the test plan, not on the GitHub issues, so reporting coverage against it is premature. Replace the min_req-specific stats with generic coverage: plan items covered by >=1 class and by a released class. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * docs: remove comment * fix(test-plan): correct BM-only host checks wrongly mapped to VM test IDs EthernetCheck, InfiniBandCheck, NvlinkCheck, NcclCheck, and GpuStressCheck are wired only in the bare_metal suite (no vm label), so their VMAAS-XX-* ids were wrong. Drop the VM ids, keeping the bare-metal ones. Surfaced by the new domain-consistency guardrail. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(test-plan): add completeness and domain-consistency guardrails Extend `make plan-coverage CHECK=1` beyond referential integrity: - Completeness: every released class must declare test_ids unless explicitly allow-listed (ALLOWLIST_UNMAPPED) as generic/no-plan-entry. Catches forgotten classes. - Consistency: a class's labels must match the domain its test_ids imply (K8S->kubernetes, SEC->security, SDN/NET->network, BMAAS->bare_metal, VMAAS->vm, SLURM->slurm). Catches mis-assignments. Add `--review` to emit a class -> test_id -> plan-summary table for human correctness review, plus unit tests and a repo-wide guardrail test. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): mark intentional gaps with an UNMAPPED sentinel in test_ids Replace the central ALLOWLIST_UNMAPPED in the coverage tool with an explicit per-class marker so intent lives next to the code and an empty test_ids can never silently pass. - Add UNMAPPED sentinel to isvtest.core.validation; declare test_ids = (UNMAPPED,) on the 31 classes that intentionally map to no plan entry (generic helpers + K8s health/GPU + network/infra checks). - Completeness guardrail now fails for ANY class with an empty test_ids (real id or (UNMAPPED,) required), not just released ones - a new check must make an explicit choice. - Coverage/integrity/consistency/review ignore the sentinel via real_test_ids(). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move host-check test_ids into suite YAML wiring Prototype the YAML-only mapping model on the BM/VM host checks: each check declares a singular, context-correct test_id in bare_metal.yaml / vm.yaml instead of a class-level test_ids tuple. test_plan_coverage.py now reads test_id from the suite wiring and unions it with remaining class metadata so the integrity/completeness/consistency guardrails keep passing mid-migration. Per-context wiring surfaces a mapping the class model missed (GpuCheck covers BMAAS-XX-08 in BM, VMAAS-XX-06 in VM) and lets BM-only gaps be honest (N/A) where the BMaaS plan has no equivalent item. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move security-suite test_ids into YAML wiring Migrate every security.yaml check to a singular, context-correct test_id in the suite wiring and drop the corresponding class-level test_ids. This empties security.py of all test_id metadata; ServiceAccountCredentialCheck (defined in iam.py but wired only here) and the two vm.yaml-owned checks (ConsoleRbacCheck, VirtualDeviceHardeningCheck) move to their wiring suites too. Generic/UNMAPPED checks (e.g. StepSuccessCheck) are intentionally left as class-level metadata for now, pending a separate decision on how reusable checks should declare intentional gaps. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move slurm-suite test_ids into YAML wiring Migrate every slurm.yaml check (validations + workloads) to a singular test_id in the suite wiring and drop the class-level test_ids, emptying the slurm modules of test_id metadata. Slurm checks are wired only as variants (SlurmPartition-cpu/-gpu, SlurmGpuAllocation-1gpu/-2gpu, SlurmSbatchWorkload-gpu/-cpu/-inline). Teach apply_config_test_ids to propagate a variant's test_id up to its base class so the bare base entry in the catalog is not orphaned once its class id is removed; variant entries keep only their own id to preserve per-wiring precision. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move IAM-lifecycle test_ids into control-plane YAML The IAM lifecycle classes (AccessKey*/Tenant*, CP-XX-05..09) are wired in control-plane.yaml, not iam.yaml (which only wires generic checks). Migrate their test_ids into the control-plane wiring and drop the class-level ids, emptying iam.py of all test_id metadata. Finding: the suite a class is wired in does not track the class's source file or domain label (iam.py classes live in the control-plane suite; iam.yaml has no real-mapped checks). Generic checks here (FieldExistsCheck, FieldValueCheck, CrudOperationsCheck, StepSuccessCheck) remain class-level UNMAPPED for now. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move observability-suite test_ids into YAML wiring Migrate the four observability checks (OBS-XX-17/18/19, TELEM-XX-04) to singular test_ids in observability.yaml and drop the class-level ids, emptying observability.py. image-registry.yaml needs no migration: its only non-generic checks (ConnectivityCheck, OsCheck) are already mapped via the bm/vm suites, and InstanceStateCheck is generic/UNMAPPED. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): split InstanceRebootCheck duality into per-suite test_id Reviewed example of the BM/VM duality resolution: the class previously carried ("CNP01-05", "CNP01-13") with no way to say which applied in which context. bare_metal.yaml now declares CNP01-05 and vm.yaml declares CNP01-13, each unambiguous per wiring; the class tuple is removed. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move remaining instance-lifecycle test_ids into YAML Migrate the rest of instance.py (stop/start/tag/serial-console/stable-id/ power-cycle/topology/list/created/specified-key) into bare_metal.yaml and vm.yaml as singular per-context test_ids, leaving only the generic InstanceStateCheck (UNMAPPED) class-level. This completes the bm/vm suites. Per-context wiring surfaced gaps the class tuples hid: - SerialConsoleCheck is a BM/VM duality (CNP06-01 vs CNP06-03); the class only declared the BM id, so the VM serial-console mapping was missing. - InstanceListCheck's CNP01-11 is a VMaaS item; bare metal has no list-nodes plan entry, so the BM wiring is an explicit N/A. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): drop UNMAPPED sentinel and completeness guardrail Adopt option 2: a check with no test_id is allowed. Remove the completeness guardrail from test_plan_coverage.py (--check now runs integrity + consistency only) and delete the now-redundant UNMAPPED sentinels from all validation and workload classes (26 across 10 files). The UNMAPPED constant moves out of the validation framework (core/validation.py) into the coverage tool, since the only remaining sentinel is the YAML test_id: "N/A" marker for intentional per-wiring gaps. BaseValidation keeps an empty test_ids default purely so the catalog can read the attribute. Tradeoff: this removes the "forgot to map a new check" tripwire. It can be restored later by exempting a known generic set and requiring a test_id on everything else wired in a suite. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move 1:1 k8s test_ids into k8s.yaml wiring Migrate the straightforward k8s checks (validations + workloads) to singular test_ids in k8s.yaml and drop their class-level ids: storage (K8S23-04/05/06/07), oidc (K8S18-01), metrics (K8S07-01), control-plane logs (K8S20-01), api network acl (K8S15-01), gpu-operator (K8S25-01), autoscaler (K8S-XX-08), conformance (K8S01-01), network-policy/dual-stack (K8S22-01), and the NCCL/NIM workloads (K8S-XX-03/04/05). UNMAPPED k8s checks are now simply blank. K8sNodePoolCheck is intentionally left for a separate decision (its class declares K8S06-01/02/03 but the wiring only exercises create and update). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): split K8sNodePoolCheck per-wiring; expose delete gap K8sNodePoolCheck declared ("K8S06-01","K8S06-02","K8S06-03") but is only wired for create (CPU + GPU pools) and update. Map create wirings -> K8S06-01 and the update wiring -> K8S06-02 in k8s.yaml, drop the class tuple, and flag K8S06-03 (delete a node pool) as an explicit gap: no wiring exercises a delete, so it is now honestly uncovered rather than falsely claimed by the class tuple. This completes the k8s suite migration; only network.yaml remains. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move 1:1 network test_ids into network.yaml wiring Migrate the singular-mapped network checks to network.yaml and drop their class-level ids: SG scoping (SDN02-05..10), SDN logging (SDN09-01/02/03), policy propagation (SDN02-08), IP/DNS/peering (SDN05-01, SDN06-01, SDN07-01, SDN-XX-01), fabric (NET01-01, NET02-02, NET03-01), and CP-XX-01/02, DMS05-01. The three CRUD-style checks (VpcCrudCheck SDN01-01..04, SgCrudCheck SDN02-01..04, VpcIsolationCheck SDN04-02/03) are left on their class tuples pending the per-operation-split decision. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): remove remaining python test_ids; strip catalog test_ids Finish removing test_id metadata from the validation framework: - nim.py: migrate the BMAAS-XX-10 / VMAAS-XX-12 duality into the bm/vm nim_* wirings; drop the class tuple. - bm_host_status.py: migrate BMAAS-XX-07 to the BmHostStatusLog wiring. - bm_gpu/bm_cuda/bm_driver: drop BMAAS-XX-08 (these on-host checks are wired in no suite; BMAAS-XX-08 stays covered by GpuCheck in bare_metal.yaml). - cluster.py: drop K8S25-01 from the superseded, catalog_exclude GpuOperatorCheck. - network.py: drop the three CRUD class tuples (SDN01/SDN02/SDN04); these are re-covered per-operation in network.yaml in a follow-up (option a). catalog.py no longer sources test_ids from the class (the catalog references the implementation, not the YAML declaration; the proper "catalog reads the declaration" refactor is deferred). Coverage now joins purely on config-declared test_ids. BaseValidation keeps only an empty test_ids default. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): split network CRUD checks per-operation (option a) Re-cover the network CRUD plan items now that the class tuples are gone, using option (a): VpcCrudCheck and SgCrudCheck accept an optional `operations` config (default = full set), so each check can be wired once per operation with a single test_id, all binding to the same vpc_crud / sg_crud step: - VpcCrudCheck-create/read/update/delete -> SDN01-01..04 - SgCrudCheck-create/read/update/delete -> SDN02-01..04 VpcIsolationCheck validates isolation holistically (its sub-tests are per-VPC, not per-direction), so it maps to SDN04-02; SDN04-03 (E/W isolation) is exercised by the same check but is not separately attributable under a single test_id, and is left as a noted gap. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): annotate bare re-invocations in bare_metal.yaml Minimum annotation stays (one real test_id per check on its canonical wiring), but each bare lifecycle re-invocation now carries a comment pointing to the section where the test_id is declared (e.g. the "ssh" / "gpu" section), so a reader can tell the check is covered rather than accidentally omitted. Generic checks (InstanceStateCheck) are noted as having no plan test_id. All references are within this suite only; suites run independently, so no comment references another suite file. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): annotate generic/gap checks and map remaining suites Propagate the YAML test_id convention beyond bare_metal: - vm: pointer comments on bare re-invocations + generic-gate comments. - network/k8s: mark never-mapped domain sanity checks as test_id "N/A". - Map previously-uncovered plan items now that generic classes can carry per-wiring ids: K8S05-01 (K8sNodeReadyCheck), DATASVC-XX-01 (api_health FieldValueCheck + s3 CrudOperationsCheck), BOOT01-01..05 + BOOT03-02 (image-registry), IAM-XX-01/03 (iam). - Add a dedicated security-labelled SanitizationCheck (subclass of StepSuccessCheck) so SEC21-03 passes the SEC consistency guardrail without mislabelling the generic StepSuccessCheck. Guardrails green: 136 mapped classes pass integrity + consistency; make test and make demo-test pass. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): source labels from YAML wiring; label vm + bare_metal suites - pytest marks, catalog, and coverage consistency now union per-wiring `labels:` with class labels - drop SanitizationCheck shim; SEC21-03 rides StepSuccessCheck + labels - remove StableIdentifierCheck label ClassVar; every vm/bare_metal wiring now carries its platform label Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): label security + image-registry suites Every wiring in security.yaml carries `security` and every wiring in image-registry.yaml carries `image_registry`, so each suite is fully selectable by `-m <platform>` as labels migrate from class ClassVars onto the YAML wiring. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): label network + control-plane suites Every wiring in network.yaml carries `network` and every wiring in control-plane.yaml carries `control_plane`, so each suite is fully selectable by `-m <platform>` as labels migrate onto the YAML wiring. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): drop redundant security label ClassVars Now that security.yaml carries `labels: ["security"]` on every wiring, the `security` class label is sourced from YAML. Remove it from the 18 SEC-domain classes wired only in security.yaml (dropping the ClassVar entirely where `security` was the sole label, keeping trait labels like network/iam/workload otherwise). ConsoleRbacCheck and VirtualDeviceHardeningCheck keep `security` since they are wired in vm.yaml, which does not supply it. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): label iam + observability suites and drop their Python label ClassVars Move labels fully onto the YAML wiring for the observability and IAM domains, then remove the now-redundant class ClassVars: - observability.yaml carries each check's full label set (observability + its network/bare_metal/security/gpu trait); observability.py is now label-free. - iam.py domain classes are wired in control-plane.yaml and security.yaml (not iam.yaml), so add `iam` to those wirings and drop the iam.py ClassVars. - iam.yaml itself gains `iam` on its generic FieldExistsCheck/StepSuccessCheck wirings so the suite is `-m iam`-selectable. Catalog labels and platforms are unchanged; coverage + make test + demo-test pass. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): drop instance.py label ClassVars Every instance.py check's vm/bare_metal labels are now supplied by its vm.yaml / bare_metal.yaml wirings (verified the YAML union equals each class's former label set), so remove all label ClassVars from instance.py. Catalog labels and platforms are unchanged. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move host.py trait labels to YAML; drop host.py ClassVars Add each host check's full trait set (ssh/gpu/network/workload/slow) to its wirings in bare_metal.yaml, vm.yaml, and image-registry.yaml, then remove all label ClassVars from host.py (catalog labels verified identical to baseline). Also honor per-wiring `labels:` in the orchestrator's include/exclude-label filtering (resolution.parse_validations), so exclude.labels keeps working once labels live on the wiring rather than the class. Provider configs inherit the labels via deep-merge, so only suite files needed changes. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): label k8s suite; drop all k8s_* label ClassVars Add each k8s check's full label set (kubernetes + gpu/workload/slow/l2 traits) to its wiring in k8s.yaml, then remove the label ClassVars from every k8s_* validation and workload module (catalog labels verified identical to baseline; ruff dropped the now-unused ClassVar imports). Update the orchestrator label-filtering tests to wire `labels: ["kubernetes"]` on K8sNodeCountCheck, since labels now live on the wiring rather than the class. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): label slurm suite; drop all slurm_* label ClassVars Add each slurm check's full label set (slurm + gpu/workload/slow traits) to its wiring in slurm.yaml, then remove the label ClassVars from every slurm_* validation and workload module (catalog labels verified identical to baseline). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move network.py trait labels to YAML; drop network.py ClassVars Add the security/ssh trait labels to the relevant network.yaml wirings (SG scoping, isolation, audit-trail, DHCP), then remove all label ClassVars from network.py (catalog labels verified identical to baseline). Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move remaining security.py trait labels to YAML; drop ClassVars Add network/iam/workload/slow trait labels to the security.yaml wirings and security/iam to the ConsoleRbacCheck and VirtualDeviceHardeningCheck wirings in vm.yaml, then remove every remaining label ClassVar from security.py (catalog labels verified identical to baseline). security.py is now label-free. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move nim + BmHostStatusLog labels to YAML; drop ClassVars Add ssh/gpu/workload/slow traits to the NIM wirings in bare_metal.yaml and vm.yaml and remove the nim.py ClassVars; remove BmHostStatusLog's ClassVar (bare_metal already supplied by its bare_metal.yaml wiring). Catalog labels unchanged. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): move on-host bm_* labels to provider configs; drop ClassVars The on-host bm_* checks (driver/cuda/gpu) are wired only in the k3s, microk8s, and minikube provider configs, so their labels belong there. Add bare_metal / bare_metal+gpu labels to those wirings, extend the catalog's label scan to read all config YAMLs (not just suites) so provider-declared labels are reflected, and remove the ClassVars from bm_cuda.py, bm_gpu.py, and bm_driver.py's BmDriverVersion. Catalog labels/platforms verified unchanged. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): remove the last label ClassVars; labels now live entirely in YAML Drives the codebase to zero `labels: ClassVar` declarations: - Drop the base-class defaults (BaseValidation.labels, BaseWorkloadCheck.labels); get_validation_labels reads via getattr with a () fallback. - Wire BmDriverInstalled into the k3s/microk8s/minikube provider configs and drop its ClassVar (it joins the other on-host bm_* checks). - Drop the vestigial labels from cluster.py (all four are catalog_exclude=True and superseded by the K8s* checks, so the labels were never read). - Update the unit-test fixtures (test_catalog/test_resolution/test_validation) to supply labels via the YAML wiring / dynamic attribute instead of a ClassVar, and refresh the AGENTS.md description. Full catalog labels verified byte-identical to the pre-migration baseline; coverage --check, make test, and make demo-test all pass. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * experiment(test-plan): map delete_tenant to CP-XX-10; mark generic/on-host checks N/A Closes the remaining test_id gaps so every wired check is explicitly accounted for (a real plan id or an intentional N/A): - StepSuccessCheck-delete_tenant -> CP-XX-10 ("Delete Tenant"), which was previously uncovered (plan coverage 128 -> 129). - StepSuccessCheck-delete_access_key -> N/A (no dedicated delete-access-key plan item; lifecycle is CP-XX-05 create / CP-XX-06 disable). - InstanceStateCheck -> N/A across the vm/bare_metal/image-registry suites (generic state helper), replacing the prior inline comments with a machine-readable sentinel. - The six on-host Bm* NVIDIA-stack checks -> N/A in the k3s/minikube/microk8s provider configs (BMAAS-XX-08 is already covered by the suite's GpuCheck). Guardrail (test_plan_coverage.py --check), make test, and make demo-test remain green. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * chore: cleanup Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(k8s): cover node-pool delete (K8S06-03) Wire a K8sNodePoolCheck for the delete leg of node-pool CRUD. Because validations run only after every step in a phase, a delete-to-zero check cannot share a phase with anything that expects the pool present. Add a throwaway CPU pool (own Terraform state) created in setup and deleted in the test phase, then assert it converges to zero nodes; a teardown safety-net destroy covers setup-only runs. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * docs: update docstring Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(isvtest): drop dead test_ids class metadata Test-plan IDs are sourced from the per-check YAML wiring; the catalog never reads the BaseValidation.test_ids ClassVar and get_validation_test_ids had no callers. Remove both now that no validation class declares test_ids. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(catalog): list omitted unreleased tests under -v Log the names of unreleased tests dropped from the catalog at DEBUG (the count stays at INFO). Also fix setup_logging so -v reaches isvtest.* logs: isvtest installs its own "isvtest" logger with propagate=False, so the root basicConfig never affected it; mirror the level onto that logger and its handlers. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(validation): migrate label handling to YAML wiring This update removes the reliance on class-level labels in validation classes, consolidating label management within the YAML configuration files. The `get_validation_labels` function has been eliminated, and all references to class-level labels have been replaced with a new approach that sources labels directly from the YAML wiring. This change enhances consistency and simplifies the codebase by ensuring that labels are uniformly managed across the system. Additionally, the documentation has been updated to reflect these changes, and various functions have been adjusted to accommodate the new label sourcing method. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): enhance coverage reporting and documentation Updated the test-plan coverage reporting to include gaps for uncovered test IDs, improving visibility into unimplemented checks. The documentation has been revised to reflect the new terminology, replacing references to class-level `test_ids` with wired `test_id`s. Additionally, the summary output now includes a detailed breakdown of uncovered plan items, enhancing the clarity of the coverage report. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * fix: use proper test id Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(validation): finish moving labels and test_ids to YAML wiring Remove the last Python label ClassVars and declare labels plus test-plan test_ids on the suite check wiring for capacity, bare metal, network, and multi-cluster validations. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * feat(validation): enforce suite check test_id and labels via pre-commit Add validate_suite_wiring.py with make validate-suites and a pre-commit hook so every wired check in isvctl/configs/suites declares test_id and labels. Errors include the repo-relative path and line number; backfill missing metadata across suite YAML files. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * chore(ci): move test-plan guardrails to pre-commit Drop the standalone test-plan CI job; plan-coverage now runs as a pre-commit hook alongside validate-suites. Normalize local hook names to lowercase "check …" wording. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * perf(test-plan): skip catalog build for coverage guardrails Use YAML-only wiring seeds for plan-coverage --check so pre-commit avoids discover_all_tests while reports keep the catalog-backed merge path. Add an equivalence test to keep both paths aligned. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): dedupe variant-union logic in coverage apply_config_labels and apply_config_test_ids were ~80% identical copy-paste. Extract the shared base-union + variant-propagation body into _apply_variant_union(entries, value_map, attr); both are now thin wrappers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * fix(test-plan): reject duplicate plan test_ids; correct --check help load_plan silently let the last entry win when a test_id was repeated in docs/test-plan.yaml, corrupting every derived report. Fail loudly instead, listing the offending ids. Also drop the stale "completeness" claim from --check help (the guardrails only enforce integrity and consistency). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * refactor(test-plan): drop issue status icons from adoc render GitHub's AsciiDoc preview does not render icon: macros. Emit plain issue links only and remove the live gh graphql fetch from make plan. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * fix(validation): address CodeRabbit review on suite wiring guardrails Surface suite YAML parse failures, require labels as lists, document deferred imports in plan-coverage, and add regression tests. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * style: ruff-format test_plan_yaml_to_adoc.py Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * fix: align catalog scans and issue refs Tighten GitHub issue rendering to the bare-reference contract and include direct dict validation wiring in catalog config scans. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> * fix(k8s): remove stray EKS provider wiring Drop the AWS EKS delete-node-pool wiring from this metadata branch; the change is preserved separately on backup/eks-yaml-node-pool-delete. Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> --------- Signed-off-by: Alexandre Begnoche <abegnoche@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Summary
New validations
ByoipCheckStablePrivateIpCheckFloatingIpCheckLocalizedDnsCheckVpcPeeringCheckFiles added (15 new)
isvctl/configs/stubs/aws/network/isvctl/configs/stubs/network/output_schemas.pyFiles modified (4)
isvtest/src/isvtest/validations/network.py— 5 new validation classesisvctl/src/isvctl/config/output_schemas.py— 5 new schemas + mappingsisvctl/configs/tests/network.yaml— canonical config with new steps +sdnvalidation groupisvctl/configs/providers/aws/network.yaml— AWS provider config with new stepsTest plan
make test— all 471 tests pass (232 isvctl + 84 isvreporter + 155 isvtest), no warningsmake lint— cleanuv run isvctl test run -f isvctl/configs/providers/aws/network.yaml -- -v -sMade with Cursor