Skip to content

Conversation

@allenrobel
Copy link
Collaborator

@allenrobel allenrobel commented Dec 1, 2025

This commit modifies the numbering of fabrics such that the number (e.g. fabric_name_X, fabric_type_X) is consistent across all tests.

Previously (for example) fabric_type_3 might be VXLAN_EVPN in one test and LAN_CLASSIC in another test.

I've opted for a larger number space for each fabric type i.e. 00-09 VXLAN_EVPN, 10-19 VXLAN_CLASSIC, etc...

fabric_type_00 - fabric_type_09 - VXLAN_EVPN
fabric_type_10 - fabric_type_19 - LAN_CLASSIC
fabric_type_20 - fabric_type_29 - BGP (Routed)
fabric_type_30 - fabric_type_39 - ISN (External)
fabric_type_40 - fabric_type_49 - IPFM
fabric_type_50 - fabric_type_59 - AI

fabric_type_100 - fabric_type_109 - VXLAN_EVPN_MSD
fabric_type_110 - fabric_type_119 - MCFG
fabric_type_120 - fabric_type_129 - FG

Notes to reviewers

  1. I've verified the following tests pass with ND 4.1.1g

NOTE: All replaced-state tests pass with ND 3.2 and fail with ND 4.1.1g (idempotence fails, everything else up to that point passes). But this was the case prior to this PR so should not stop merge of this PR. I did verify all the replaced-state tests (except IPFM) against ND 3.2 and they pass.

  • dcnm_fabric_deleted_basic
  • dcnm_fabric_deleted_basic_ipfm
  • dcnm_fabric_deleted_basic_isn
  • dcnm_fabric_deleted_basic_lan_classic
  • dcnm_fabric_deleted_basic_msd
  • dcnm_fabric_deleted_basic_vxlan
  • dcnm_fabric_merged_basic
  • dcnm_fabric_merged_basic_ipfm
  • dcnm_fabric_merged_basic_isn
  • dcnm_fabric_replaced_basic
  • dcnm_fabric_replaced_basic_ipfm (tested only with ND 4.1.1g)
  • dcnm_fabric_replaced_basic_isn
  • dcnm_fabric_replaced_basic_vxlan
  • dcnm_fabric_replaced_basic_vxlan_site_id

This commit modifies the numbering of fabrics such that the number (e.g. fabric_name_X, fabric_type_X) is consisstent across all tests.

Previously (for example) fabric_type_3 might be VXLAN_EVPN in one test and LAN_CLASSIC in another test.

The new numbering will be as follows once the changes are made.

fabric_type_01 - VXLAN_EVPN
fabric_type_02 - VXLAN_EVPN

fabric_type_03 - LAN_CLASSIC
fabric_type_04 - LAN_CLASSIC

fabric_type_05 - BGP (Routed)
fabric_type_06 - BGP (Routed)

fabric_type_07 - ISN (External)
fabric_type_08 - ISN (External)

fabric_type_20 - IPFM
fabric_type_21 - IPFM

fabric_type_30 - AI
fabric_type_31 - AI

fabric_type_40 - FG
fabric_type_41 - FG

fabric_type_50 - VXLAN_EVPN_MSD
fabric_type_51 - VXLAN_EVPN_MSD

fabric_type_60 - MCFG
fabric_type_61 - MCFG
@allenrobel allenrobel self-assigned this Dec 1, 2025
@allenrobel allenrobel added the Work in Progress Code not ready for review. label Dec 1, 2025
1. Use standardized fabric_name and fabric_type numbering across all tests

2. Shorten fabric names from *_Fabric to * e.g. LAN_CLASSIC_Fabric to LAN_CLASSIC

3. Shorted VXLAN_EVPN_MSD_Fabric name to MSD.
1. playbooks/roles/dcnm_fabric/dcnm_tests.yaml

Update with new fabric numbering and naming
@allenrobel allenrobel requested a review from Copilot December 1, 2025 20:51
Copilot finished reviewing on behalf of allenrobel December 1, 2025 20:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request standardizes fabric naming conventions across integration tests by introducing a consistent numbering scheme. Each fabric type is allocated a specific number range (e.g., 00-09 for VXLAN_EVPN, 10-19 for LAN_CLASSIC, 40-49 for IPFM, 100-109 for VXLAN_EVPN_MSD), ensuring that fabric identifiers remain consistent across all test files. This eliminates previous confusion where the same fabric number (e.g., fabric_type_3) could represent different fabric types in different tests.

Key Changes

  • Introduced standardized fabric numbering scheme with allocated ranges per fabric type
  • Updated variable definitions in dcnm_tests.yaml with new naming convention
  • Systematically updated all test files to use the new fabric_name_XX and fabric_type_XX variables
  • Enhanced test descriptions and comments to reference the new variable names

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
playbooks/roles/dcnm_fabric/dcnm_tests.yaml Defines new fabric naming scheme with consistent number ranges and fabric names matching types
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_save_deploy_ipfm.yaml Updated IPFM fabric references from fabric_name_4 to fabric_name_40
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_save_deploy.yaml Updated VXLAN_EVPN (00) and LAN_CLASSIC (10) references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_basic_vxlan_site_id.yaml Updated VXLAN_EVPN references to fabric_name_00
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_basic_vxlan.yaml Updated VXLAN_EVPN references to fabric_name_00
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_basic_isn.yaml Updated ISN fabric references from fabric_name_5 to fabric_name_30
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_basic_ipfm.yaml Updated IPFM fabric references from fabric_name_4 to fabric_name_40
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_replaced_basic.yaml Updated all three fabric types (VXLAN_EVPN, MSD, LAN_CLASSIC) to new scheme
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_query_basic.yaml Updated query tests with new fabric references for all three types
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_merged_save_deploy_ipfm.yaml Updated IPFM merge/deploy test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_merged_save_deploy.yaml Updated VXLAN_EVPN and LAN_CLASSIC merge/deploy tests
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_merged_basic_isn.yaml Updated ISN basic merge test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_merged_basic_ipfm.yaml Updated IPFM basic merge test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_merged_basic.yaml Updated all fabric types in basic merge tests
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic_vxlan.yaml Updated VXLAN_EVPN deletion test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic_msd.yaml Updated MSD fabric references from fabric_name_2 to fabric_name_100
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic_lan_classic.yaml Updated LAN_CLASSIC references from fabric_name_3 to fabric_name_10
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic_isn.yaml Updated ISN deletion test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic_ipfm.yaml Updated IPFM deletion test references
tests/integration/targets/dcnm_fabric/tests/dcnm_fabric_deleted_basic.yaml Updated all fabric types in comprehensive deletion tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

1. action is “fabric_update” rather than “fabric_update_bulk”

Did a global search/replace and tested that it’s working.

2. Updated test titles
The following used to work, but is deprecated:

assert:
  that:
      - result.response[0].DATA == "Invalid JSON response: Fabric 'ISN' is deleted successfully!"

Changed to conform to new YAML quoting requirements

assert:
  that:
      - "result.response[0].DATA == \"Invalid JSON response: Fabric 'ISN' is deleted successfully!\""
Doh!

omnipotence -> idempotence
@allenrobel allenrobel changed the title Unique fabric_types (strawman) IT: dcnm_fabric: Unique and consistent numbering for fabric_types Dec 2, 2025
@allenrobel allenrobel added ready for review PR is ready to be reviewed and removed Work in Progress Code not ready for review. labels Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review PR is ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants