nd_interface_loopback module by allenrobel · Pull Request #220 · CiscoDevNet/ansible-nd

allenrobel · 2026-04-03T20:27:34Z

Summary

Adds the nd_interface_loopback Ansible module for managing loopback interfaces on Nexus Dashboard 4.2
Fixes state:overridden incorrectly attempting to delete system-provisioned loopback interfaces (Loopback0/Loopback1 with policyType: "underlayLoopback")
Adds shared AsciiDescription Annotated type to reject non-ASCII interface descriptions client-side (applied to LoopbackPolicyModel.description)
Adds integration test suite covering all four states (merged, replaced, overridden, deleted)
Adds dynamic inventory generator (tests/integration/inventory.py) for running integration tests against live ND testbeds
Adds unit tests for manage_interfaces endpoint classes
File-based logging integrated via setup_logging() in main(). Module-level nd.nd_interface_loopback logger emits debug records at manage_state begin/end, and log.exception is called from both the NDStateMachineError handler and the broad Exception handler so tracebacks reach the log file regardless of output_level. Logging activates only when ND_LOGGING_CONFIG points at a logging.config.dictConfig JSON file; otherwise the calls are no-ops. The test target's tasks/main.yaml wraps its include_tasks in a block that forwards nd_logging_config (set in inventory.networking [nd:vars]) into the module subprocess as ND_LOGGING_CONFIG, since ansible-test strips the controller's shell environment.

Merge & rebase note

This PR is part of a stack (develop → nd_interface_loopback → nd_interface_ethernet_access → nd_interface_ethernet_trunk). Because the team uses squash merges, after this PR is merged, @allenrobel will rebase the dependent branches onto develop before the next PR (#222) can be merged. Please ping him once this is squashed so he can rebase.

Details

System loopback filtering fix

query_all() in LoopbackInterfaceOrchestrator previously filtered only by interfaceType == "loopback", which included system-provisioned loopbacks that ND creates during switch role configuration. These interfaces have policyType: "underlayLoopback" and cannot be deleted.

The fix adds a second filter that checks policyType against the set of policy types this module manages (loopback, ipfmLoopback, userDefined). System loopbacks are now excluded from the before collection, ensuring accurate changed flags and no unnecessary API calls.

Non-ASCII description rejection (`AsciiDescription` type)

Adds plugins/module_utils/models/types.py with a shared AsciiDescription Annotated type (str | None + AfterValidator(ascii_only)). The Cisco backend pipes interface descriptions through CLI generators that return a generic 500 ("unexpected error during policy execution") when the description contains non-ASCII characters (em-dash, smart quotes, emoji, latin-1, etc.). AsciiDescription catches this client-side with a clear validation error before the request is sent.

LoopbackPolicyModel.description now uses AsciiDescription while keeping its min_length=1, max_length=254 constraints intact via Field(...). Parametrized tests cover em-dash, smart quotes, emoji, and latin-1 rejection alongside ASCII passthrough. The same type is reused by the ethernet_access, ethernet_trunk_host, and svi policy models in the stacked PRs.

Integration tests

Test coverage organized into per-state task files:

Merged: create single/multiple, idempotency, update, deploy: false
Replaced: full replace, idempotency, multi-interface replace
Overridden: reduce to single, idempotency, system loopback filtering validation, swap interfaces
Deleted: single/multi delete, idempotency, non-existent interface

Dynamic inventory generator

tests/integration/inventory.py reads ND connection params from environment variables and generates a static inventory.networking for ansible-test, which sanitizes the subprocess environment. Also works directly as a dynamic inventory with ansible-playbook -i.

Test plan

All 365 unit tests pass (python -m pytest tests/unit/)
All integration tests pass against live ND 4.2 testbed via ansible-test network-integration nd_interface_loopback
System loopback filtering verified: Loopback0/Loopback1 excluded from before collection in overridden state
Peer review
To enable file-based logging during an integration run, add nd_logging_config=/absolute/path/to/logging_config.json to [nd:vars] in tests/integration/inventory.networking. An example config conforming to logging.config.dictConfig is documented in plugins/module_utils/common/log.py.

🤖 Generated with Claude Code

Copilot

Pull request overview

Adds a new nd_interface_loopback Ansible module (and supporting endpoint/models/orchestrators) to manage Nexus Dashboard loopback interfaces, including a fix to prevent state: overridden from trying to delete ND system-provisioned loopbacks.

Changes:

Introduces loopback interface Pydantic models, a loopback orchestrator, and manage-interfaces endpoint models to support CRUD + bulk deploy/remove.
Updates overridden-state behavior by filtering query_all() results to only user-managed loopback policyType values (excluding underlayLoopback system loopbacks).
Adds unit tests + integration test target (all states) and an inventory generator for running integration tests against live ND testbeds.

Reviewed changes

Copilot reviewed 17 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`plugins/modules/nd_interface_loopback.py`	New module entrypoint wiring NDStateMachine to the loopback orchestrator + deploy batching.
`plugins/module_utils/models/interfaces/loopback_interface.py`	New Pydantic models for loopback payload/config/response normalization and argument_spec.
`plugins/module_utils/orchestrators/base_interface.py`	New base orchestrator for interface lifecycle (queue deploy/remove, FabricContext resolution).
`plugins/module_utils/orchestrators/loopback_interface.py`	New loopback orchestrator with policyType filtering to exclude system loopbacks.
`plugins/module_utils/endpoints/v1/manage/manage_interfaces.py`	New endpoint models for manage-interfaces CRUD + interfaceActions deploy/remove.
`plugins/module_utils/endpoints/mixins.py`	Adds `InterfaceNameMixin` used by manage-interfaces endpoints.
`plugins/module_utils/fabric_context.py`	Adds FabricContext used by interface orchestrators for fabric validation + switch resolution.
`plugins/module_utils/endpoints/v1/manage/manage_switches.py`	Adds/updates manage-switches endpoint used to build the switch IP→switchId map.
`tests/unit/module_utils/models/test_loopback_interface.py`	Unit tests for loopback models and serialization/normalization behavior.
`tests/unit/module_utils/endpoints/test_endpoints_api_v1_manage_interfaces.py`	Unit tests for manage-interfaces endpoint path/verb behavior.
`tests/integration/targets/nd_interface_loopback/**`	Integration tests for merged/replaced/overridden/deleted states and shared vars.
`tests/integration/inventory.py`	Inventory generator / dynamic inventory helper for live ND testbeds.

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

1. Team decided not to allow switch level deploy in resource modules. - Removed deploy_type parameter and use only interface-level deploy endpoint. - Updated module documentation 2. Leverage bulk endpoint for interface deletion. POST …interfaceActions/remove 3. Remove single-interface DELETE endpoint DELETE …interfaces/{interface_name} 4. Add initial unit tests for nd_interface_loopback

… interfaces query_all() in LoopbackInterfaceOrchestrator previously filtered interfaces only by interfaceType == "loopback", which included system-provisioned loopbacks (Loopback0 routing, Loopback1 VTEP) that ND creates during initial switch role configuration. These have policyType "underlayLoopback" and are not deletable. When state:overridden was used, the module saw these system loopbacks in the before collection, identified them as "not in proposed", and queued them for removal. ND silently rejected the delete requests (the interfaces have deletable: false), but the module incorrectly reported changed=true and made unnecessary API calls. The fix adds a second filter stage to query_all() that checks each loopback's configData.networkOS.policy.policyType against the set of policy types this module manages (loopback, ipfmLoopback, userDefined). System loopbacks with policyType "underlayLoopback" are now excluded from the before collection. This ensures: - Accurate changed flag (no false positives from undeletable interfaces) - No unnecessary remove API calls for system-managed interfaces - The module correctly understands its own management scope Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Integration test suite covering all four states (merged, replaced, overridden, deleted) for the nd_interface_loopback module. Tests are organized into separate task files per state for easier debugging and selective execution. Test coverage includes: - Merged: create single/multiple, idempotency, update, deploy=false - Replaced: full replace, idempotency, multi-interface replace - Overridden: reduce to single, idempotency, system loopback filtering, swap interfaces in one pass - Deleted: single/multi delete, idempotency, non-existent interface The overridden tests include a dedicated system loopback filtering test that verifies Loopback0/Loopback1 (policyType "underlayLoopback") do not appear in the before collection and are not targeted for deletion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The ansible-test sanity no-assert check disallows bare assert statements in module code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Match base class Optional type for create_bulk_endpoint and delete_bulk_endpoint. Add pyright ignore for the guarded callsite where the @requires_bulk_support decorator ensures non-None at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

We no longer need the following, so removing. from __future__ import absolute_import, division, print_functtion __metaclass__ = type

Move _required() and environ.get() calls from field defaults into __post_init__() so env var validation happens at instantiation, not at class definition time. This fixes --host mode which should return {} without requiring env vars. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Use 'nd' as the inventory hostname and set ansible_host in _meta.hostvars, matching the INI inventory format. Previously the raw IP was used as the hostname with no ansible_host set, which caused playbooks to fail when ansible_host was undefined. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The docstring and module hierarchy comment incorrectly described a single identifier (interface_name) when the model actually uses a composite identifier (switch_ip, interface_name). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove incorrect claim that None is "excluded by exclude_none" when the test actually dumps without exclude_none and asserts the key is present with value None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace bare [] defaults for _pending_deploys and _pending_removes with Field(default_factory=list) to ensure each orchestrator instance gets its own queue and avoids shared state across instances. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Pydantic disallows Field() on names with leading underscores. Move _pending_deploys and _pending_removes initialization into model_post_init() so each instance gets its own fresh list without triggering the Pydantic naming restriction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The module only caught NDStateMachineError, leaving AssertionError and other runtime exceptions to produce unhandled tracebacks. Add a fallback except Exception handler so all failures are consistently reported via module.fail_json. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Follow-up to PR #250 (NDA-26), which replaced NDModule with RestSend in the orchestrator framework. The interface-module infrastructure still referenced self.sender and broke at runtime (AttributeError on FabricContext / base_interface / loopback_interface). - fabric_context.py: accept rest_send instead of sender; add _query_get helper that commits via RestSend and treats HTTP 404 as empty dict, preserving prior query_obj(ignore_not_found_error=True) semantics. - base_interface.py: read fabric_name from rest_send.params, build FabricContext with rest_send, and route deploy/remove through self._request. - loopback_interface.py: route all CRUD and query calls through self._request; use not_found_ok for query_all. - nd_state_machine.py: pass all module params to RestSend so downstream orchestrator code can read fabric_name. - Add tests/unit/module_utils/test_fabric_context.py with six tests covering fabric summary 200/404, switch_map parsing, get_switch_id failure, and validate_for_mutation success/failure. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Drop fabric_is_local and fabric_is_read_only from validate_for_mutation since both are stubs that silently pass. The previous behavior advertised three pre-flight checks but only enforced one, which is misleading for callers relying on the safety net. The class-level docstring records why they are excluded and how to re-enable them once the underlying API fields are identified. Add a switch_map_by_id property and get_switch_ip helper, mirroring the existing IP-keyed surface. Both maps are now populated from a single fetch via _load_switch_maps, with .get guards so a switch missing either fabricManagementIp or switchId is skipped instead of raising. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Resets the fabric summary and both switch maps so the next property access re-fetches from the API. Intended for callers that mutate the fabric and need subsequent reads to see the new state without reconstructing a FabricContext. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Migrate to builtin generics and union syntax in files introduced on this branch: Optional[X] -> X | None, List[X] -> list[X], Dict -> dict, Set -> set, Type -> type. Drop the now-unused imports from typing. Also: - Add `from __future__ import annotations` to fabric_context.py and loopback_interface.py so sanity pylint tolerates PEP 604 union syntax under older py-version settings. - Convert two `assert` type-narrowing checks in fabric_context.py to explicit `if ... raise AssertionError(...)` to satisfy the no-assert sanity rule. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Wire setup_logging() into the module's main() and emit debug records at manage_state begin/end. log.exception is called from both the NDStateMachineError handler and the broad Exception handler, so tracebacks reach the log file regardless of output_level. Logging activates only when ND_LOGGING_CONFIG points at a logging.config JSON file; otherwise the calls are no-ops. ansible-test strips the controller's shell environment, so the integration test's main.yaml now wraps its include_tasks in a block that forwards nd_logging_config (set in inventory.networking [nd:vars]) into the module subprocess as ND_LOGGING_CONFIG. Added a header comment documenting the variable.

Adds plugins/module_utils/models/types.py with a shared `AsciiDescription` Annotated type (`str | None` + `AfterValidator(ascii_only)`). Catches non-ASCII descriptions client-side instead of letting the Cisco backend return a generic 500 ("unexpected error during policy execution") from the CLI generator. `LoopbackPolicyModel.description` now uses `AsciiDescription` while keeping `min_length=1, max_length=254` constraints intact via the `Field(...)` annotation. Adds parametrized tests covering em-dash, smart quotes, emoji, and latin-1 rejection alongside ASCII passthrough. Same change will follow on ethernet_access, ethernet_trunk_host, and svi in the stacked PRs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Adds EpManageFabricsDeploymentFreezeGet endpoint model and wires fabric_is_deployment_frozen() into FabricContext.validate_for_mutation so orchestrators surface a clear error when the fabric is in deployment freeze mode and configuration changes cannot be deployed to switches. Result is lazily cached and reset on invalidate(). Includes endpoint and FabricContext unit tests with matching fixtures. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Optional[str] replaces str | None in the Annotated[] runtime expression; from __future__ import annotations does not protect runtime positions from pylint's unsupported-binary-operation check when py-version is unset. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Replace literal Unicode smart quotes with “/” escape sequences so the source file contains only ASCII, matching the pattern already used for emoji and latin-1 test data on nearby lines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ND 4.2 returns routeMapTag as an integer despite the template defining it as a string; same drift affects SVI routingTag. The TODO marker lets a future grep find both coercions for removal once ND 4.3 ships. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Remove black line

Black expects two blank lines between methods.

Add module docstring, disable wrong-import-position for the post-DOCUMENTATION imports, and disable broad-except on the top-level fallback handler in main(). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Promote the loopback `ip` / `ipv6` field validators into shared `IPv4CIDR` / `IPv6CIDR` Annotated types in `models/types.py`, alongside the existing `AsciiDescription`. Other interface modules (SVI, ethernet, port-channel) need the same CIDR check and can now reuse these types instead of re-implementing the validator. `LoopbackPolicyModel` drops the inline `@field_validator` methods and the `import ipaddress`; field annotations carry the validation. Test docstrings updated to reference the new types. Behavior unchanged -- all 83 loopback model tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Pylance flagged `result.get("interfaces", ...)` on the `_request()` return value because `ResponseType` is `Union[List, Dict, None]`. Replace the truthiness check with `isinstance(result, dict)` to narrow the type; the manage_interfaces GET-by-switch endpoint always returns a dict. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Replace verbatim-repeated module argument blocks (check / normal / idempotent runs of the same call) with YAML anchors. Removes a class of typo where one of the duplicated blocks could drift from its siblings. Anchors are defined at first use within each section to keep them local to the reader.

Move the top-of-file SETUP block from merged.yaml (delete loopback100/101/102) into a dedicated tasks/setup.yaml and call it from main.yaml before the state-test blocks. Makes the orchestration explicit and keeps merged.yaml focused on its state. Intra-test setup that's coupled to specific tests stays inline: - loopback103 cleanup after the no-deploy test in merged.yaml - Recreate loopback100/101 before the multi-delete test in deleted.yaml

allenrobel requested review from akinross, anvitha-jain, gmicol, lhercot, mikewiebe, mtarking, sajagana, samiib and shrsr as code owners April 3, 2026 20:27

allenrobel changed the title ~~Add nd_interface_loopback module with integration tests~~ Add nd_interface_loopback module with integration tests (WIP) Apr 3, 2026

allenrobel changed the title ~~Add nd_interface_loopback module with integration tests (WIP)~~ Add nd_interface_loopback module with integration tests Apr 3, 2026

allenrobel changed the title ~~Add nd_interface_loopback module with integration tests~~ Add nd_interface_loopback module with integration tests (WIP) Apr 6, 2026

allenrobel marked this pull request as draft April 6, 2026 23:55

allenrobel self-assigned this Apr 7, 2026

allenrobel changed the title ~~Add nd_interface_loopback module with integration tests (WIP)~~ Add nd_interface_loopback module (WIP) Apr 7, 2026

allenrobel changed the title ~~Add nd_interface_loopback module (WIP)~~ nd_interface_loopback module (WIP) Apr 7, 2026

Base automatically changed from nd42_integration to develop April 9, 2026 15:41

allenrobel force-pushed the nd_interface_loopback branch 2 times, most recently from 4b2453f to f5ee201 Compare April 10, 2026 21:33

allenrobel changed the title ~~nd_interface_loopback module (WIP)~~ nd_interface_loopback module Apr 15, 2026

allenrobel marked this pull request as ready for review April 15, 2026 21:16

Copilot AI review requested due to automatic review settings April 15, 2026 21:16

Copilot started reviewing on behalf of allenrobel April 15, 2026 21:16 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

allenrobel and others added 5 commits April 21, 2026 12:26

Add InterfaceNameMixin

d1be95a

nd_loopback_interface initial commit

f143f03

allenrobel and others added 10 commits April 21, 2026 12:26

Replace bare assert with raise AssertionError for ansible-test sanity

1d40f83

The ansible-test sanity no-assert check disallows bare assert statements in module code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove unneeded boilerplate

5f04634

We no longer need the following, so removing. from __future__ import absolute_import, division, print_functtion __metaclass__ = type

Fix misleading docstring in test_loopback_interface_00052

5ce232c

Remove incorrect claim that None is "excluded by exclude_none" when the test actually dumps without exclude_none and asserts the key is present with value None. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

allenrobel force-pushed the nd_interface_loopback branch from abc59bc to 78a5554 Compare April 21, 2026 22:27

allenrobel and others added 19 commits April 21, 2026 15:31

Add logging usage to integration tests main.yaml

43035f4

Appease black linter

0f340ef

Appease black linter

38e0344

Remove black line

Appease black linter

7be0cea

Black expects two blank lines between methods.

Appease pylint in nd_interface_loopback

09ab453

Add module docstring, disable wrong-import-position for the post-DOCUMENTATION imports, and disable broad-except on the top-level fallback handler in main(). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nd_interface_loopback module#220

nd_interface_loopback module#220
allenrobel wants to merge 48 commits intodevelopfrom
nd_interface_loopback

allenrobel commented Apr 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Summary

Merge & rebase note

Details

System loopback filtering fix

Non-ASCII description rejection (AsciiDescription type)

Integration tests

Dynamic inventory generator

Test plan

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!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

allenrobel commented Apr 3, 2026 •

edited

Loading

Non-ASCII description rejection (`AsciiDescription` type)