Handle IPv6 entries in discovery #20842

Merged
merged 2 commits into from Mar 2, 2016

Conversation

Projects
None yet
6 participants
@dongluochen
Contributor

dongluochen commented Mar 2, 2016

This is part of fix for swarm issue #1906. Current discovery doesn't handle IPv6 entry properly. Entry [2001:db8:0:f101::2]:4444 submitted to discovery would be returned as 2001:db8:0:f101::2:4444. This change uses net.JoinHostPort to deal with this format issue. Unit tests are updated.

Signed-off-by: Dong Chen dongluo.chen@docker.com

Handle IPv6 entries.
Signed-off-by: Dong Chen <dongluo.chen@docker.com>
@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Mar 2, 2016

Contributor

LGTM

Contributor

aaronlehmann commented Mar 2, 2016

LGTM

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Mar 2, 2016

Contributor

I did a grep and noticed other places in Docker where IP addresses and ports may be joined improperly.

api/client/port.go:                             fmt.Fprintf(cli.out, "%s:%s\n", frontend.HostIP, frontend.HostPort)
api/client/port.go:                     fmt.Fprintf(cli.out, "%s -> %s:%s\n", from, frontend.HostIP, frontend.HostPort)
vendor/src/github.com/docker/libnetwork/network.go:                                     return types.BadRequestErrorf("non parsable secondary ip address (%s:%s) passed for network %s", k, v, n.Name())
vendor/src/github.com/docker/libnetwork/network.go:                                     return types.InternalErrorf("failed to allocate secondary ip address (%s:%s): %v", k, v, err)
pkg/discovery/backends.go:      addr = fmt.Sprintf("%s:%s", addr, port)

I'm not sure offhand whether most of these are problematic, but the last one on the list looks like something worth fixing in this PR.

Contributor

aaronlehmann commented Mar 2, 2016

I did a grep and noticed other places in Docker where IP addresses and ports may be joined improperly.

api/client/port.go:                             fmt.Fprintf(cli.out, "%s:%s\n", frontend.HostIP, frontend.HostPort)
api/client/port.go:                     fmt.Fprintf(cli.out, "%s -> %s:%s\n", from, frontend.HostIP, frontend.HostPort)
vendor/src/github.com/docker/libnetwork/network.go:                                     return types.BadRequestErrorf("non parsable secondary ip address (%s:%s) passed for network %s", k, v, n.Name())
vendor/src/github.com/docker/libnetwork/network.go:                                     return types.InternalErrorf("failed to allocate secondary ip address (%s:%s): %v", k, v, err)
pkg/discovery/backends.go:      addr = fmt.Sprintf("%s:%s", addr, port)

I'm not sure offhand whether most of these are problematic, but the last one on the list looks like something worth fixing in this PR.

Use net.JoinHostPort to handle address format.
Signed-off-by: Dong Chen <dongluo.chen@docker.com>
@dongluochen

This comment has been minimized.

Show comment
Hide comment
@dongluochen

dongluochen Mar 2, 2016

Contributor

Thanks @aaronlehmann. Fix the last one in pkg/discovery/backends.go.

Contributor

dongluochen commented Mar 2, 2016

Thanks @aaronlehmann. Fix the last one in pkg/discovery/backends.go.

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann Mar 2, 2016

Contributor

Re-affirming LGTM.

Contributor

aaronlehmann commented Mar 2, 2016

Re-affirming LGTM.

@icecrime

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Mar 2, 2016

Contributor

Good catch, LGTM!

Contributor

icecrime commented Mar 2, 2016

Good catch, LGTM!

calavera added a commit that referenced this pull request Mar 2, 2016

Merge pull request #20842 from dongluochen/IPv6Support
Handle IPv6 entries in discovery

@calavera calavera merged commit 266a75a into moby:master Mar 2, 2016

8 checks passed

docker/dco-signed All commits signed
Details
documentation success 1 tests run, 1 skipped, 0 failed.
Details
experimental Jenkins build Docker-PRs-experimental 15697 has succeeded
Details
gccgo Jenkins build Docker-PRs-gccgo 2593 has succeeded
Details
janky Jenkins build Docker-PRs 24484 has succeeded
Details
userns Jenkins build Docker-PRs-userns 6834 has succeeded
Details
win2lin Jenkins build Docker-PRs-Win2Lin 22606 has succeeded
Details
windowsTP4 Jenkins build Docker-PRs-WoW-TP4 1906 has succeeded
Details

@dongluochen dongluochen deleted the dongluochen:IPv6Support branch Mar 2, 2016

@shmcfarl

This comment has been minimized.

Show comment
Hide comment
@shmcfarl

shmcfarl Mar 29, 2016

Hey gang. This certainly fixed the issue with a 'swarm join' but I seem to have an issue with 'swarm manage' and pointing to etcd. Was this fix supposed address all of the IPv6 address entry issues in Docker or should I file a issue specific to 'swarm manage'?

`docker run -p 4000:2375 swarm manage etcd://[2001:db8:cafe:14::a]:2379/swarm
ERRO[1306] Handler for POST /v1.23/containers/create returned error: No such image: swarm:latest
Unable to find image 'swarm:latest' locally
latest: Pulling from library/swarm

25da0aa87182: Pull complete
45707a9f4c2b: Pull complete
7f0c09406c8f: Pull complete
a3ed95caeb02: Pull complete
Digest: sha256:5f2b4066b2f7e97a326a8bfcfa623be26ce45c26ffa18ea63f01de045d2238f3
Status: Downloaded newer image for swarm:latest
time="2016-03-16T21:46:09Z" level=info msg="Initializing discovery without TLS"
time="2016-03-16T21:46:09Z" level=info msg="Listening for HTTP" addr=":2375" proto=tcp
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: client: etcd cluster is unavailable or misconfigured"
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: client: etcd cluster is unavailable or misconfigured"
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: Unexpected watch error"`

Hey gang. This certainly fixed the issue with a 'swarm join' but I seem to have an issue with 'swarm manage' and pointing to etcd. Was this fix supposed address all of the IPv6 address entry issues in Docker or should I file a issue specific to 'swarm manage'?

`docker run -p 4000:2375 swarm manage etcd://[2001:db8:cafe:14::a]:2379/swarm
ERRO[1306] Handler for POST /v1.23/containers/create returned error: No such image: swarm:latest
Unable to find image 'swarm:latest' locally
latest: Pulling from library/swarm

25da0aa87182: Pull complete
45707a9f4c2b: Pull complete
7f0c09406c8f: Pull complete
a3ed95caeb02: Pull complete
Digest: sha256:5f2b4066b2f7e97a326a8bfcfa623be26ce45c26ffa18ea63f01de045d2238f3
Status: Downloaded newer image for swarm:latest
time="2016-03-16T21:46:09Z" level=info msg="Initializing discovery without TLS"
time="2016-03-16T21:46:09Z" level=info msg="Listening for HTTP" addr=":2375" proto=tcp
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: client: etcd cluster is unavailable or misconfigured"
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: client: etcd cluster is unavailable or misconfigured"
time="2016-03-16T21:46:09Z" level=error msg="Discovery error: Unexpected watch error"`

@dongluochen

This comment has been minimized.

Show comment
Hide comment
Contributor

dongluochen commented Mar 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment