Allow '--link' with '--network bridge'

[robmry]

- What I did

Commit f1048e1 ("Create default EndpointSettings if no --network provided") added an exception for default in applyContainerOptions() ... so that --link isn't copied to endpoint settings if it's a legacy-link option for the default bridge.

But, that missed "--network bridge", or the long form "--network name=bridge,..." (needed to specify endpoint options).

- How I did it

Check for IsUserDefined instead of != "default.

- How to verify it

docker run --rm -ti --network name=bridge,driver-opt=com.docker.network.endpoint.sysctls=net.ipv6.conf.IFNAME.disable_ipv6=1 --name c1 busybox works with this change.

- Description for the changelog

Fix validation of `--link` option.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.42%. Comparing base (ad54f75) to head (2c35778).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5739      +/-   ##
==========================================
+ Coverage   59.04%   59.42%   +0.38%     
==========================================
  Files         344      347       +3     
  Lines       29391    29402      +11     
==========================================
+ Hits        17353    17472     +119     
+ Misses      11064    10958     -106     
+ Partials      974      972       -2     

[thaJeztah]

Thanks! I'll probably need to dig into this one again 🙈 ISTR there was some hairy logic where options for the default network had to be passed to old structs (for compatibility with older daemons), and any network after that to be passed through network-options. And then, there were non-legacy --link options, which are supported for non-default-bridge networks, but in that case implemented through container-scoped aliases (I think?).

[robmry]

And then, there were non-legacy --link options, which are supported for non-default-bridge networks, but in that case implemented through container-scoped aliases (I think?).

Oh, there's a non-legacy --link option?! I guess that's where the confusion's come from, but I'm not sure what non-legacy links are.

In that case, perhaps the fix here should be in parseNetworkAttachmentOpt() (not copying to the endpoint's options if !IsUserDefined, rather than only if it's not "default").

[thaJeztah]

Yeah, there is; when we reimplemented networks using the embedded DNS, we kept --link as option, but changed the implementation to be DNS based instead of /etc/hosts;

docker network create mybridge

docker run  -d --rm --network=mybridge --name webserver nginx:alpine

docker run  -it --rm --network=mybridge --link webserver:webbie nginx:alpine ping webbie
PING webbie (172.19.0.2): 56 data bytes
64 bytes from 172.19.0.2: seq=0 ttl=64 time=0.783 ms
64 bytes from 172.19.0.2: seq=1 ttl=64 time=0.125 ms
64 bytes from 172.19.0.2: seq=2 ttl=64 time=0.153 ms
^C
--- webbie ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 0.125/0.353/0.783 ms

[robmry]

Right - yes - I see it adds an alias ... https://github.com/moby/moby/blob/d7ad7a9534afb673fcbb6dabb64bae3be720e2b8/daemon/network.go#L1121-L1127

That makes sense - I'll fix this fix.

[akerouanton]

As discussed during our 1-1, it'd be great to add a test for that.

[vvoland]

LGTM; but would appreciate review from @akerouanton :)