Skip to content

Fix vPC peering default payload - #801

Open
dacasti2mx wants to merge 16 commits into
netascode:refactor_collectionfrom
dacasti2mx:dacasti2/fix-vpc-fabric-peering-default
Open

Fix vPC peering default payload#801
dacasti2mx wants to merge 16 commits into
netascode:refactor_collectionfrom
dacasti2mx:dacasti2/fix-vpc-fabric-peering-default

Conversation

@dacasti2mx

Copy link
Copy Markdown
Contributor

Summary

Always render useVirtualPeerlink in roles/dtc/common/templates/ndfc_vpc/ndfc_vpc_peering_pairs.j2, defaulting to false when fabric_peering is omitted from a vpc_peers entry.

Problem

When fabric_peering is not set on a vpc_peers entry, the template currently skips the useVirtualPeerlink field entirely:

{% if vpc_peers_pair.fabric_peering is defined %}
  useVirtualPeerlink: {{ vpc_peers_pair.fabric_peering }}
{% endif %}

Against a freshly-built fabric, the resulting dcnm_vpc_pair call has been observed to receive the pair as input but the artifact does not show a POST for it (the pair is absent from result.diff[0].merged and from result.response). A subsequent dcnm_interface step then fails when it tries to create a vPC port-channel on a peer in that pair:

Create pipeline failed at step 'interface_all' (dcnm_interface):
Switch '<ip>' is not part of VPC pair, but given I/F 'vpc<N>' is of type VPC

Full evidence (artifact JSON snippets and reproducer data model) is in the linked issue.

Fix

  useVirtualPeerlink: {{ vpc_peers_pair.fabric_peering | default(false) }}

This aligns the rendered payload with:

  • The documented dcnm_vpc_pair default — useVirtualPeerlink: { type: bool, default: false }.
  • The existing validation rule roles/validate/files/rules/ibgp_vxlan/208_manual_ipaddress_allocation.py, which already uses peer.get("fabric_peering", False).
  • The reference data model nac-fabric1, which sets fabric_peering: false explicitly and works end-to-end.

Scope

  • One file changed: roles/dtc/common/templates/ndfc_vpc/ndfc_vpc_peering_pairs.j2.
  • No behavior change for entries that already define fabric_peering (rendering for those is bit-identical).

Verification

Manual Jinja render against three cases (fabric_peering: true, fabric_peering: false, omitted):

- peerOneId: 10.122.226.107
  peerTwoId: 10.122.226.108
  useVirtualPeerlink: True

- peerOneId: 10.122.226.115
  peerTwoId: 10.122.226.116
  useVirtualPeerlink: False

- peerOneId: 10.122.226.105
  peerTwoId: 10.122.226.106
  useVirtualPeerlink: False

The third case is the fix: previously, the useVirtualPeerlink line would not be emitted at all.

Related issue

Closes #800.

@dacasti2mx dacasti2mx self-assigned this May 28, 2026
@dacasti2mx dacasti2mx added bug Something isn't working ready for review PR Ready for Review labels May 28, 2026
@dacasti2mx
dacasti2mx marked this pull request as ready for review May 28, 2026 05:05
@dacasti2mx
dacasti2mx requested a review from a team as a code owner May 28, 2026 05:05
mtarking and others added 3 commits May 31, 2026 16:36
* start of refactor

* vpc working

* split vrfs and networks

* add deploy param to create vrfs and networks workflow

* comment out vrfs and networks deploy param in create workflow

* updates for vrfs and networks functionality

* refactor deploy

* enable create pipeline for policy

* updates for tags and remove

* refactor to abstract objs

* Add update hostname pipeline step

* initial updates for tor

* modify action plugin executor

* update create resources

* update config save for mcfg

* many updates - renames, tor, switch deploy, etc

* close gaps on missing functionality for manual allocation and unmanaging policy

* fix tag bug with tors and update data model check flag

* cleanup and refactor imports

* rename objects to resources

* add banner for pipeline task to know where execution is in create and remove

* update remove for ints, vrf, networks when diff run disabled

* optimize config save runs

* update resources

* update display printing

* updates for msd resources

* updates for msd

* update role tag issue

* enable ebgp

* update color

* remove meta end play

* Fix remove when config list empty with state overridden

* Diff Run Fixes

* Update ebgp fabric

* update deferred msite overlay data render

* update create and remove resources yaml

* Update create_resources.yml

* updates for fabric links

* update data model check for ibgp vxlan

* update for policy and fabric links

* clean up lint

* update vrf and network removal

* fixes to vrfs and networks

* fix for child fabric removal

* update config save for fabric

* adjust runtime status msgs

* update vrf loopback attachment to execute only if valid loopback or freeform

* update changed status check for vrf loopback attach

* update msd and mcfg vrf loopback templates

* update ebgp and bring in mcfg

* update change flag for config save tasks

* resolve remove for links and update data model guard for inventory config save

* updates for deferred msd and mcfg overlay processing

* enable edge connections

* enable edge connections

* Update existing_links_check.py (netascode#779)

* updates for msite diff run

* updates for mcfg

* comment out config save request debugs

* Refactor skip validation fabric (netascode#780)

* Update pipeline_base.py

Add Skip_validation for ISN fabric

* Update ndfc_executor.py

add skip_validation flag for dcnm_fabric

* Update pipeline_base.py

add use case for external fabric

* Update pipeline_base.py

add eBGP_VXLAN use case

* Update create_resources.yml

* Update pipeline_base.py

* Update pipeline_base.py

* Update ndfc_executor.py

* updates to fabric deployer

* add display in deployer

* Improve role validate performance (netascode#789)

* Improve role validate performance

* Improve mark stage complete tasks

* Build resources performance improvements

* resolve lint & sanity errors

* Update ndfc_vpc_domain_id_resource.j2 (netascode#792)

* comment out unused discovery tasks

* Refactor var name for underlay ip / bgw anycast (netascode#793)

* Update ndfc_underlay_ip_address.j2

* Update ndfc_bgw_anycast_vip.j2

* Update create_resources.yml

* Bulk Update Switch Hostname Policy (netascode#795)

* Bulk API for Hostname Fabric Policy Get

* Fix Sanity

* Fix Pep8

* Diff underlay ip address (netascode#773)

* Update create_resources.yml

* Update manage_resources.py

* Create underlay_ip_manual_allocation_filter.py

* Update create_resources.yml

* Update manage_resources.py

* Update manage_resources.py

* Update underlay_ip_manual_allocation_filter.py

* Address PR netascode#773 review comments: single-pass optimization, remove Tier 3 fallback, fix verbosity, remove dead code

- Merge extract/filter/index into single-pass loop (review comment #1)
- Move pools list logging from -vv to -vvv (review comment netascode#2)
- Remove Tier 3 pool+resource fallback, keep two-tier only (review comment netascode#3)
- Remove unused underlay_ip_audit_pools and underlay_ip_audit_scope_filter (review comment netascode#4)

Note: After dcnm_resource_manager updates pool allocations, a Recalculate
and Deploy is required for changes to be applied to device configuration.

* fix lint errors

* Update main.yml

Enable CI tests on refactor_collection.

Remove before merge to develop

* Refactor - enhance diff controller links (netascode#794)

* Update manage_resources.py

* Move fabric links profile comparison into existing_links_check

- Move value-level profile comparison from manage_resources.py prefilter
  into existing_links_check.py during endpoint matching (single pass)
- Add _link_profile_changed() method with forward check (desired vs controller)
  and reverse check (field removal detection for peer1_cmds/peer2_cmds)
- Remove redundant second-pass prefilter from manage_resources.py
- Both YAML and Python pipeline callers now benefit from profile filtering

Addresses review comments on PR netascode#794.

* Update manage_resources.py

* Refactor - Diff policies (netascode#784)

* Update diff_compare.py for policy

* Update manage_resources.py add remote policies diff

* Update create_resources.yml

* Update resource_types.yml

* Update manage_resources.py

* Update diff_compare.py

* Update manage_resources.py

* Address PR netascode#784 review: clean policy diff, resolve merge conflict, add use_desc_as_key

- Remove extra },  in diff_compare.py (review comment #1)
- Rewrite manage_resources.py policy diff to use per-switch API (review comment netascode#2)
- Remove dead code: _fetch_all_policy_pages, urlencode import, ndfc_internal_keys
- Remove all DEBUG blocks from manage_resources.py and pipeline_base.py
- Resolve merge conflict: integrate _underlay_ip_remote_diff from upstream
- Add use_desc_as_key=True for dcnm_policy in ndfc_executor.py (fixes changed=False bug)

* Update diff_compare.py

* Update manage_resources.py

* Fix E131 pep8: use parenthesized ternaries for hanging indent

* Fix MCFG Fabric Handling (netascode#798)

* Fix MCFG Fabric Handling

* Fix Lint

* remove docstring line

---------

Co-authored-by: mwiebe <mwiebe@cisco.com>
Co-authored-by: Charly Coueffe <75327499+ccoueffe@users.noreply.github.com>
Co-authored-by: Akshayanat C S <83760554+AKDRG@users.noreply.github.com>
* Release Prep Changes

* Nest bullet points in changelog

* Nested bullet points and formatting

* Nested bullet points and formatting

* Update readme

* changelog updates
@dacasti2mx
dacasti2mx force-pushed the dacasti2/fix-vpc-fabric-peering-default branch from ba8e2ee to 43e9e75 Compare June 3, 2026 20:26
@dacasti2mx
dacasti2mx force-pushed the dacasti2/fix-vpc-fabric-peering-default branch from 43e9e75 to 6bfe446 Compare July 7, 2026 01:44
mthurstocisco and others added 7 commits July 7, 2026 11:30
* Initial edge connections removal fix

* Update pipeline_base.py

* Update unmanaged_edge_connections.py

---------

Co-authored-by: Peter Lewis <pelewis2@cisco.com>
* fixing indentation

* Adding filter to dedent str

* adding condition and dedent filter

* Adding empty key for removal use case

* remove unused key
* Fixing typo

* Adding missing flags and reorder in alphabetical order

* Changing order for alphabetical order
* Adding greenfield cleanup option

* moving greenfield_cleanup under ibgp and ebgp
…etascode#819)

* Fix netascode#813: continue create pipeline on config-save HTTP 500

When a fabric is pre-provisioned, its switches may still be reloading or
not yet reachable. In that state NDFC returns HTTP 500 to the intermediate
config-save (recalculate). In 0.7.2 the run logged this and kept going; in
0.8.0 it aborts at the inventory_config_save step, so the rest of the
fabric intent is never applied.

The "treat 500 as non-fatal" logic in pipeline_base._config_save never ran,
because the response that carries the 500 was thrown away earlier:
fabric_deploy_manager set results['failed']=True but no 'msg', so the
RETURN_CODE never reached the check.

Changes:
- fabric_config_save() now returns the NDFC response.
- manage_fabrics() puts that response in results['msg'] when config-save
  fails, so callers can read the RETURN_CODE.
- _config_save() treats RETURN_CODE 500 as non-fatal and logs a clear
  warning. Other failures (400, auth, malformed responses) still abort.

Unit tests are deferred to a follow-up: the collection has no unit-test
harness today and CI only runs build, sanity, and lint. Verified locally
with a temporary mock harness (not committed): 500 -> continues with a
warning, 400 -> aborts, missing/malformed msg -> aborts, 200 -> ok.

* Gate non-fatal config-save HTTP 500 on pre-provisioned switches

The netascode#813 fix treated any HTTP 500 from config-save as non-fatal, which
could also swallow an unrelated 500 on a healthy fabric. Restrict the
non-fatal path to fabrics whose data model declares pre-provisioned
switches (poap.preprovision) -- the documented condition where NDFC
legitimately returns 500 while switches are still reloading or not yet
reachable. A 500 on a fabric without pre-provisioned switches, and any
other failure (400, auth, malformed), remain fatal.

Adds PipelineRunnerBase._fabric_has_preprovisioned_switches() and makes
the warning message explicit about why the run continues.
* Create 502_topology_switch_cross_reference.py

* Update 502_topology_switch_cross_reference.py

* ISN updates

* change from keys_data to keys_found

* refactor into a single rule

* fix sanities
Always render `useVirtualPeerlink` in `ndfc_vpc_peering_pairs.j2`,
defaulting to `false` when `fabric_peering` is omitted.

Previously, omitted `fabric_peering` caused the field to be dropped from
the generated payload. In a fresh fabric, this can result in the vPC pair
not being created/recognized by NDFC; later `dcnm_interface` fails when
creating vPC interfaces with:

  Switch '<ip>' is not part of VPC pair, but given I/F 'vpc<N>' is of type VPC

This aligns the template with the documented `dcnm_vpc_pair` default and
existing validation behavior, both of which treat omitted `fabric_peering`
as `false`.
@dacasti2mx
dacasti2mx force-pushed the dacasti2/fix-vpc-fabric-peering-default branch from 6bfe446 to 246238a Compare July 10, 2026 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready for review PR Ready for Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants