Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(eos_designs): Handle overlapping vlan numbers with filter.only_in_use and trunkgroups #2628

Conversation

ClausHolbechArista
Copy link
Contributor

Change Summary

Handle overlapping vlan numbers with filter.only_vlans_in_use and trunkgroups

Related Issue(s)

Issue seen in deployment, where duplicate vlans were erroneously configured because the matching on trunk groups was done too lazy (converting to vlan numbers and thereby loosing information about trunk group)

Component(s) name

arista.avd.eos_designs

Proposed changes

Refactor vlan filtering to address corner cases

  • Set list of trunk groups as fact so filtering can be done on trunk groups in network_services code
  • Split endpoint_vlans and endpoint_trunk_groups into separate properties for local, mlag and downstream switches.
  • Use sets instead of converting back and forth in internal functions.
  • Various optimizations.

How to test

First commit in this PR adds a molecule example of the issue. This is a snip from the output with only that commit:

TASK [arista.avd.eos_designs : Generate device configuration in structured format] ***
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible_collections.arista.avd.plugins.plugin_utils.errors.errors.AristaAvdError: Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'.
fatal: [trunk-group-tests-l2leaf4 -> localhost]: FAILED! => {"changed": false, "msg": "Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'."}
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible_collections.arista.avd.plugins.plugin_utils.errors.errors.AristaAvdError: Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'.
fatal: [trunk-group-tests-l3leaf2a -> localhost]: FAILED! => {"changed": false, "msg": "Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'."}
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible_collections.arista.avd.plugins.plugin_utils.errors.errors.AristaAvdError: Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'.
fatal: [trunk-group-tests-l3leaf2b -> localhost]: FAILED! => {"changed": false, "msg": "Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'."}
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible_collections.arista.avd.plugins.plugin_utils.errors.errors.AristaAvdError: Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'.
fatal: [trunk-group-tests-l2leaf3 -> localhost]: FAILED! => {"changed": false, "msg": "Duplicate VLAN ID '200' found in Tenant 'TRUNK_GROUP_TESTS' during configuration of SVI in VRF 'TG_200'. Other VLAN is in Tenant 'TENANT_WITH_DUPLICATE_VLANS'."}

The second commit contains the fix.

No configurations are changed as part of this fix/refactor.

Checklist

User Checklist

  • N/A

Repository Checklist

  • My code has been rebased from devel before I start
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation and documentation have been updated accordingly.
  • I have updated molecule CI testing accordingly. (check the box if not applicable)

@ClausHolbechArista ClausHolbechArista requested a review from a team as a code owner March 16, 2023 10:07
@github-actions github-actions bot added role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR labels Mar 16, 2023
@carlbuchmann carlbuchmann added the cherry-pick-for-3.8.x PR to be cherry-picked to releases/v3.8.x brach later label Mar 16, 2023
@ClausHolbechArista ClausHolbechArista added this to the v4.0.0-dev7 milestone Mar 16, 2023
…s/network_services/utils_filtered_tenants.py

Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Copy link
Contributor

@gmuloc gmuloc left a comment

Choose a reason for hiding this comment

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

LGTM - I still would recommend documenting "somewhere centralized" (maybe artificially a section in the currently very big file) how the full logic is working for the block of endpoints/trunk groups.

@gmuloc gmuloc requested review from carlbuchmann and a team March 17, 2023 09:48
Copy link
Member

@carlbuchmann carlbuchmann left a comment

Choose a reason for hiding this comment

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

LGTM - replicated the issue locally and tested PR.

@carlbuchmann carlbuchmann merged commit 0c2166a into aristanetworks:devel Mar 17, 2023
carlbuchmann pushed a commit to carlbuchmann/avd that referenced this pull request Mar 20, 2023
…_use and trunkgroups (aristanetworks#2628)

Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
@carlbuchmann carlbuchmann removed the cherry-pick-for-3.8.x PR to be cherry-picked to releases/v3.8.x brach later label Mar 20, 2023
carlbuchmann added a commit that referenced this pull request Mar 20, 2023
…_use and trunkgroups (#2628) (#2641)

Co-authored-by: Claus Holbech <holbech@arista.com>
Co-authored-by: Guillaume Mulocher <gmulocher@arista.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn: Fix(eos_designs) role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants