ND Manage VRFs & Networks: Add Internal Staged Workflow by AKDRG · Pull Request #509 · CiscoDevNet/ansible-nd

AKDRG · 2026-08-13T19:45:21Z

Related Issue(s)

Related to:

Proposed Changes

Add a private/internal _staged workflow state for nd_manage_vrfs and nd_manage_networks.

The new workflow supports staged configuration removal where omitted resources should have attachments detached, but the omitted VRFs or Networks themselves should not be deleted.

Key behavior:

_staged follows overridden attachment scope.
Current VRF/Network attachments are queried across the full current resource set.
Attachments omitted from desired config are detached.
Generic CRUD execution is mapped to replaced, preventing overridden-style delete requests.
Deployment is always suppressed for _staged.
Delete deploy payloads, pending deploy fallback, and delete-readiness wait flows are not invoked.
The implementation stays localized to the VRF/Network state machines and attachment managers.

Test Notes

Commands run:

PYTHONPATH=/Users/achengam/Documents/Ansible_Dev/NDNetwork_empty_interfaces
/Users/achengam/.pyenv/versions/ndfclab/bin/python
-m pytest tests/unit/module_utils/orchestrators/test_networks.py -k staged -q

Result:

2 passed, 80 deselected

PYTHONPATH=/Users/achengam/Documents/Ansible_Dev/NDNetwork_empty_interfaces
/Users/achengam/.pyenv/versions/ndfclab/bin/python
-m pytest tests/unit/module_utils/orchestrators/test_vrf_workflow_coordinator.py -k staged -q

Result:

2 passed, 54 deselected

black -l 159 --check
plugins/module_utils/orchestrators/network_attachment_manager.py
plugins/module_utils/orchestrators/network_state_machine.py
plugins/module_utils/orchestrators/vrf_attachment_manager.py
plugins/module_utils/orchestrators/vrf_state_machine.py
plugins/modules/nd_manage_networks.py
plugins/modules/nd_manage_vrfs.py
tests/unit/module_utils/orchestrators/test_networks.py
tests/unit/module_utils/orchestrators/test_vrf_workflow_coordinator.py

Result:

8 files would be left unchanged.

Cisco Nexus Dashboard Version

Workflow-layer change. No new API endpoint schema dependency introduced.

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

akinross · 2026-08-25T09:48:47Z

      - V(deleted) removes specified VRFs (or all if config is empty).
      - V(gathered) returns current VRF state (the only state allowed on child
        fabrics when targeted directly).
+      - V(staged) is an internal/private workflow state. It follows


Why is staged called “internal/private” while also being included in the module’s public choices?

Since "state" comes under the argspec, the choices for the state needs documentation else get's flagged by validate-module sanity.

did you consider exposing this "feature" through an undocumented environment variable that can be specified in at task level? this way the documentation would remain clean

@AKDRG just remove the usage of internal/private as last conversation on this was to fully document it as a state.

My bad, was thinking we needed to document it as an internal state. Amended the wordings now.

akinross · 2026-08-25T10:25:38Z

      - V(deleted) removes specified VRFs (or all if config is empty).
      - V(gathered) returns current VRF state (the only state allowed on child
        fabrics when targeted directly).
+      - V(staged) is an internal/private workflow state. It follows


Should we add example for this state with description what it is actually doing?

Since it's an private state and not intended for customer use cases, I guess we need not document examples.

@AKDRG customers can use it since we'll support it as a state. Original intention was undocumented state, but it's documented and in arg spec now.

Got it, fixed!

akinross · 2026-08-25T10:48:23Z

      - V(deleted) removes specified VRFs (or all if config is empty).
      - V(gathered) returns current VRF state (the only state allowed on child
        fabrics when targeted directly).
+      - V(staged) is an internal/private workflow state. It follows


I am a bit confused on what staged is supposed to be doing, could you clarify my overall understanding which might be wrong.

Should state: staged use full overridden attachment semantics while running definition CRUD as replaced and suppressing deployment, so that:

Removing an attachment from a VRF that remains in config submits an undeployed attach:false; the current code appears to skip this attachment removal.

Omitting an existing VRF entirely submits undeployed detach requests for its current attachments while retaining the VRF definition; the current code appears to handle this case as expected.

Adding an attachment to an existing VRF submits an undeployed attach:true; the current code appears to skip this attachment addition.

Creating a new VRF with attachments creates the VRF definition and submits undeployed attach:true requests; the current code appears to create the definition but skip its attachments.

It appears to me we that we do not test all of these scenarios but could have missed this.

Hey Akini,
Thanks a lot for your feedback and testing. You had identified a critical bug here. I have fixed the gaps that were left out due to my commit miss and we are fully covered now.

Removing an attachment from a VRF that remains in config submits an undeployed attach:false; the current code appears to skip this attachment removal.

Not skipped, attachment is removed with attach:false. Not un-deployed -> Staged.

Adding an attachment to an existing VRF submits an undeployed attach:true; the current code appears to skip this attachment addition.

Not skipped, attachment is posted with attach:true. Not deployed -> Staged.

Creating a new VRF with attachments creates the VRF definition and submits undeployed attach:true requests; the current code appears to create the definition but skip its attachments.

Not skipped, attachment is posted with attach:true. Not deployed -> Staged.

Added the integration tests too!

Thanks,
Akshay

allenrobel

Code review

Four findings, all on the staged workflow plumbing in vrf_state_machine.py (the first is a functional bug; the network-side twin of each is noted inline where it applies).

🤖 Generated with Claude Code

allenrobel · 2026-08-31T22:40:07Z

            omitted_vrf_names,
            attachment_details=omitted_attachment_details,
        )
+        if module_args.get("state") == "staged":


This staged short-circuit comes after the unconditional self.coordinator._ensure_vrfs_have_no_networks(...) call at the top of _prepare_overridden_deletions (line 378). That guard exists to "Fail before VRF deletion when networks still reference the VRFs" (vrf_dependency_checker.py::ensure_no_networks) and calls fail_json("Cannot delete VRF(s) because network(s) still reference them ...").

Since staged never deletes ("omitted VRFs are detached but not removed"), a staged task that omits a VRF still carrying networks hard-fails with a deletion error instead of performing the legitimate detach-only reconciliation. Suggest skipping the dependency check when module_args.get("state") == "staged", i.e. hoisting this guard above the _ensure_vrfs_have_no_networks call.

Note the new unit test (test_vrf_staged_detaches_omitted_vrfs_without_running_overridden_crud_delete) stubs _ensure_vrfs_have_no_networks as a call logger, so this path is not exercised by the added tests. The network twin (_ensure_networks_have_no_networks) is currently a no-op placeholder, so only the VRF side fails today — but the same reordering there would keep the two symmetric if that checker is ever implemented.

allenrobel · 2026-08-31T22:40:07Z

+    @staticmethod
+    def _crud_module_args(module_args: dict) -> dict:
+        """Return module args for the generic CRUD state machine."""
+        if module_args.get("state") != "staged":
+            return module_args
+        crud_args = dict(module_args)
+        crud_args["state"] = "replaced"
+        return crud_args
+
+    @staticmethod
+    def _query_module_args(module_args: dict) -> dict:
+        """Return module args for the current-state query phase."""
+        if module_args.get("state") != "staged":
+            return module_args
+        query_args = dict(module_args)
+        query_args["state"] = "overridden"
+        return query_args
+
+    @staticmethod
+    def _prepare_crud_state(sm: Any, requested_state: str) -> None:
+        """Switch staged workflows to replacement CRUD after query."""
+        if requested_state != "staged":
+            return
+        sm.state = "replaced"


These three helpers (_crud_module_args, _query_module_args, _prepare_crud_state) are byte-for-byte identical to the copies added in network_state_machine.py (lines 293-316), and the two classes share no base. This is the same two-independent-copies-may-drift situation raised on PR #505, which was resolved there by extracting the shared attachment_vpc_peer_expander.py. Consider the same treatment here — a small shared helper module or mixin — so a future change to the staged state mapping lands in one place instead of two.

allenrobel · 2026-08-31T22:40:07Z

        desired_vrf_names = self.coordinator._configured_vrf_names(config)

-        sm, original_config, original_state = self.coordinator._new_state_machine(module_args, strategy)
+        sm, original_config, original_state = self.coordinator._new_state_machine(self._query_module_args(module_args), strategy)


Because _query_module_args remaps state to "overridden" before _new_state_machine constructs the NDStateMachine, results.state (documented in rest/results.py as "The Ansible state for the current task") is captured as "overridden" for the whole staged run, and every API call's metadata["state"] in the -vvv api_metadata output reports overridden rather than staged. Metadata-only (requests, diff, and changed are unaffected), but it will mislead anyone debugging a staged task from verbose output. Worth restoring the user-requested state on sm.results after construction (e.g. in _prepare_crud_state). Same applies to the network twin.

allenrobel · 2026-08-31T22:40:07Z

-        sm, original_config, original_state = self.coordinator._new_state_machine(module_args, strategy)
+        sm, original_config, original_state = self.coordinator._new_state_machine(self._query_module_args(module_args), strategy)
        try:
+            self._prepare_crud_state(sm, state)


Minor/latent: NDStateMachine.__init__ threads context={"state": self.state} into pydantic validation "so models can apply state-aware validation", so sm.proposed here was validated under "overridden"; _prepare_crud_state then flips sm.state to "replaced" without re-validating. No current VRF/Network model reads the validation context, so this has no effect today — flagging it so a future state-aware validator doesn't silently validate staged runs under the wrong state. A comment noting the assumption would be enough.

AKDRG requested review from akinross, allenrobel, anvitha-jain, gmicol, lhercot, mikewiebe, mtarking, sajagana, samiib and shrsr as code owners August 13, 2026 19:45

AKDRG added 4 commits August 14, 2026 15:57

Add internal staged VRF and Network state

2586642

Suppress deploy for internal staged state

f1a9114

Expose private staged workflow state

de58a7f

Fix staged current-state query scope

47d5826

AKDRG force-pushed the vrf_network_overridden_no_delete_internal branch from 661a7f6 to 47d5826 Compare August 14, 2026 10:32

AKDRG added 2 commits August 14, 2026 16:05

Rename staged workflow state

2e1b4ca

Apply black formatting

0827942

mtarking added 2.0.0 Release 2.0.0 nac-0.0.1 NaC ND release 0.0.1 labels Aug 14, 2026

AKDRG self-assigned this Aug 14, 2026

AKDRG added the ready for review Submitter is requesting a PR review label Aug 14, 2026

akinross requested changes Aug 25, 2026

View reviewed changes

AKDRG added 3 commits August 25, 2026 21:15

Add staged state for VRF and Network workflows

3e47eee

Add staged attachment integration assertions

e58e625

Expand staged integration coverage

0bcb7b5

AKDRG force-pushed the vrf_network_overridden_no_delete_internal branch from 636838b to 0bcb7b5 Compare August 26, 2026 18:05

Document staged state

e28de1f

allenrobel reviewed Aug 31, 2026

View reviewed changes

Uh oh!

Conversation

AKDRG commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue(s)

Proposed Changes

Test Notes

Cisco Nexus Dashboard Version

Related ND API Resource Category

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allenrobel left a comment

Choose a reason for hiding this comment

Code review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AKDRG commented Aug 13, 2026 •

edited

Loading

akinross Aug 25, 2026 •

edited

Loading