-
Notifications
You must be signed in to change notification settings - Fork 76
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
addons: vxlan: add vxlan-local-tunnelip6 #182
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Markus Hauschild <markus@moepman.eu>
Since #172 seems to be going in the wrong direction I gave it a try using a new, distinct attribute. Please review and comment. |
For a very simple test case it does show that is has successfully set the local tunnel ipv6 addr:
|
) or self.syntax_check_localip_anycastip_equal( | ||
ifaceobj.name, | ||
ifaceobj.get_attr_value_first('vxlan-local-tunnelip6') or self._vxlan_local_tunnelip6, | ||
self._clagd_vxlan_anycast_ip) | ||
return True | ||
|
||
def syntax_check_localip_anycastip_equal(self, ifname, local_ip, anycast_ip): |
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.
The syntax check will still report "tunnelip" even if tunnelip6 is identical to clagd-vxlan-anycast-ip.
I am unsure wether clagd-vxlan-anycast-ip could even be IPv6 or (if that is not the case) how this should be properly refactored.
you are right this needs to be refactored. We should isolate the
clagd-anycast ip checks.
Its ok for the clagd anycast ip checks to be not included along with the
tunnelip6 patch.
We can add a comment there. (Maybe julien has other thoughts).
…On 11/16/20 12:56 PM, Markus Hauschild wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In ifupdown2/addons/vxlan.py
<#182 (comment)>:
> return False
return self.syntax_check_localip_anycastip_equal(
ifaceobj.name,
ifaceobj.get_attr_value_first('vxlan-local-tunnelip') or self._vxlan_local_tunnelip,
self._clagd_vxlan_anycast_ip
- )
+ ) or self.syntax_check_localip_anycastip_equal(
+ ifaceobj.name,
+ ifaceobj.get_attr_value_first('vxlan-local-tunnelip6') or self._vxlan_local_tunnelip6,
+ self._clagd_vxlan_anycast_ip)
return True
def syntax_check_localip_anycastip_equal(self, ifname, local_ip, anycast_ip):
The syntax check will still report "tunnelip" even if tunnelip6 is
identical to clagd-vxlan-anycast-ip.
I am unsure wether clagd-vxlan-anycast-ip could even be IPv6 or (if
that is not the case) how this should be properly refactored.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#182 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG4ZPVECSGOC3AM2OAPUITSQGGYJANCNFSM4TXTHTWA>.
|
I know it is quite a bit of duplicated code, but I am open to suggestions on how to improve this, so that it can be merged. |
@julienfortin it is a bit sad that you didn't implement a change similar to this to solve #175 in your recent vxlan changes can you still do this or should I give it another try? |
Any updates on this? We just wanted to setup our new cluster ipv6 only to simplify networking a lot and just stumbled across this. Do we really have to resort to v4 to get VXLAN working? This is a huge PITA and not expected at all :( |
@MPStudyly are you a NVIDIA Cumulus Linux user? If you need this patch now and if you are happy with it, you can always patch your system and use it. I'm quite busy with other stuff at the moment. Though I would like this patch to be different, more like a refactor of the existing ip4 code, and accommodate both v4 and v6 settings through the same function instead of code duplicates. |
No, I'm on Proxmox, using it's native SDN features to setup VXLAN. Proxmox/Debian(generally?) relies on ifupdown2 as well.
While Proxmox is quite open about local modifications, I'd rather avoid this in a production environment. For our use case we were able to resort to IPv4 in a minimal fashion to work around this limitation. We still want to throw it out ASAP.
I fully understand this approach 👍 Yesterdays post was mostly made out of the initial frustration, learning of this (kinda stupid) limitation. Especially after seeing this is being open for almost 4 years now ^^" Don't feel pushed by my request though. I'm already glad to hear this wasn't forgotten :) |
Free beer to someone who fix it. |
Signed-off-by: Markus Hauschild markus@moepman.eu