interface capability preflight (#273) by allenrobel · Pull Request #275 · CiscoDevNet/ansible-nd

allenrobel · 2026-05-11T23:50:38Z

Related Issue(s)

Proposed Changes

Pre-flights ND's unpublished /api/v1/manage/fabrics/{fabric}/capableSwitches endpoint before per-switch configuration calls, converting confusing HTTP 400s on incapable switches into a single aggregate error that names the offending switches.

Endpoint model EpManageFabricsCapableSwitchesGet, inheriting _EpManageFabricsBase for path construction, with required interfaceType / mode query params.
InterfaceCapabilityPreflight class (plugins/module_utils/interface_capability_preflight.py) mirroring FabricContext shape:
- (interface_type, mode) cache (switch records + derived switch-id set); one GET per pair per task run
- Client-side taxonomy enforcement (captured from the ND 4.2 GUI 2026-05-11) — invalid combos raise ValueError without consuming a round trip
- Optional FabricContext injection enriches error messages with switch_ip
- invalidate() for cache reset
- Treats HTTP 404 from capableSwitches as a hard failure (rather than caching an empty capable set that would mislabel every requested switch); returns defensive copies from get_capable_switches / get_capable_switch_ids so callers can't corrupt cached state.
Orchestrator wiring on NDBaseInterfaceOrchestrator:
- interface_type / interface_mode ClassVars (both default "") — subclasses opt in by setting both to a valid taxonomy pair
- Lazy capability_preflight property
- validate_switches_capable(model_instances) — resolves the target switch-id set, issues a single capableSwitches GET, and raises one RuntimeError naming every incapable switch; also raises a clear RuntimeError if a subclass sets interface_type without interface_mode. Parameter typed Sequence[ModelType] so static analysis rejects generator callers (commit 0157f2f, fixes NDBaseInterfaceOrchestrator.validate_switches_capable consumes its input; bulk callers iterate it twice #300). Resolves all switch_ip values up front and aggregates unresolvable ones into a single RuntimeError so a typo on one entry no longer masks problems on the rest (commit 8835915, fixes validate_switches_capable: a single unresolvable switch_ip masks capability problems on other switches #301). In --check mode, downgrades capability-endpoint failures and capability mismatches to warnings via AnsibleModule.warn so dry-runs stay green when the unpublished endpoint is unavailable (commit 215601a, fixes Capability preflight issues a live GET in --check mode and can fail dry-runs that previously succeeded #302).
Loopback as reference impl — interface_type = "loopback", interface_mode = "managed"; preflight invoked from create, update, create_bulk. Delete and read paths intentionally skip preflight.

Taxonomy embedded in `InterfaceCapabilityPreflight` docstring

interface_type	valid modes
`loopback`	`managed`
`svi`	`managed`
`tunnel`	`managed`
`ethernet`	`trunk`, `access`, `routed`, `fex`, `pvlan`, `dot1qTunnel`, `unmanaged`
`portChannel`	`trunk`, `access`, `routed`, `fex`, `pvlan`, `dot1qTunnel`, `unmanaged`

VPC uses a different endpoint (/vpcPairs?view=intendedPairs) and is handled separately (follow-up). Breakout and subinterface are out of scope (no pre-check available / absent from taxonomy).

Impact on the merged `nd_interface_loopback` module

nd_interface_loopback is already merged to develop, so this PR is not purely additive — it modifies files owned by that merged module to wire loopback in as the reference implementation:

plugins/module_utils/orchestrators/loopback_interface.py — adds the interface_type / interface_mode ClassVars and the validate_switches_capable calls in create / update / create_bulk.
tests/unit/module_utils/orchestrators/test_loopback_interface.py — each mutating test now consumes one capableSwitches GET.
tests/unit/module_utils/fixtures/fixture_data/test_loopback_interface.json — adds the capableSwitches fixture responses.

No change to plugins/modules/nd_interface_loopback.py, the loopback model, or loopback's user-facing behavior — preflight is internal and runs ahead of the existing API calls. These three files are the "modifies already-merged code" surface for review; anyone with an in-flight branch off loopback should expect a small merge touch-point here.

Code-review follow-ups resolved in this PR

A high-effort /code-review pass on this branch surfaced three additional findings whose fixes touch files originating in develop (base_interface.py, loopback_interface.py, both from PR #220). All three are now resolved in this PR rather than deferred:

NDBaseInterfaceOrchestrator.validate_switches_capable consumes its input; bulk callers iterate it twice #300 — Resolved (commit 0157f2f). validate_switches_capable accepted Iterable but bulk callers iterate the same parameter twice; a generator caller would have become a silent no-op. Narrowed the base-class signature to Sequence[ModelType] so mypy rejects generator callers at the boundary. The method body and all existing call sites (list[...]) are unchanged.
validate_switches_capable: a single unresolvable switch_ip masks capability problems on other switches #301 — Resolved (commit 8835915). A single unresolvable switch_ip masked aggregate capability problems for the rest of the batch. Now resolves every switch_ip up front, collects the unresolvable ones, and raises a single aggregate RuntimeError naming every unknown IP; the capability GET only runs on the resolved subset.
Capability preflight issues a live GET in --check mode and can fail dry-runs that previously succeeded #302 — Resolved (commit 215601a). Capability preflight issues a live GET in --check mode; a 404 (post-Add interface capability pre-flight check as a shared class #273-merge) would now fail a dry-run that previously succeeded. In --check mode, capability-endpoint failures and capability mismatches are downgraded to warnings via AnsibleModule.warn so dry-runs stay green and the information still surfaces. Unresolvable switch_ip values still raise even in check mode because they reflect user input errors.

The two in-branch findings — 404 silently cached as empty capable set, and cache returned by reference — are fixed in interface_capability_preflight.py (commit 7a130c4).

Adopting preflight in downstream interface modules

Once this lands, each new interface orchestrator (svi, ethernet access / trunk-host, port-channel, ...) opts in with no boilerplate beyond two ClassVars:

Inherit NDBaseInterfaceOrchestrator — already standard for interface orchestrators; nothing new.
Set both ClassVars to a valid taxonomy pair from the table above:
- interface_type: ClassVar[str] = "..." (e.g. "ethernet", "portChannel", "svi")
- interface_mode: ClassVar[str] = "..." (e.g. "access", "trunk", "routed", "managed")
One orchestrator = one (interface_type, interface_mode) pair; per-mode variants (e.g. ethernet access vs. trunk-host) each set their own interface_mode.
Call validate_switches_capable(...) at the top of create, update, and create_bulk, before resolving switch IDs or building payloads — [model_instance] for the single-instance methods, the full list for create_bulk. Skip it in delete and query paths (matching loopback).
Nothing else to override. _resolve_mode() no longer exists — mode is the interface_mode ClassVar. If a subclass sets interface_type but omits interface_mode, validate_switches_capable raises a clear RuntimeError naming the class.
Opt out by leaving interface_type as "" (the base default) — for interface types with no capableSwitches coverage (breakout, subinterface).
VPC is separate — vpc orchestrators use a different endpoint (/vpcPairs?view=intendedPairs) and must not set these ClassVars; vpc preflight is a follow-up.
Tests — every mutating test gains one capableSwitches GET response immediately after the switches-list response; test_loopback_interface.json shows the fixture shape.

Test Notes

50 new unit tests for the preflight feature pass:
- tests/unit/module_utils/endpoints/test_endpoints_api_v1_manage_fabric_capable_switches.py — 9 tests
- tests/unit/module_utils/test_interface_capability_preflight.py — 35 tests (incl. 24 parametrized taxonomy cases, plus 404-failure and defensive-copy tests added in commit 7a130c4)
- tests/unit/module_utils/orchestrators/test_base_interface.py — 6 tests covering validate_switches_capable: opted-out no-op, half-configured RuntimeError, aggregate unresolved-IP error (validate_switches_capable: a single unresolvable switch_ip masks capability problems on other switches #301), check-mode soften on endpoint failure (Capability preflight issues a live GET in --check mode and can fail dry-runs that previously succeeded #302), check-mode soften on capability mismatch (Capability preflight issues a live GET in --check mode and can fail dry-runs that previously succeeded #302), and the regression guard ensuring mismatches still raise outside check mode (Capability preflight issues a live GET in --check mode and can fail dry-runs that previously succeeded #302)
test_loopback_interface.py updated for the new preflight GET: each mutating test (create / update / create_bulk) now consumes one capableSwitches response after the switches-list. Loopback suite green at 21/21.
MockAnsibleModule gains a warn() method that records to self.warnings so the check-mode soften tests can assert on emitted warnings.
Full local unit suite: 586 passed, no regressions
ansible-test sanity --docker passes on all changed files (compile / import / pep8 / pylint / validate-modules / empty-init / yamllint / ... across Python 3.8–3.13)
black + isort clean
Integration smoke against the ND 4.2 lab is deferred to a follow-up — depends on having a reproducer for an incapable switch.

Cisco Nexus Dashboard Version

4.2.1

Related ND API Resource Category

Checklist

Latest commit is rebased from develop with merge conflicts resolved
New or updates to documentation has been made accordingly
Assigned the proper reviewers

🤖 Generated with Claude Code

Copilot

Pull request overview

Adds an interface-capability “preflight” layer to the collection so interface orchestrators can query ND’s unpublished .../capableSwitches endpoint up-front and fail with a single aggregated, switch-specific error instead of scattered per-switch HTTP 400s.

Changes:

Introduces a new endpoint model for GET /api/v1/manage/fabrics/{fabric}/capableSwitches with query params (interfaceType, mode) and unit tests.
Adds InterfaceCapabilityPreflight (taxonomy enforcement + per-(interface_type, mode) caching + validation/error aggregation) with fixture-backed unit tests.
Wires capability preflight into NDBaseInterfaceOrchestrator and enables it for the loopback orchestrator (create/update/create_bulk).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`plugins/module_utils/interface_capability_preflight.py`	Implements cached capability lookup + taxonomy enforcement + aggregate validation errors.
`plugins/module_utils/endpoints/v1/manage/manage_fabric_capable_switches.py`	Adds endpoint model for the unpublished `capableSwitches` resource.
`plugins/module_utils/orchestrators/base_interface.py`	Adds orchestrator-level preflight plumbing (`interface_type`, lazy preflight, grouping/aggregation).
`plugins/module_utils/orchestrators/loopback_interface.py`	Opts loopback orchestrator into preflight and calls it on create/update/bulk create.
`tests/unit/module_utils/test_interface_capability_preflight.py`	Unit tests for caching, taxonomy enforcement, and validation behavior.
`tests/unit/module_utils/fixtures/fixture_data/test_interface_capability_preflight.json`	Fixture responses for preflight unit tests.
`tests/unit/module_utils/endpoints/test_endpoints_api_v1_manage_fabric_capable_switches.py`	Unit tests for new endpoint params/path behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pre-flights ND's unpublished `/api/v1/manage/fabrics/{fabric}/capableSwitches` endpoint so we can fail fast with a single aggregate error naming offending switches instead of letting per-switch configuration calls return confusing HTTP 400s. - `EpManageFabricsCapableSwitchesGet` endpoint model with required `interfaceType` / `mode` query params - `InterfaceCapabilityPreflight` class with `(interface_type, mode)` cache, client-side taxonomy enforcement, and optional `FabricContext`-driven error enrichment - `NDBaseInterfaceOrchestrator.validate_switches_capable()` groups model instances by `(interface_type, mode)`, aggregates per-group errors into one `RuntimeError` - Loopback orchestrator opts in via `interface_type: ClassVar[str] = "loopback"` and calls the preflight from `create`, `update`, `create_bulk` - 42 new unit tests (9 endpoint, 33 preflight) including 24 parametrized taxonomy cases captured from the ND 4.2 GUI Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replace `Optional[str]` with `str | None` per CLAUDE.md modernization rule for new code; the legacy form leaked in from mirroring manage_fabrics.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

get_capable_switch_ids rebuilt the {switchId} set on every call. validate() runs once per interface, so for N interfaces the comprehension ran N times over the full fabric switch list (O(N*S)). Cache the derived set per (interface_type, mode) alongside the existing record cache, and clear it in invalidate() so the two caches stay consistent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

EpManageFabricsCapableSwitchesGet hand-rolled set_identifiers, the fabric_name check, and BasePath path-building -- all already provided by _EpManageFabricsBase, the same base used by the sibling EpManageFabricsSummaryGet. The class already declared the base-only _require_fabric_name / _path_suffix ClassVars, confirming the intent. Inherit _EpManageFabricsBase, drop the duplicated set_identifiers and _require_fabric_name, and reduce the path override to query-param validation plus a super().path delegation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

validate() hand-formatted the capableSwitches URL as an f-string, duplicating what the endpoint model already produces and risking drift if the path or query encoding ever changes. Add a _build_endpoint() helper as the single source of truth for the URL, used by both get_capable_switches() (live request) and validate() (the "Verify via GET ..." error hint). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The class, __init__, and validate docstrings claimed error messages were enriched with switch_ip / switch_name / model. The code only adds switch_ip (FabricContext exposes no switch-name/model map). Correct all three to describe what the code actually does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- get_capable_switches: drop the dead `if result else []` ternary; _query_get always returns a dict so `result.get("switches") or []` suffices. - validate: add a WHY comment explaining the soft switch_map_by_id.get() lookup (FabricContext.get_switch_ip() raises on miss). - _resolve_mode: add a comment explaining why getattr() is used -- ModelType is bound to NDBaseModel, which has no `mode`; the attribute is duck-typed. - base_interface: import Iterable from collections.abc per PEP 585. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`mode` is a per-interface-type constant (a frozen Literal nested under config_data) on every interface model -- loopback, svi, ethernet-access -- not per-instance data. The base _resolve_mode() default getattr(model_instance, "mode") matched zero real models and would raise AttributeError for any opted-in orchestrator whose model lacked a top-level `mode`. Promote `mode` to an `interface_mode` ClassVar alongside `interface_type`. validate_switches_capable() reads it directly and raises a clear RuntimeError when an orchestrator sets interface_type without interface_mode, enforcing the opt-in contract structurally rather than by docstring. The (interface_type, mode) grouping is dropped -- it only existed to handle a per-instance mode. Add base-level tests for validate_switches_capable (opted-out no-op; half-configured error); it had no coverage before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Treat HTTP 404 from the capableSwitches endpoint as a hard failure rather than silently caching an empty capable set, which would mislabel every requested switch as "not capable". Return defensive copies from get_capable_switches/get_capable_switch_ids so callers can't corrupt cached state. See #273. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Bulk callers (e.g. LoopbackInterfaceOrchestrator.create_bulk) iterate the same parameter a second time to build per-switch payload groups after validate_switches_capable has already walked it once. The Iterable[ModelType] signature openly invited a future caller to pass a generator/filter/map, which would exhaust the iterator on the first pass and silently turn the bulk create into a no-op POST chain while the state machine still reports changed=True. Narrow the parameter to Sequence[ModelType] so mypy catches generator callers at the call boundary. Today's only caller passes list[...], so behavior is unchanged. Fixes #300 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

`validate_switches_capable` resolved every `switch_ip` inside a single set comprehension, so the first unknown IP short-circuited the whole preflight before any capability check ran. A typo on one entry hid resolution and capability problems on the remaining entries, forcing a round-trip-per-error workflow. Resolve all `switch_ip` values up front, collect the unresolvable ones, and raise a single aggregate RuntimeError naming every unknown IP. The capability GET still runs on the resolved subset only after the resolution check passes. Fixes #301. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

`validate_switches_capable` issues a live GET against the unpublished `capableSwitches` endpoint. `RestSend.commit()` only short-circuits writes in `--check` mode, so this GET runs in dry-runs too. Once the 404-aware error handling treats endpoint failures as hard errors, a `--check` run on an ND release that has retired or relocated the endpoint will fail a dry-run that previously succeeded. In `--check` mode, catch `RuntimeError` from `capability_preflight.validate(...)` and downgrade it to a warning via `AnsibleModule.warn`. Endpoint outages and capability mismatches surface as informational output instead of failing the dry-run. Outside check mode, behavior is unchanged. Unresolvable `switch_ip` values still raise even in check mode because they reflect user input errors. Add `warn()` to `MockAnsibleModule` so tests can assert on emitted warnings. Fixes #302. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

samiib · 2026-06-01T07:02:08Z

+    - Via any method that takes `(interface_type, mode)` when the pair is not in the supported taxonomy.
+    """
+
+    _TAXONOMY: ClassVar[dict[str, frozenset[str]]] = {


Since this captured experimentally from an undocumented API, should we add the ability to allow unknown types and modes? Additional values could be supported in future versions but this validation would still block it.

samiib · 2026-06-01T07:07:00Z

+
+        None
+        """
+        self.rest_send.sender.ansible_module.warn(message)  # type: ignore[attr-defined]


Are there any contexts where ansible_module may not be defined? eg in tests?
Perhaps it would be good to use a getattr guard or a method in RestSend instead?

allenrobel requested review from akinross, anvitha-jain, gmicol, lhercot, mikewiebe, mtarking, sajagana, samiib and shrsr as code owners May 11, 2026 23:50

allenrobel self-assigned this May 12, 2026

allenrobel linked an issue May 12, 2026 that may be closed by this pull request

Add interface capability pre-flight check as a shared class #273

Open

6 tasks

allenrobel force-pushed the nd_interface_capability_preflight branch 2 times, most recently from 05d6436 to 6d822a3 Compare May 14, 2026 20:43

allenrobel changed the title ~~Add interface capability preflight (#273)~~ interface capability preflight (#273) May 18, 2026

allenrobel force-pushed the nd_interface_loopback branch from bff5d88 to 921294d Compare May 20, 2026 21:23

allenrobel force-pushed the nd_interface_capability_preflight branch from 6d822a3 to 7ca333a Compare May 20, 2026 21:23

Base automatically changed from nd_interface_loopback to develop May 21, 2026 13:25

allenrobel force-pushed the nd_interface_capability_preflight branch from 7ca333a to 930a991 Compare May 21, 2026 19:41

Copilot AI review requested due to automatic review settings May 21, 2026 21:59

Copilot started reviewing on behalf of allenrobel May 21, 2026 22:00 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread plugins/module_utils/orchestrators/loopback_interface.py

Comment thread plugins/module_utils/orchestrators/base_interface.py Outdated

Comment thread plugins/module_utils/endpoints/v1/manage/manage_fabric_capable_switches.py

allenrobel force-pushed the nd_interface_capability_preflight branch from a2f288a to 509cc8c Compare May 21, 2026 22:19

allenrobel and others added 8 commits May 28, 2026 07:33

Modernize CapableSwitchesEndpointParams annotations to PEP 604

9612418

Replace `Optional[str]` with `str | None` per CLAUDE.md modernization rule for new code; the legacy form leaked in from mirroring manage_fabrics.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

allenrobel force-pushed the nd_interface_capability_preflight branch from 806a39a to 4e4a362 Compare May 28, 2026 17:34

allenrobel and others added 4 commits May 29, 2026 14:02

samiib reviewed Jun 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

interface capability preflight (#273)#275

interface capability preflight (#273)#275
allenrobel wants to merge 12 commits into
developfrom
nd_interface_capability_preflight

allenrobel commented May 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samiib Jun 1, 2026

Uh oh!

samiib Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

allenrobel commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue(s)

Proposed Changes

Taxonomy embedded in InterfaceCapabilityPreflight docstring

Impact on the merged nd_interface_loopback module

Code-review follow-ups resolved in this PR

Adopting preflight in downstream interface modules

Test Notes

Cisco Nexus Dashboard Version

Related ND API Resource Category

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samiib Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

samiib Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

allenrobel commented May 11, 2026 •

edited

Loading

Taxonomy embedded in `InterfaceCapabilityPreflight` docstring

Impact on the merged `nd_interface_loopback` module