-
Notifications
You must be signed in to change notification settings - Fork 883
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 (openstack): Append interface / scope_id for IPv6 link-local metadata address #5419
Conversation
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.
This requires an interface together with the address itself
to be valid.
Why?
Also, please sign the CLA and add your name to the signers list so that we can accept this contribution.
# Various defaults/constants... | ||
DEF_MD_URLS = ["http://[fe80::a9fe:a9fe]", "http://169.254.169.254"] | ||
DEF_MD_URLS = [ | ||
"http://[fe80::a9fe:a9fe%{iface}]".format(iface=net.find_fallback_nic()), |
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.
We don't want to call this function outside of a function definition. This would call net.find_fallback_nic()
on all datasources (including non-openstack).
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.
@holmanb do you like it better when I move the addresses to the only function they are used (and likely mangled)?
Should / could I also use self.distro.fallback_interface
?
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.
@holmanb do you like it better when I move the addresses to the only function they are used (and likely mangled)?
Yes. This isn't referenced externally so that should be fine.
Should / could I also use self.distro.fallback_interface ?
Please do, that would be preferred.
7bad298
to
a79edf7
Compare
|
a79edf7
to
6131d39
Compare
6131d39
to
75d5fbd
Compare
Hi @frittentheke thank you for your bug report and your PR. Here the cloud-init.log output:
@holmanb thanks for the code review. |
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.
One minor nitpick, otherwise looks good!
Link-Local IPv6 addresses (fe80::/10) without a scope_id or interface are invalid, see [1]. This made urllib3 reject an URL of `http://[fe80::a9fe:a9fe]` as invalid argument. This change now appends an interface in the proper URL-encoded format ([2]). [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/netinet_in.h.html [2] https://datatracker.ietf.org/doc/html/rfc6874 Fixes canonicalGH-5422 Signed-off-by: Christian Rohmann <[email protected]>
75d5fbd
to
af4ac7f
Compare
Fixed/removed @holmanb |
Thanks for fixing the test @holmanb and for the quick review process! |
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 code looks good to me. Thanks @frittentheke for this contribution!
…ata address (#5419) Link-Local IPv6 addresses (fe80::/10) without a scope_id or interface are invalid, see [1]. This made urllib3 reject an URL of `http://[fe80::a9fe:a9fe]` as invalid argument. This change now appends an interface in the proper URL-encoded format ([2]). [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/netinet_in.h.html [2] https://datatracker.ietf.org/doc/html/rfc6874 Fixes GH-5422 Signed-off-by: Christian Rohmann <[email protected]>
@holmanb when we apply this patch to 24.1.4, we are failing a test which surprisingly passes when we use Ubuntu's 24.3.1 version of cloud-init under identical setup:
Any ideas why it fails after backport? |
Based on the message "Failed to establish a new connection: [Errno -2] Name or service not known", I would guess that there is no network connection. Based on the lack of IPv4 routes, I would assume that this is an ipv6-only test of some kind? It is hard to tell without more information. What was the test environment? Is it reproducible? What network configuration did Openstack pass to cloud-init? Why aren't there any assigned IPv6 gateways in the output? To diagnose this I would probably run |
Hi @ani-sinha @holmanb The openstack IPv6 metadata service connection failed during the first boot, but it can be connected when I login the VM. Let me know if require more information. |
Here is the case which succeeds with Ubuntu:
Here is the one which fails with RHEL VM
In the Ubuntu VM we have
For RHEL we have
I am wondering whether Ubuntu uses some special ipv6 config that we are missing in RHEL? In any case, this change seems to break us and it will be problematic for us when we rebase to latest upstream even if we ended up fixing RHEL (if the issue is in RHEL). |
Yes @ani-sinha as this breaks routing between your VM's link-local and the metadata service listening to |
Can you please explain how? I am not a ipv6 expert but what change does |
See torvalds/linux@761aac7 for some background.
Please check the versions of Python If that is missing in your case, then that is likely the reason why the URL string |
@frittentheke
We seem to have the version 1.26.19 |
Proposed Commit Message
Additional Context
Test Steps
Start VM on OpenStack with only IPv6.
#1805 (comment)
Checklist
Merge type