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.

Sign up for GitHub

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

Jump to bottom

Feat(eos_designs): Compact MLAG allocations #2903 #2946

Merged
merged 19 commits into from Jun 15, 2023
Merged

Feat(eos_designs): Compact MLAG allocations #2903 #2946

merged 19 commits into from Jun 15, 2023

Conversation

hamptonmoore
Copy link
Member

@hamptonmoore hamptonmoore commented Jun 1, 2023 •
edited

Change Summary

Add a compact MLAG boolean flag for nodes which forces MLAG pairs to have an odd and even ID allowing more for compact numbering. Currently the only generation algorithm will skip every other /31 causing extra space to be wasted.

Related Issue(s)

Fixes #2903

Component(s) name

arista.avd.eos-designs

Proposed changes

  • Add an option mlag_compact_addressing to the eos-designs
  • In avdipaddressing.py check if mlag_compact_addressing is set to True then
    • Verify MLAG pair has a node with an even ID and an odd ID
    • Calculate Offset using the odd ID
    • Run AvdIpAddressing._ip() in a way that skips multiply by the subnet_size, which previously was causing the wasted /31

How to test

Add molecule tests, currently waiting for Carl to finish 4.0.0 molecule migration

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)

@github-actions github-actions bot added the role: eos_designs issue related to eos_designs role label Jun 1, 2023
@pvinci-arista
Copy link
Contributor

How can we document that all mlag node_types for a given address pool need to be configured consistently with mlag_compact_addressing to prevent an address collision.

Can we add a molecule test to cover this code?

ClausHolbechArista
ClausHolbechArista requested changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

Please add a test case to eos_designs_unit_tests using the new ID method.

...ble_collections/arista/avd/roles/eos_designs/python_modules/ip_addressing/avdipaddressing.py Outdated Show resolved Hide resolved
...ble_collections/arista/avd/roles/eos_designs/python_modules/ip_addressing/avdipaddressing.py Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_designs/schemas/eos_designs.schema.yml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@hamptonmoore hamptonmoore marked this pull request as ready for review June 2, 2023 16:54
@hamptonmoore hamptonmoore requested a review from a team as a code owner June 2, 2023 16:54
@carlbuchmann carlbuchmann marked this pull request as draft June 3, 2023 00:38
ClausHolbechArista
ClausHolbechArista requested changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

A few comments and then please look into adding a molecule test under the eos_designs_unit_tests scenario.

...ble_collections/arista/avd/roles/eos_designs/python_modules/ip_addressing/avdipaddressing.py Outdated Show resolved Hide resolved
...ble_collections/arista/avd/roles/eos_designs/python_modules/ip_addressing/avdipaddressing.py Outdated Show resolved Hide resolved
...ble_collections/arista/avd/roles/eos_designs/python_modules/ip_addressing/avdipaddressing.py Outdated Show resolved Hide resolved
...e_collections/arista/avd/roles/eos_designs/schemas/schema_fragments/ip_addressing.schema.yml Outdated Show resolved Hide resolved
ClausHolbechArista
ClausHolbechArista requested changes Jun 5, 2023
View reviewed changes
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

A few comments and then please look into adding a molecule test under the eos_designs_unit_tests scenario.

@github-actions github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Jun 5, 2023
hamptonmoore and others added 3 commits June 5, 2023 12:38
@hamptonmoore @ClausHolbechArista
Update ansible_collections/arista/avd/roles/eos_designs/python_module…
313a9b2 
…s/ip_addressing/avdipaddressing.py

Co-authored-by: Claus Holbech <holbech@arista.com>
@hamptonmoore @ClausHolbechArista
Update ansible_collections/arista/avd/roles/eos_designs/python_module…
c8e7648 
…s/ip_addressing/avdipaddressing.py

Co-authored-by: Claus Holbech <holbech@arista.com>
@hamptonmoore @ClausHolbechArista
Update ansible_collections/arista/avd/roles/eos_designs/python_module…
54fb383 
…s/ip_addressing/avdipaddressing.py

Co-authored-by: Claus Holbech <holbech@arista.com>
@carlbuchmann carlbuchmann added this to the v4.1.0 milestone Jun 5, 2023
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the state: conflict PR with conflict label Jun 6, 2023
@github-actions github-actions bot removed the state: conflict PR with conflict label Jun 6, 2023
@github-actions
Copy link

github-actions bot commented Jun 6, 2023

Conflicts have been resolved. A maintainer will review the pull request shortly.

@hamptonmoore hamptonmoore marked this pull request as ready for review June 6, 2023 16:40
@hamptonmoore hamptonmoore mentioned this pull request Jun 6, 2023
Closed
1 task
@github-actions github-actions bot added the state: conflict PR with conflict label Jun 7, 2023
@github-actions
Copy link

github-actions bot commented Jun 7, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the state: conflict PR with conflict label Jun 13, 2023
@github-actions
Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

emilarista
emilarista approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@emilarista emilarista left a comment

Choose a reason for hiding this comment

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

Tested and reviewed, LGTM!

@ClausHolbechArista ClausHolbechArista merged commit 7658e01 into aristanetworks:devel Jun 15, 2023
31 checks passed
@hamptonmoore hamptonmoore deleted the compactmlag branch June 16, 2023 15:39
@carlbuchmann carlbuchmann mentioned this pull request Jun 19, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

@emilarista emilarista emilarista approved these changes

@pvinci-arista pvinci-arista Awaiting requested review from pvinci-arista

Assignees

@hamptonmoore hamptonmoore

Labels
rn: Feat(eos_designs) role: eos_designs issue related to eos_designs role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated
Projects
None yet
Milestone
v4.1.0
Development

Successfully merging this pull request may close these issues.

Feature: Add more compact MLAG ip addressing scheme
5 participants
@hamptonmoore @pvinci-arista @ClausHolbechArista @emilarista @carlbuchmann