Config Actions: Wire nd_manage_switches to the Common Save and Deploy Framework - #499
Config Actions: Wire nd_manage_switches to the Common Save and Deploy Framework#499AKDRG wants to merge 9 commits into
Conversation
…n' into config_actions_switches_integration
allenrobel
left a comment
There was a problem hiding this comment.
Code review
Reviewed the switch-module wiring into the common config-actions framework (findings in the config_actions/ package itself are out of scope here since they land via #498). Two findings, posted inline.
🤖 Generated with Claude Code
| switch_ids=tuple(serial_numbers or ()), | ||
| ) | ||
| controller = ConfigActionsController(SWITCH_CONFIG_ACTIONS, SwitchConfigActionsBackend(self.fabric_utils)) | ||
| result = controller.execute(actions, context) |
There was a problem hiding this comment.
When deploy_type is switch and serial_numbers is empty, the controller returns status: "skipped", error: "no_targets" for the deploy step and the module still reports success. The code this replaces fell back to a global deploy with a warning in exactly that case ("Switch-level deploy requested but no serial numbers provided — falling back to global deploy", removed in this hunk).
The idempotent config-sync paths call finalize(serial_numbers=sync_serials) (three sites, ~L3241/L3389/L3528) where sync_serials is built from a filtered comprehension (switch_id must be truthy) and can legitimately be empty while a deploy is still required — in that case pending config-sync now goes undeployed, with the skip visible only in the nested DATA.actions[].error field.
Suggest either restoring the global-deploy fallback for this path, or at minimum warning/failing when a requested deploy is skipped for lack of targets, so the pending config doesn't silently stay undeployed.
There was a problem hiding this comment.
Great catch.
Since we are specfically targeting "switch" deploy_type, doing a global deploy should not be done. So, I'm throwing up an error now.
Related Issue(s)
Related to #368
Merge dependency on common framework #498
Proposed Changes
nd_manage_switchesinto the sharedconfig_actionsframework.config_actionsthrough the common policy-driven parser.config_actions.save: trueconfig_actions.deploy: trueconfig_actions.type: switchconfig_actions: {}deploy: truewithsave: falseconfig_actions.deployis enabled.Test Notes
Ran formatting:
black --check -l 159 plugins testsRan targeted sanity checks:
ansible-test sanity --test pep8 --color --truncate 0ansible-test sanity --test pylint --color --truncate 0Ran full sanity from a materialized collection tree:
ansible-test sanity --color --truncate 0Live tested switch config actions on
AK-VXLAN:Cisco Nexus Dashboard Version
Cisco Nexus Dashboard 4.2.1
Related ND API Resource Category
Checklist