Fix silently skipped deletion of the final MCFG VRFs/networks (unsafe sentinel serialization)

[dacasti2mx]

Related Issue(s)

Fixes #846

Related Collection Role

• cisco.nac_dc_vxlan.validate
• cisco.nac_dc_vxlan.dtc.create
• cisco.nac_dc_vxlan.dtc.deploy
• cisco.nac_dc_vxlan.dtc.remove
• other

Related Data Model Element

• vxlan.fabric
• vxlan.global
• vxlan.topology
• vxlan.underlay
• vxlan.overlay
• vxlan.overlay_extensions
• vxlan.policy
• vxlan.multisite
• defaults.vxlan
• other

Proposed Changes

This replaces the sentinel read/write mismatch in plugins/action/dtc/build_resource_data.py that caused MCFG multisite overlay removal to be silently skipped.

Root cause, in plain terms: the collection keeps a small file per MCFG fabric (the sentinel, _msite_overlay_sentinel.yml) to remember what the overlay's VRFs/networks looked like last time, so the next run can tell if anything changed. The function that writes it — _detect_msite_overlay_changes() — didn't save plain values. Instead of name: VRF01, the file ended up with something like this for that same value:

? !!python/object/apply:ansible.utils.unsafe_proxy.AnsibleUnsafeText
  - VRF01

That's a Python-specific instruction embedded in the file, not just plain text. The function that reads it back — _sentinel_had_key() — deliberately refuses to load a file containing that kind of instruction (a normal security precaution, so it never rebuilds arbitrary code from a file). The read always failed, and that failure was silently swallowed instead of surfaced, so the collection assumed "nothing existed before" every time.

That specific gap only matters once every VRF/network is removed: the current state becomes just an empty list either way, so the previous sentinel is the only place left to tell "these existed and were removed" from "there was never anything here." Since that read always silently failed, changes_detected_vrfs/changes_detected_networks never got set, dcnm_network/dcnm_vrf were never called, and nothing ever reached NDFC. Present since 0.8.0 (see #846).

Fix (single file, plugins/action/dtc/build_resource_data.py):

• Write with yaml.safe_dump() on values normalized to plain Python types (_normalize_for_yaml()), which raises TypeError rather than silently guessing via str() for any type it doesn't explicitly handle.
• Read with a single, load-before-mutate _load_overlay_sentinel() call, using yaml.safe_load() only — no custom loader. This read happens before the sentinel file (and the unrelated deferred-build cache file) are backed up, removed, or overwritten, so a bad read never leaves things half-touched.
• A sentinel this cannot read as a plain YAML mapping — the pre-existing Python-object-tagged format, a wrong top-level structure, or genuinely malformed YAML — now raises a RuntimeError naming the exact recovery step: re-run once with force_run_all: true and the complete, unchanged data model. A separate message is used for filesystem/permission failures, since force_run_all: true would not fix those.
• The failure-swallowing _sentinel_had_key() helper and its repeated sentinel reads are removed; the already-loaded sentinel is reused directly, so the sentinel is opened and parsed once per build instead of twice.

Migration required for existing installations: after upgrading, run once with force_run_all: true using the complete, unchanged data model before removing any VRFs/networks, so the existing sentinel gets regenerated in the new, plain-YAML format. Without this step, the next diff-run against a pre-upgrade sentinel will fail loud (by design) rather than silently skip the removal as before.

Scope boundaries:

• The reproduced and end-to-end validated scenario is MCFG. The modified sentinel implementation (_detect_msite_overlay_changes(), _load_overlay_sentinel(), _write_overlay_sentinel()) is shared by MCFG and MSD (self.fabric_type in ('MSD', 'MCFG')). No end-to-end MSD support claim is made by this PR — MSD has not been separately reproduced or validated.
• _msite_build_overlay() (plugins/plugin_utils/pipeline_base.py) is not modified by this PR. This PR covers the populated → empty transition. A separate, independent defect in the empty → populated (recreate) direction was found during this PR's live validation and is not fixed here — tracked separately, out of scope of this PR.

Test Notes

Local: a 21-case/25-assertion matrix (ResourceDataBuilder/ResourceRemover/PipelineRunnerBase, no mocks of the fix itself) covering: the legacy sentinel format failing loud in diff-run (sentinel and cache file left byte-for-byte untouched); safe read/write round-tripping; missing/corrupt/wrong-structure/permission-denied sentinels (each producing the correct, distinct recovery message); full integration through a real ResourceRemover.run_pipeline() (controller-diff discovery, dispatch order, dispatch content); the run_map retry/self-healing path (unrelated to and unmodified by this fix); idempotent re-runs; and the full mandatory migration flow driven against the real, unmodified ResourceDataBuilder.build() (force_run_all=True → real _cleanup_files() → fresh safe sentinel → subsequent diff-run → real dispatch). Also covers several related scenarios to characterize remove_resources.py's existing, unmodified behavior: only one resource type emptied while the other remains, a true partial removal within one resource type, and an attach-group-only change.

Results:

• Full matrix: 25/25 PASS
• 19/25 assertions fail or differ against the baseline (reverted, original pre-fix code). Cases 20 and 21 pass on both versions because they characterize behavior unaffected by the sentinel defect (a real diff.removed-based partial removal, and an attach-group-only change that never needed the sentinel read to begin with), not evidence of the fix itself.

The test harness and captured results live in the integration repository, not in this PR.

Live validation: I also validated this live against ND 4.1.1g / NDFC 12.4.1.321 (virtual N9K-C9300v, NX-OS 10.6(2)), reproducing the exact reported scenario end to end. With the pre-migration (legacy) sentinel still on disk and a real data-model change (removing VRFs/networks) in diff-run mode, the build now fails with the exact force_run_all: true recovery message, leaving the sentinel completely untouched and zero controller requests made. force_run_all: true with the complete, unchanged data model then completes successfully and regenerates a plain, safe sentinel. The following diff-run correctly dispatches dcnm_network/dcnm_vrf with state: deleted (controller diff: 2 networks and 1 VRF on the controller, 0 in the data model, all discovered for deletion), and NDFC confirmed the actual deletion afterward.

Cisco Nexus Dashboard Version

Confirmed on 4.1.1g (NDFC 12.4.1.321). The failure occurs before any controller request and has no NDFC-version conditional in the code, so it is expected on ND 4.2 as well — ND 4.2 has not yet been tested directly.

Checklist

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