Staged preview refinement for vpc_pair with additional tests

[sivakasi-cisco]

This PR fixes the "stage now, deploy later" flow for vPC pairs.

Before, re-running to deploy a previously staged pair did nothing (changed: false) and left it stuck pending.

Now the module detects pending work on save (not just deploy), trusts the switch's real status over the controller's optimistic summary, and reports changed: true correctly.

Added 4 integration tests in addition (staged ladder, check-mode, deploy-scope, negative).

[allenrobel]

Code review

Two observations on the sync-state refinement — one comment/dead-code cleanup in query.py, one question about the changed promotion in resources.py. The utils.py OperationType.UPDATE fix looks correct (verified register_action_api_call has a single caller, and the per-call Results scoping means no blast radius).

🤖 Generated with Claude Code

[allenrobel]

Just a comment rather than a review comment, but it would be helpful to reviewers if you describe what this PR does in the PR description, along with a description of the changes and why they are needed.

[sivakasi-cisco]

Just a comment rather than a review comment, but it would be helpful to reviewers if you describe what this PR does in the PR description, along with a description of the changes and why they are needed.

Hi Allen, True. I moved it from draft to open and had missed it

[nikhilsrikrishna]

Could you please confirm whether this is the intended behavior for config_actions.type: switch? Currently, the module appears to deploy every switch in the fabric that is not explicitly in-sync, rather than only the vPC peer switches managed by the current invocation (code). Would it be more appropriate to limit the deployment to the affected vPC peer switches?

[nikhilsrikrishna]

Problem scenario

A vPC pair deletion can be staged using save: false and deploy: false, with the intention of saving and deploying it in a later module invocation.

During the staged deletion, the controller removes the pair from the vPC pair inventory before the switch-side removal is saved or deployed. When the module is subsequently invoked with save: true and deploy: true, it sees that the pair is absent and treats the operation as complete.

The later invocation therefore returns changed: false and does not call either configSave or switchActions/deploy, even though the removal is still pending for the switches.

Steps to reproduce

1. Create and deploy a vPC pair:

   - cisco.nd.nd_manage_vpc_pair:
       fabric_name: "<fabric>"
       state: merged
       config:
         - peer1_switch_id: "<peer1>"
           peer2_switch_id: "<peer2>"
           use_virtual_peer_link: false
       config_actions:
         save: true
         deploy: true
         type: switch

2. Stage its deletion without saving or deploying:

   - cisco.nd.nd_manage_vpc_pair:
       fabric_name: "<fabric>"
       state: deleted
       config:
         - peer1_switch_id: "<peer1>"
           peer2_switch_id: "<peer2>"
       config_actions:
         save: false
         deploy: false
         type: switch

3. After this task:

   • The module reports changed: true.
   • The intended-pair inventory no longer contains the pair.
   • The direct vpcPair endpoints for both peers return {}.
   • Both switches report configSyncStatus: pending.
   • The vPC configuration has not yet been removed.
   • pendingConfig is empty because configSave has not been called.

4. Invoke the same deletion again with save and deploy enabled:

   - cisco.nd.nd_manage_vpc_pair:
       fabric_name: "<fabric>"
       state: deleted
       config:
         - peer1_switch_id: "<peer1>"
           peer2_switch_id: "<peer2>"
       config_actions:
         save: true
         deploy: true
         type: switch

5. The second invocation returns:

   changed: false
   VPC pair <peer1>-<peer2> is already unpaired on the controller.
   No deletion required.
   

   The module trace contains no calls to:

   POST /actions/configSave
   POST /switchActions/deploy
   

Controller intent verification

After the second module invocation returned changed: false, I called POST /actions/configSave directly.

Subsequent pendingConfig responses for both peers contained the staged removal commands:

pendingConfigs:
  - no interface port-channel500
  - no interface Vlan3600
  - no vlan 3600
  - no vpc domain 1
  - no feature vpc
  # plus peer-link member cleanup and interface restoration commands

This confirms that the staged removal intent was still available to the controller even though the pair was no longer present in the pair inventory.

Calling POST /switchActions/deploy directly for the same peers completed the removal. The endpoint returned HTTP 207 with status: success and message: Deployed Successfully for both switches. Final verification showed both peers inSync, empty pendingConfig responses, and empty direct vpcPair responses.

[nikhilsrikrishna]

I noticed that config_actions.save and config_actions.deploy default to true when they are not explicitly provided. Could you please confirm whether this is the expected behavior?

[nikhilsrikrishna]

Impact: Low

Problem scenario

A vPC pair can be saved without being deployed using save: true and deploy: false. Because deployment is disabled, the switches remain pending/outOfSync.

Repeating the identical save-only operation performs another configSave and reports changed: true, even when there is no declarative pair difference and the pending configuration is unchanged.

Steps to reproduce

1. Create a controller-only pair:

- cisco.nd.nd_manage_vpc_pair:
    fabric_name: "<fabric>"
    state: merged
    config:
      - peer1_switch_id: "<peer1>"
        peer2_switch_id: "<peer2>"
        use_virtual_peer_link: false
    config_actions:
      save: false
      deploy: false
      type: switch

2. Save the pending pair without deploying:

- cisco.nd.nd_manage_vpc_pair:
    fabric_name: "<fabric>"
    state: merged
    config:
      - peer1_switch_id: "<peer1>"
        peer2_switch_id: "<peer2>"
        use_virtual_peer_link: false
    config_actions:
      save: true
      deploy: false
      type: switch

3. Repeat the exact task from step 2.

Observed behavior

The repeated save-only invocation returns:

changed: true
class_diff:
  created: []
  updated: []
  deleted: []
deployment_changed: true
deployment_needed: true
deployment_paths:
  - /api/v1/manage/fabrics/<fabric>/actions/configSave

The API trace for the repeated invocation contains:

POST /api/v1/manage/fabrics/<fabric>/actions/configSave
{"type":"switch"}

HTTP 200
{"status":"Config save is completed"}

It contains no vPC-pair PUT and no switchActions/deploy request.

Direct pendingConfig reads for both peers returned the same command lists before and after the repeated operation. The pair identity and configuration were also unchanged.

The repeated task is therefore reported as changed: true solely because the still-undeployed pair triggers another configSave, whose successful result is promoted to the module-level changed value. This makes repeated save: true, deploy: false operations non-idempotent.

[sivakasi-cisco]

Could you please confirm whether this is the intended behavior for config_actions.type: switch? Currently, the module appears to deploy every switch in the fabric that is not explicitly in-sync, rather than only the vPC peer switches managed by the current invocation (code). Would it be more appropriate to limit the deployment to the affected vPC peer switches?

Hi Nikhil, thank you for the comments. There is one more PR where the config_actions are explictly handled for vpc pair here. Just to avoid repetitive changes, I have handled the default deploy to false in that.

https://github.com/CiscoDevNet/ansible-nd/pull/513/changes