IT: dcnm_fabric: Unique and consistent numbering for fabric_types#569
Open
allenrobel wants to merge 8 commits intodevelopfrom
Open
IT: dcnm_fabric: Unique and consistent numbering for fabric_types#569allenrobel wants to merge 8 commits intodevelopfrom
allenrobel wants to merge 8 commits intodevelopfrom
Conversation
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
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
Contributor
There was a problem hiding this comment.
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!\""
allenrobel
changed the title
allenrobel
added
ready for review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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...
Notes to reviewers
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.