-
Notifications
You must be signed in to change notification settings - Fork 880
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
Make IPv6 Great Again #826
Conversation
@@ -931,7 +931,7 @@ func (n *network) ipamAllocateVersion(ipVer int, ipam ipamapi.Ipam) error { | |||
} | |||
|
|||
if len(*cfgList) == 0 { | |||
if ipVer == 6 { | |||
if ipVer == 6 && !n.enableIPv6 { |
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 think this check should instead take into account whether the ipam driver supports auto-allocation of ipv6 pools.
Now that ipamapi.Capability
is in master, I'd suggest to add a SupportsAutoIPv6
bool to the ipam driver capability.
In fact the reason why this function was bailing out in case of empty ipamV6Config
was just because the in-built ipam driver does not have the capability of self assigning an ipv6 pool from some predefined list.
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.
That's fine as well, in fact I would even prefer if the user does not have to explicitly request it.
@tgraf Put few comments. In summary, I think we should not overload the I see this PR is trying to achieve at least two distinct things:
For the first one I'd suggest to let the ipam driver express he can support it (now we have ipam capability), for the second one I think this has to come from user first class config, maybe an WDYT ? |
@aboch What I had in mind is to add a
The IPAM capability allows to do (1) automatically but I doubt we can go to (2) without an additional user option such as |
@tgraf I am not sure about the option 2 you mentioned. Considering the behavior would be controlled by a user specified option, how would it be useful to create a network which may get either an ipv4 or an ipv6 pool without knowing it a priori ? In a scenario where the ipam driver and network driver are implemented by the same entity, I am guessing why this would seem to be needed: It would give freedom to the network driver to decide whether to assign ipv4 or ipv6 pool for a specific network, without libnetwork complaining about it. But this would go against the reason for which the IPAM was added into libnetwork in first place, which is to give the docker user control over the ip address management for the containers/networks. The assigned ipam behavior should be deterministic. Probably we should allow the ipam driver also express its capability in supporting only v4, only v6 or both during registration. This way libnetwork will handle the ipam request accordingly and, as an example, won't ask for ipv4 pool if ipam driver does not or does not want to support it. This would be more explicit and confined to the iapm driver. Regarding In other words, ipv6 is currently considered optional and complementary to ipv4. |
@aboch Let me know what you think and I'll add a couple of test cases as well. |
@tgraf apologies on the delay. we will get to this shortly |
Casual ping @mavenugo |
@tgraf we couldn't prioritize this for the 1.10 release. And there are known ipv6 limitations which we are attempting to solve in 1.11 & beyond. Please let us know If you are interested in contributing to ipv6 fixes and bring it in par with the ipv4 handling. |
@mavenugo Sure. What issues are you referring to? |
FYI moby/moby#17513 will do exactly this. The exact detail of what that flag will mean is (of course) up to libnetwork. |
@aidanhs This looked like the right thing to do first but I don't see a reason anymore why the user should be required to opt into IPv6. Whether to use IPv6 or IPv4 or both depends on the infrastructure and should be handled at the driver level. At least that possibility should be given. |
Sure, maybe the default will change, but people may still want to opt out so the flag is still necessary. |
@mavenugo Ping, any interest in going forward with this? |
@@ -263,7 +264,10 @@ As of now libnetwork accepts the following capabilities: | |||
It is a boolean value which tells libnetwork whether the ipam driver needs to know the interface MAC address in order to properly process the `RequestAddress()` call. | |||
If true, on `CreateEndpoint()` request, libnetwork will generate a random MAC address for the endpoint (if an explicit MAC address was not already provided by the user) and pass it to `RequestAddress()` when requesting the IP address inside the options map. The key will be the `netlabel.MacAddress` constant: `"com.docker.network.endpoint.macaddress"`. | |||
|
|||
### SupportsAutoIPv6 | |||
|
|||
This boolean tells libnetwork that the ipam driver is capable of deciding whether an IPv4 address, IPv6 address, or both should be assigned to the container by returning either an address or an empty string in response to `RequestAddress()`. |
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 would not overload this flag with two meaning. The name seems clear, it tells whether the IPAM driver can come up with a IPv6 pool if user did not configure one.
@tgraf Sorry for the back and forth. Looking at your new diffs, I am wondering whether it would be better to follow the approach you are using for request address (driver to return a specific error) for pool request instead, and drop the new capability flag. We would just need to error out in case user has specified a Do you think it makes sense ? Also, your changes would likely not make it for 1.11, code freeze is today, but we are committed to have them in 1.12. Thanks |
@aboch I addressed your feedback and reverted the initial suggestion of the capability. The proposal is now back to the original approach. I left out the gateway opt out decision for now. Let me know what you think. |
ping @nerdalert @mrjana @mavenugo |
ping @aboch |
…es/kubernetes#23090 claims support for IPv6, it turns out that as docker can't give a container an IPv6 address without an IPv4 address, we never hit the code path in kubernetes, so such support doesn't work :(
Allow IPAM plugins to return ipamapi.ErrNoIPReturned to indicate that no address of the particular address family should be assigned. Since this is a newly introduced error, there is no risk of an existing plugin mistakenly switching to this behaviour. Signed-off-by: Thomas Graf <tgraf@suug.ch>
This brings IPv6 in line with IPv4 behaviour which does not require the presence of an ipamV6Config on the network in order to trigger allocating an address. Since the enableIPv6 flag is an opt-in, this change does not alter the default behaviour. The user has clearly indicated intent by explicitly enabling the flag. Signed-off-by: Thomas Graf <tgraf@suug.ch>
@tgraf The way we have to introduce ipv6 only networks is by introducing an |
@mrjana Isn't that option already there in the form of |
Just a vote of support for this. This (and #23090 in kubernetes) are blocking us from adopting k8s/docker as a container solution. |
...and another +1 |
Really need ipv6 only and auto ipv6 allocation. Hard to do IPv6 development w/o these things. |
@mrjana @aboch @tgraf What's the current status of this pr? |
Very much interested in this, too. Especially because we need to turn IPv4 off in various network constellations nowadays |
Would be also interested in this. We should all move to IPv6 only networks. |
Any Update on this? IPv6-only would save me a lot of hassle with exhausted Ipv4-Networks during VPN-Connections at some customers ... |
It would be great to get this into 20.10. This problem alone is making me consider replacing Docker for something more forward thinking |
Which would be? |
podman? |
As far as i can tell podman is just as broken as docker is when it comes to IPv6-only networking because CNI also relies on deprecated NAT technology. (but I'll look into it) |
This makes IPv6 a proper citizen by:
Existing IPv4 behaviour is kept intact: Unless network.enableIPv6 is explicitly set by the user, an IPv4 configuration failure is considered fatal even in the presence of a working IPv6 configuration.