Add optional post-write verification for ND state machine modules

[gmicol]

Related Issue(s)

Fixes #385

Proposed Changes

• Add optional post-write controller readback to state-machine-based modules through:
  • verify.enabled
  • verify.attempts
  • verify.interval
• Keep generic verification disabled by default to preserve existing module behavior.
• Preserve the released vPC verification defaults and compatibility aliases for retries and timeout.
• Add a stack-safe, scoped RestSend retry policy with:
  • exact attempt semantics;
  • retry intervals without sleeping after the final attempt;
  • conservative transport retries for safe GET requests only;
  • no expansion of mutation retry behavior.
• Add FinalizationContext and finalize_result() as the explicit finalization boundary after all write, action, save, and deploy operations.
• Preserve operation-derived changed and replace after atomically only after a complete successful readback.
• Invalidate orchestrator caches before final readback.
• Use bounded intended-list pagination plus direct-detail queries for vPC readback instead of the full discovery workflow.
• Skip final readback for check mode, no-op operations, read-only states, and disabled verification.
• Update module documentation, examples, lifecycle call sites, and unit coverage.

Related PRs and Merge Order Sugestion

This PR overlaps several open shared-core and module-specific changes:

• #502 defines canonical terminal/retryable HTTP response classification. This PR relies on that classification rather than introducing a competing implementation.
• #498 introduces the shared config_actions framework and overlaps nd_argument_specs.
• #524 centralizes interface config_actions handling and overlaps interface orchestrators, module call sites, and lifecycle tests.
• #513 changes vPC deploy behavior and overlaps vPC arguments, models, module documentation, and post-deploy finalization.
• #525 is a draft prototype for request-outcome and final-state reconciliation. Its provenance concerns are complementary, but it substantially overlaps NDStateMachine and result handling.

Recommended conflict-minimizing merge order:

1. Merge Classify 4xx (except 429) as terminal in RestSend retry policy #502 to establish canonical REST response classification.
2. Merge Config Actions: Add Common Framework for Repository-Wide Save and Deploy Handling #498 as the shared config_actions foundation.
3. Merge nd_interface_*: adopt shared config_actions_spec() and apply_config_actions() bridge #524 and nd_manage_vpc_pair: Add resource deploy scope and make config_actions.deploy opt-in #513 after rebasing them on the shared framework. These may merge in either order.
4. Rebase and merge this PR after the preceding changes, preserving the finalization boundary after all interface and vPC actions.
5. Rebase and refine draft PR Prototype NDStateMachine final-state reconciliation #525 on the resulting finalization hooks once its public result/provenance contract is accepted.

PR #525 is a design-reconciliation dependency, not currently a required merge predecessor. If its result contract is accepted first, this PR must instead rebase and populate those canonical fields rather than adding a competing schema.

Test Notes

• Focused post-write verification suite: 418 passed
• Complete unit suite: 4246 passed
• ansible-test sanity: passed
• Live validation is still required for vPC intended-list pagination, direct-detail response shape, and eventual controller readback behavior.

Cisco Nexus Dashboard Version

ND v4.2+

Related ND API Resource Category

• analyze
• infa
• manage
• onemanage
• other

Checklist

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

[nikhilsrikrishna]

Post-write verification after a partial bulk failure

One scenario that may be worth considering is a bulk create for resources A, B, and C with verify.enabled: true.

As I understand the current flow, _manage_create_update_state() first builds the complete predicted state:

predicted after = [A, B, C]

The controller could then return a mixed 207 Multi-Status response:

A: success
B: failure
C: success

The response handling correctly detects the failure. However, _request() raises, the error is propagated as an NDStateMachineError, and execution moves directly to the module’s exception handler. This means finalize_result() is not reached, so the optional readback does not run.

The failure response may therefore contain:

changed: true
after:
  - A
  - B
  - C

while the controller actually contains only A and C.

The reverse can happen for bulk deletes: if A and C are deleted but B fails, after may still contain all three because delete_many() is not reached.

The state machine also reads module.params.get("ignore_errors", False), but the production argument specifications do not currently expose this option. Ansible’s task-level ignore_errors: true only allows the playbook to continue after the module fails; it does not suppress the exception inside the state machine or allow finalization to run.

Would it make sense to make the final readback boundary reachable after an attempted mutation fails as well? When verification is enabled, it could requery the controller to correct after while preserving the original mutation error. If the readback also fails, the mutation error could remain the primary error and after would remain predictive rather than being presented as controller-confirmed.

[allenrobel]

Code review

Two comments inline: one on the verify.retries deprecation shim in nd_manage_vpc_pair, and one question about how a failed verification readback is reported. Neither overlaps the points nikhilsrikrishna already raised.

Note for whoever rebases: a trial merge against current develop conflicts only in tests/unit/module_utils/orchestrators/test_base_interface.py; the source files auto-merge, and finalize_result() lands after deploy_pending() so it composes correctly with the #547 failure-path finalizer.

🤖 Generated with Claude Code

[allenrobel]

verify.retries has never shipped: nd_manage_vpc_pair is not in the v1.5.0 tag (git ls-tree v1.5.0 -- plugins/modules/nd_manage_vpc_pair.py is empty) and galaxy.yml is still 1.5.0, so the module and this option first appear in 2.0.0. Deprecating it with version="2.0.0" announces removal in the same release it is introduced.

Since no user has the old spelling, I think this whole shim can go: the retries alias + deprecate() calls here, the retries field/argspec entry in vpc_pair_model.py, and the retries fallback in common.get_verify_settings. A plain rename to attempts/interval would leave one code path instead of three.

Minor, only if the shim stays: the module.params["verify"]["attempts"] = ... mutation on lines 486-489 is never executed by the unit tests in test_manage_vpc_pair_verify.py, because _CompatibilityModule has no params attribute, so getattr(module, "params", {}) short-circuits.

[allenrobel]

Question on the failure semantics here. By the time finalize_result() runs, every create/update/delete, remove, deploy and save has already succeeded. If only the readback fails (retry policy exhausted on a transient 5xx, or from_api_response rejecting an unexpected shape), this raises NDStateMachineError, the same type a real mutation failure raises, so the module reports failed: true for a task whose configuration fully landed. A playbook cannot tell "config landed, confirmation failed" from "config never landed", and the doc fragment does not mention this outcome.

Was module.warn(...) plus the predicted after (and changed: true) considered for this case, reserving failed for the mutation path? If failing hard is the intended contract for verify.enabled, a sentence in doc_fragments/verification.py saying so would help users set attempts/interval accordingly.