Enhancing Fabric Type Support for Switches Module

[AKDRG]

Proposed Changes

This PR improves nd_manage_switches support for fabric-specific switch onboarding by adding fabric capability validation, exposing supported switch platform types, and improving lifecycle output visibility for config-save and deploy operations.

Changes

• Added fabric capability validation before switch discovery/add workflows.
• Enforced fabric-specific support for:

1. switch role
2. preserve_config
3. platform_type

• Exposed platform_type in the playbook argspec with supported choices:

1. nx-os
2. ios-xe
3. ios-xr
4. other

• Registered finalize operations in module output:

1. config_save
2. deploy_switches
3. deploy_config

• Preserved full controller response metadata for finalize API calls where available.
• Added/updated unit tests for fabric validation, platform input handling, and finalize output visibility.

Test Notes

Validated against live controller
pytest tests/unit/module_utils/test_nd_switch_resources.py -q
44 passed

Cisco Nexus Dashboard Version

4.2(1)

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

[allenrobel]

@AKDRG — a heads-up and a question, not a request to change anything here. This PR is doing the right thing; the collision is on my side.

Situation. In #404 I added a PlatformTypeEnum to plugins/module_utils/enums.py for FabricContext.get_platform_type(), which reads additionalData.platformType off GET /fabrics/{fabric}/switches. I did that without noticing your PlatformType in models/manage_switches/enums.py already covers the same ground — same values, except yours also has SONIC, which mine is missing (a real gap: my lookup would silently return None for a SONiC switch). Two enums one token apart in the name is a trap for the next reader, so I'd rather converge than ship the duplicate.

Proposal. Promote PlatformType from models/manage_switches/enums.py up to module_utils/enums.py, and have FabricContext use it. Per CLAUDE.md, module_utils/enums.py is for "Enums commonly used by most module utilities" — with both manage_switches and fabric_context needing it, that's what it's become. The reverse (a top-level util importing from models/manage_switches/) would be a layering inversion.

This shouldn't cost you anything. Done as a move + re-export — leaving PlatformType re-exported from models/manage_switches/enums.py (via __all__, to keep pylint quiet) — every existing import site keeps working untouched, nothing in this PR changes, and you don't need to rebase. module_utils/enums.py imports only from enum import Enum, so there's no cycle, and the re-export lives in a named module rather than __init__.py, so it's clear of the empty-init sanity rule. Two of your changed files (nd_switch_resources.py, config_models.py) are PlatformType consumers, which is exactly why I'd rather not touch import sites while you're in flight.

The question I actually want your read on. Your enums look deliberately endpoint-scoped — PlatformType documented as AddSwitches (POST switches), ShallowDiscoveryPlatformType split out because shallowDiscovery excludes apic. My use is a third context: the GET switches response. Is that response set known to match the AddSwitches request set, or would you rather it stay a separate enum on principle? If they can legitimately diverge, sharing one is the wrong call and I'll keep a distinct read-side enum — properly named and documented, with SONIC added. You know this API surface better than I do.

One detail either way: normalize() defaults None -> NX_OS, which suits the write path but not mine, where "switch reports no platformType" has to stay None. So I'd use the bare enum rather than normalize() regardless.

Happy to do the move in #404 if you're good with it, or leave it to you if you'd rather own that file.

[allenrobel]

@AKDRG thanks for moving PlatformType to plugins/module_utils/enums.py 👍

I'll wait for #405 to merge to leverage PlatformType at its new home.

[allenrobel]

Code review

Four findings, each as an inline comment: endpoint-import coupling in shared utils.py, role: None rejected by the new fabric-capability validation, a now-dead all_preserve_config flag, and an undocumented ValueError in normalize_platform_type.

🤖 Generated with Claude Code

[allenrobel]

LGTM after comments were addressed.

[sivakasi-cisco]

#438

[mikewiebe]

High: Onboarding-only preserve_config validation rejects existing no-op switches

Issue

Capability validation runs before compute_changes(), so it validates preserve_config for every desired switch, including switches already managed and ultimately classified as idempotent. Because the model injects preserve_config: false, an existing External switch fails unless every reconciliation playbook repeats the historical onboarding choice.

Evidence

• nd_switch_resources.py lines 3042-3059 runs the new capability validation before diff classification and idempotency.

• config_models.py lines 323-327 defaults omitted preserve_config to false.

• fabric_switch_capabilities.py lines 243-247 rejects that value unconditionally for External fabrics.

• nd_switch_resources.py lines 2748-2756 also hard-codes preserve_config: false in gathered inventory.

• An exact-head probe classified an existing External switch as a complete no-op, with every work bucket empty, but the pre-classification capability check still raised:

  preserve_config 'false' is not supported for External ...
  Supported preserve_config values: true.
  

• For example, an External switch is initially onboarded with preserve_config: true. On a later idempotency run, the playbook describes the same managed switch but omits this onboarding-only option:

  - cisco.nd.nd_manage_switches:
      state: merged
      config:
        - ip_address: 192.0.2.20
          username: admin
          password: password

  The model inserts preserve_config: false, and capability validation rejects it before compute_changes() can establish that the operation is a no-op:

  preserve_config 'false' is not supported for External.
  Supported preserve_config values: true.
  

  Had change computation run first, every work bucket would have been empty:

  switches_to_add: []
  switches_to_update: []
  switches_to_remove: []
  

  Gathered External configurations have the same problem because the serializer explicitly emits preserve_config: false.

Existing PR overlap

No matching existing PR comment found. Earlier comments discuss a dead preserve-config wait flag but do not cover pre-diff validation of an onboarding-only setting.

Existing open issue overlap

No matching open issue found. Focused searches for External switch and preserve_config idempotency did not identify an owner.

Impact

Existing External switch playbooks that were idempotent before this PR can now fail before diffing, discovery, or writes. Gathered External configuration is inherently unreplayable because it carries the rejected default.

Suggested fix

Classify the plan before enforcing onboarding-only fields and validate preserve_config only for switches that will actually be discovered, added, or re-added. Preserve input explicitness if omission must differ from false. Add a regression test for an existing External no-op switch with omitted preserve_config.

---

[mikewiebe]

Can you clarify why External switches require preserve_config: true? Is this an ND controller requirement or an intended module policy?

preserve_config appears to control initial onboarding behavior. Requiring it to remain true during every subsequent reconciliation means an omitted value—and the module’s own gathered value of false—causes an already-managed no-op switch to fail.

If this is a controller requirement, could we document the rationale and limit its validation to discovery, add, and re-add operations? Otherwise, should omission be accepted for switches that are already managed?

[AKDRG]

External Fabric has fixed preserve config "true". Omission has been now handled through the default validator which will fix it to true.

[mtarking]

What is other?

[AKDRG]

Other is used for Switches from other vendors such as Arista.

[mtarking]

I do not think this statement is true. Lets discuss in standup if needed.

[AKDRG]

Preserve config is disabled(false) for these fabrics.

[mtarking]

This is implicit true for external fabrics and not user configurable and in general defaults to true.

[AKDRG]

Defaults derived and validated depending on fabric type.

[mtarking]

We shouldn't expose yet imho.

[AKDRG]

Fixed