Expose bridge IPv6 setting to docker network inspect

[aidanhs]

Using docker network inspect bridge.

Before:

        "options": {
            "com.docker.network.bridge.default_bridge": "true",
            "com.docker.network.bridge.enable_icc": "true",
            "com.docker.network.bridge.enable_ip_masquerade": "true",
            "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0",
            "com.docker.network.bridge.name": "docker0",
            "com.docker.network.driver.mtu": "1500"
        }

After:

        "options": {
            "com.docker.network.bridge.default_bridge": "true",
            "com.docker.network.bridge.enable_icc": "true",
            "com.docker.network.bridge.enable_ip_masquerade": "true",
            "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0",
            "com.docker.network.bridge.name": "docker0",
            "com.docker.network.driver.mtu": "1500",
            "com.docker.network.enable_ipv6": "false"
        }

[aidanhs]

I think this is @mavenugo related?

[aboch]

netOption is really a collection of per network specific options. We consider EnableIPv6 a driver specific option instead, which is why it is being passed down separately from the network options.

[aidanhs]

I don't understand the distinction. Unless I've misread the libnetwork code, it all ends up as properties on the network struct...so I'm not sure what it means to be 'driver specific'.

Regardless, this is just making the default bridge construction be more similar to manually created networks (for consistency).

[aidanhs]

To elaborate more:

root@lp01679:/usr/bin# docker network create -o com.docker.network.enable_ipv6=true newbridge
13fc7ada2827d349d21a2c32fdd5a6fec32b6c0b8f591b7ec80d1dc4156146e1
root@lp01679:/usr/bin# docker network inspect newbridge
[...]
        "options": {
            "com.docker.network.enable_ipv6": "true"
        }
    }
]
root@lp01679:/usr/bin# ip addr
[...]
394: br-13fc7ada2827: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN 
    link/ether 02:42:8b:20:12:de brd ff:ff:ff:ff:ff:ff
    inet 172.18.0.1/16 scope global br-13fc7ada2827
       valid_lft forever preferred_lft forever
    inet6 fe80::1/64 scope link tentative 
       valid_lft forever preferred_lft forever
root@lp01679:/usr/bin# docker network rm newbridge
root@lp01679:/usr/bin# docker network create -o com.docker.network.enable_ipv6=false newbridge
410ec2597a8ed01125fc113ab1c51c37e55d481df2d82a50bd731b95702cfa0f
root@lp01679:/usr/bin# docker network inspect newbridge
[...]
        "options": {
            "com.docker.network.enable_ipv6": "false"
        }
    }
]
root@lp01679:/usr/bin# ip addr
[...]
395: br-410ec2597a8e: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN 
    link/ether 02:42:71:ff:88:ed brd ff:ff:ff:ff:ff:ff
    inet 172.18.0.1/16 scope global br-410ec2597a8e
       valid_lft forever preferred_lft forever

i.e. you can detect ipv6 on a manually created network by looking at labels. It doesn't make sense to me that the default bridge would be a special case.

In fact, this is the only place in Docker where NetworkOptionGeneric is used! Although some of the labels might partition into driver-specific options, network specific options and generic options, that's a decision for the libnetwork internals to make.

Of course, I've only been playing around libnetwork for a day, so I may be misunderstanding.

[mavenugo]

@aidanhs apologies on the delayed response. As you know libnetwork doesn't work on driver specific options/labels. Driver options are transparently sent to the drivers/plugins. Whereas libnetwork handles some of the generic labels such as enable_ipv6 to provide driver independent functionality. In this case, enable_ipv6 is used to update DNS records. That is the history behind keeping the enable_ipv6 flag as a driver independent flag (but still send it to the driver after libnetwork processing).
Having said that, I think there is an issue for user-defined networks wherein libnetwork doesn't process the this flag. I will confirm.

@mrjana can add more details to this.

[LK4D4]

ping @mavenugo @aboch

[crosbymichael]

ping @mavenugo @aboch @mrjana yes/no/idk?

[aidanhs]

Maybe some commentary will help clarify this PR:

1. the original intent was the desire to see the ipv6 enabled status as part of the default bridge settings, as you can for custom networks (as demonstrated in Expose bridge IPv6 setting to docker network inspect #17513 (comment)). Looking at the libnetwork design document, this seems to mean turning the "option" into a "label". To reiterate: it is already a label for manually created networks.
2. as part of looking around the code, I observed that the default bridge is constructed in a different way to manually created networks (CreateNetwork in daemon/network.go). This motived the change in this PR to use NetworkOptionDriverOpts, which also fixes point 1.

If the reviewer believes that consistency is a good thing (i.e. agrees with these two points) then I think this PR is a step forward.

From a libnetwork design point of view, I'm not convinced that consumers of the library should be worrying about what kind of option something is, or how it's implemented behind the scenes - you just know that you want a network with setting S set to a value V. As things currently stand, libnetwork doesn't offer any guidance for choosing what type of option is appropriate (nor is anything enforced, nor are different types of options treated differently - both NetworkOptionDriverOpts and NetworkOptionGeneric are aware of the ipv6 option). Following this PR, I would therefore like to discuss (on the libnetwork repo) what value is provided by having these different types of options, because the topic is not touched on in the design document.

[thaJeztah]

ping @mavenugo @aboch @mrjana could you have a look, and provide some input? (or a (not)LGTM)

@aidanhs looks like this needs a rebase (just something I noted, but no mayor issue, because it's still in design review)

[tiborvass]

Ping @mavenugo @mrjana for design

[mavenugo]

@tiborvass @aidanhs as explained in #17513 (comment), this is a libnetwork options processing design especially providing a way for driver specific vs driver independent options. We had a few issues with this in the past (due to the labels vs options support).
@mrjana will have more things to say on this topic.

[aidanhs]

this is a libnetwork options processing design especially providing a way for driver specific vs driver independent options

Just so I'm completely clear, you're saying that the split between driver specific and driver independent options (i.e. forcing them to be two different parameters to libnetwork) was a decision made to resolve some issues related to labels vs options?

The problem is that library consumers (like docker) now need to keep their own record of which options are driver specific/independent. This issue (com.docker.network.enable_ipv6 only appearing in docker network inspect for user-defined networks) is caused by exactly that - someone didn't partition the options beforehand and so all options defined by the user are passed as 'driver-specific' options.

Having said that, I think there is an issue for user-defined networks wherein libnetwork doesn't process the this flag. I will confirm.

As it happens the flag is actually processed for user networks, despite their creation not conforming to the intended design (where options should be split before being passed) - https://github.com/docker/docker/blob/3d82564c7a6ae139879b869a987bf9274ed0275d/daemon/network.go#L103. I assume this is a (happy) accident - when I read the libnetwork code a while ago, it seemed to treat driver-specific and driver-independent options in the same way, it just had a list of settings it would look out for.

You can see some evidence of this working by comparing the output of ip addr:

root@lp01679:~# docker network create -o com.docker.network.enable_ipv6=false x
149adf318ef59feb4eb1d8d6bd2ca1fb4e1eb177cdff9963457a9b8cd9a9aeff
root@lp01679:~# docker run --net x --rm ubuntu:14.04.3 ip addr show eth0
67: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default 
    link/ether 02:42:ac:11:00:02 brd ff:ff:ff:ff:ff:ff
    inet 172.17.0.2/16 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::42:acff:fe11:2/64 scope link 
       valid_lft forever preferred_lft forever
root@lp01679:~# docker network rm x
root@lp01679:~# docker network create -o com.docker.network.enable_ipv6=true x
d1ea63fc81e7c26f8ad63114b3839bb746dda84c4ad32396b72e48730ee2e0d8
root@lp01679:~# docker run -it --net x --rm ubuntu:14.04.3 ip addr show eth0
62: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default 
    link/ether 02:42:ac:12:00:02 brd ff:ff:ff:ff:ff:ff
    inet 172.18.0.2/16 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::42:acff:fe12:2/64 scope link 
       valid_lft forever preferred_lft forever
    inet6 fe80::242:ac12:2/64 scope link 
       valid_lft forever preferred_lft forever

Note the extra inet6 address for the latter.

Just as a reminder of the issue I actually want fixed: com.docker.network.enable_ipv6 should be consistently displayed (or not displayed) for both the default bridge and user-created networks. At this point I really don't mind how it's resolved, I just think consistency is good. Perhaps your preferred solution (assuming you like the driver independant/dependant split) is to add some code in libnetwork to recombine everything into a consistent view of labels to display to the user.

I appreciate your responses and look forward to the response from @mrjana, but if you're sure that you're not going to merge the PR in its current state then I encourage you to just close it - I will raise an appropriate issue against docker and you can decide how to resolve it in line with your preferred design.

[thaJeztah]

ping @mrjana can you have a look at this one please? thanks!

[mrjana]

@aidanhs @mavenugo Let me give some context on what we actually want to do here regardless of what state the current implementation is in. Before 1.9 we only had default bridge and hence all the optional flags that were meant for the bridge driver were either just provided as a daemon option if it applies to all containers using bridge or in docker run if it is container specific.

One such daemon option is --ipv6 which is introduced to enable ipv6 connectivity in default bridge network. But with the introduction of multiple networks this should really be a per-network option except that for backward compatibility we cannot really remove that daemon option.

So the decision we made is that all the existing daemon wide networking options which existed pre-1.9 will only be applicable to default bridge network(with the exception of some options which are truly global and will apply to all bridge networks)

For all the other non-default bridge networks these options have to specified during the network create time. So in theory we should have --ipv6 flag available as part of network create command and ipv6 connectivity should only be enabled in that network only if that flag is present. Not as it is currently done today where the daemon level --ipv6 flag is applied on all networks.

Once we have the --ipv6 option in the network command as a first class option, then there is no need to expose the label (as we do it today) for any network since it should show up as a first class configuration info for that network. We may still pass this information down to the driver in some arbitrary way but that doesn't need to be exposed in the UI.

So to me the design roughly should like this:

• Add a --ipv6 option at the network level. It is for any network and if the backing driver cannot support ipv6 it has the option to reject the configuration and hence result in network creation failure.
• Remove accepting ipv6 configuration via labels
• Remove listing this as part of labels list in network inspect for any network
• Add a field in network inspect to show the first class ipv6 configuration for every network including default bridge network.

Thoughts?

[mavenugo]

@mrjana thanks for the clarification and your suggestion make sense to me.

[aidanhs]

So the decision we made is that all the existing daemon wide networking options which existed pre-1.9 will only be applicable to default bridge network(with the exception of some options which are truly global and will apply to all bridge networks)

Makes sense.

So to me the design roughly should like this:

• Add a --ipv6 option at the network level. It is for any network and if the backing driver cannot support ipv6 it has the option to reject the configuration and hence result in network creation failure.
• Remove accepting ipv6 configuration via labels
• Remove listing this as part of labels list in network inspect for any network
• Add a field in network inspect to show the first class ipv6 configuration for every network including default bridge network.

This is a way forward and it's better from the status quo, so +1 from that point of view.

However, my personal vague feeling is that labels are great! Why create another way of passing options down to drivers when you have an extensible, decent method already available to you? .ini files are just a string -> string set of keys and values, and they work pretty well for general configuration - I'm not sure what making ipv6 a first-class option buys you. If you like --ipv6, couldn't you use it as sugar for defining the label?

Or is it about the conceptual difference between labels and options (which I've not yet quite grasped)?

[mrjana]

@aidanhs I think the term labels has been used to mean many things. But in docker it is a means to assign a tag/alias to a particular object whether it be an image or container or may be in the future to networks etc. What we really have in libnetwork is driver options(I guess the usage of the term netlabel there is the source of confusion) and as the name says they are driver specific options which only make sense for a particular driver and as such these should only be used to specify driver specific parameters.

If there is some option which is driver independent, then that should be really a first class option in docker api and cli. --ipv6 belongs in that category and that is the reasoning behind what design I suggested.

If you agree with the above rough sketch, can you please rework the PR to address the design or close this one and create another one which addresses the problem the correct way?

[icecrime]

Ping @aidanhs! Thanks :-)

[aidanhs]

I see, so you're coming at it from the point of view that the number of driver-independent options is finite (and small), so they can all be supported as first-class values without asking too much of libnetwork consumers to keep track of them.

I'm not sure I 100% agree with the proposed route (and feel that you may end up redesigning how options work), but I understand the justification and can see it's an improvement so I'll make the required changes to this PR so forward progress can be made. People actually using libnetwork can then give feedback in the future for incremental improvement.

While I'm making the changes, I'm curious - what is the significance of the unique com.docker.network.driver.mtu name? Driver-specific options appear to replace driver with the name of a driver (like bridge) and driver-independent options simply omit driver entirely. Is mtu a special case?

[mrjana]

@aidanhs Thanks for moving this forward.

com.docker.network.driver.mtu also falls in the same bucket as ipv6. It needs to become generic as it is applicable to all drivers. Once we do that we can remove this label as well.

[thaJeztah]

ping @aidanhs what's the status on this one; are you still working on this?

[mavenugo]

@aidanhs #20214 brings in your changes from libnetwork. once this is merged you can rebase this PR.
cc @icecrime

[aidanhs]

Thanks @mavenugo!

[calavera]

LGTM

[aboch]

LGTM

[calavera]

@aidanhs This is going to need docs.

[aidanhs]

I've just added docs as a followup commit for ease of review.

[icecrime]

Thanks! Moving to docs review.

[vdemeester]

I'm not sure what removing this from there though ? It's still a valid option when creating a bridge network right ?

[aidanhs]

The context is the paragraph above

When creating a custom network, the default network driver (i.e. bridge) has additional options that can be passed. The following are those options and the equivalent docker daemon flags used for docker0 bridge:

com.docker.network.enable_ipv6 is no longer a valid 'driver option' (also known as 'label') and it will be ignored, so I've removed this line. Instead, --ipv6 is a valid option for all networks.

[vdemeester]

@aidanhs There is a few update to do in the docs/reference/api/docker_remote_api_v1.23.md (https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api_v1.23.md#25-networks — mainly in the response) and also probably one in docs/reference/api/docker_remote_api.md for the changes (https://github.com/docker/docker/blob/master/docs/reference/api/docker_remote_api.md#v123-api-changes)

/cc @thaJeztah

[thaJeztah]

What does the "equivalent" column relate to here? Is that the daemon flag?

[mavenugo]

Also, docker network create takes in few more flags that are applicable to any driver such as subnet, ip-range, gateway & others. They are all already documented in docs/reference/commandline/network_create.md. am not sure if we need to repeat it here.

[aidanhs]

@thaJeztah yes, this is in the context of the table above which says "and the equivalent docker daemon flags used for docker0 bridge" above it.

@mavenugo true. Want me to remove them? If so, I'll add a sentence above the exiting table saying something like "There are also arguments to docker network create applicable to any network driver, which can be found on the reference page for that command."

[mavenugo]

@aidanhs thanks. Actually, the table is infact useful for those who like to compare the existing daemon flags with the network flags. So, maybe you can include all the options and compare them ?
For example --ip-range is equivalent to --fixed-cidr, the --subnet is equivalent to the subnet in bip , etc.

[thaJeztah]

We may want to add a small bit to explain "what" they're equivalent to (i.e. that the "equivalent" column shows the equivalent docker daemon option)?

[thaJeztah]

Okay, we discussed this; to prevent more rebase hell, well merge this, and look at the final changes in a follow up

LGTM

[calavera]

docs LGTM. Let's follow up with another PR to add more information to that table with other options.