-
Notifications
You must be signed in to change notification settings - Fork 72
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
lag: Support M:M MLAG topologies #1604
Conversation
The 'determine_mlag_sides' function makes my head explode. There must be an easier way to get that done. Its sole purpose is to figure out which nodes are on the left- and the right- side of the bundle, right? |
Probably because it's done incrementally, we could do it in one shot upfront:
Note that we could auto-detect the user's intent, without requiring a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't review this. The diffs are a spaghetti mess. I will make a few suggestions and after you look at them merge it as-is (it shouldn't break anything else anyway) and then maybe take a look at individual functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least the documentation should be changed to include the diagram.
* Update test cases
* Allow bool for lag.mlag.peergroup, auto-assign in case of True (useful for composed topologies) * Update test cases
* Don't count 'True' as int
… flag instead * Refactor peer links code
@@ -84,7 +84,8 @@ def normalized_members(l: Box, topology: Box, peerlink: bool = False) -> list: | |||
for idx,member in enumerate(l.lag.members): | |||
member = links.adjust_link_object(member,f'{l._linkname}.{_name}[{idx+1}]',topology.nodes) | |||
if 'lag' in member: # Catch potential sources for inconsistency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, list(member.lag.keys()) == ['ifindex']
would be way easier to understand, but whatever.
Merged as-is. |
lag.ifindex
attributes on interfacesFixes #1602
To be discussed:
mlag
flag or not? Currently left out