Skip to content
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

[Bug]: MappedPort potentially does not return correct value for IPv6 enabled systems #551

Closed
mdonkers opened this issue Oct 5, 2022 · 11 comments
Labels
bug An issue with the library

Comments

@mdonkers
Copy link
Contributor

mdonkers commented Oct 5, 2022

Testcontainers version

0.14.0

Using the latest Testcontainers version?

Yes

Host OS

Linux

Host Arch

x86

Go Version

1.19

Docker version

$ docker version
Client: Docker Engine - Community
 Version:           22.06.0-beta.0
 API version:       1.42
 Go version:        go1.18.3
 Git commit:        3e9117b
 Built:             Fri Jun  3 17:56:12 2022
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          22.06.0-beta.0
  API version:      1.42 (minimum version 1.12)
  Go version:       go1.18.3
  Git commit:       38633e7
  Built:            Fri Jun  3 17:56:12 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.8
  GitCommit:        9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6
 runc:
  Version:          1.1.4
  GitCommit:        v1.1.4-0-g5fd4c4d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Docker info

$ docker info
Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.9.1
    Path:     /home/miel/.docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.10.2
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 3
  Running: 0
  Paused: 0
  Stopped: 3
 Images: 15
 Server Version: 22.06.0-beta.0
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 io.containerd.runtime.v1.linux runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 9cd3357b7fd7218e4aec3eae239db1f68a5a6ec6
 runc version: v1.1.4-0-g5fd4c4d
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 5.19.0-1-amd64
 Operating System: Debian GNU/Linux bookworm/sid
 OSType: linux
 Architecture: x86_64
 CPUs: 16
 Total Memory: 31.09GiB
 Name: housepaper
 ID: TKCM:VE5M:466R:AMPV:XPP3:CZ45:BZRN:N6WX:YR4E:ZBR5:VYDD:5SWD
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Username: miel
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

What happened?

Trying to run integration tests for the project https://github.com/ClickHouse/clickhouse-go, it times out as container fails to start up.
Analysing, the startup checks fail because the port and IP address (picked by Golang net.Dialer) does not match. localhost is used but the IPv4 port is returned and the [::1] address.

Local hosts file:

$ cat /etc/hosts
127.0.0.1        localhost
127.0.1.1        housepaper
::1              localhost ip6-localhost ip6-loopback
ff02::1          ip6-allnodes
ff02::2          ip6-allrouters

Docker container running:
image
Port 33789 is mapped to the IPv4 interface (0.0.0.0), port 33788 is mapped to IPv6 ([::1]).

When using MappedPort(), just the first port is always returned, which happens to be the IPv4 one (

return nat.NewPort(k.Proto(), p[0].HostPort)
)
But Golang, given the net.DialContext (https://cs.opensource.google/go/go/+/master:src/net/dial.go;l=428;drc=432158b69a50e292b625d08dcfacd0604acbabd3) happens to pick IPv6 as primary and try to connect (which I don't fully understand why it succeeds, but at least generates exceptions on both server and client side).
image

The solution could (hopefully) be to add methods explicitly returning the interface for which the port is returned. Also the wait.For... methods such as wait.ForSQL need to get an additional parameter with the IP though, so it can connect to the explicit address.

Relevant log output

No response

Additional Information

No response

@mdelapenya
Copy link
Member

@mdonkers thanks for such detailed report. We appreciate your time

Would you be willing to contribute the enhancement?

@mdonkers
Copy link
Contributor Author

mdonkers commented Oct 6, 2022

Hi,

I can give that a shot. But not sure if you have any proposals on your end how the API should look like?

The wait.ForSQL method e.g. could take an interface / IP parameter next to the port. But since Golang doesn't allow for method overloading, that either means a backwards incompatible change, or different method name. Same for the MappedPort method.

What actually might be easiest, is allow specifying tcp4 / tcp6 next to tcp as network type, as Golang itself supports.
Retrieving an existing port mapping should be fairly easy.
Difficulty would be how to handle the CreateContainer with the tcp4 / tcp6 names as it probably needs to be filtered and I don't know how to specify to Docker which interface it must bind to for flexible host ports. Next to that if users specify the full mapping explicitly, the interface might not match the network type.

Would it be allowed to only take the tcp4/tcp6 on 'query' side, so e.g. for MappedPort() and the wait methods, and not when creating a container? Then I can at least start a PR for this.

@mdelapenya
Copy link
Member

The wait.ForSQL method e.g. could take an interface / IP parameter next to the port. But since Golang doesn't allow for method overloading, that either means a backwards incompatible change, or different method name. Same for the MappedPort method.

Regarding the SQL host missing, we merged #524 very recently, although not released yet. Would it resolve that part?

What actually might be easiest, is allow specifying tcp4 / tcp6 next to tcp as network type, as Golang itself supports. Retrieving an existing port mapping should be fairly easy. Difficulty would be how to handle the CreateContainer with the tcp4 / tcp6 names as it probably needs to be filtered and I don't know how to specify to Docker which interface it must bind to for flexible host ports. Next to that if users specify the full mapping explicitly, the interface might not match the network type.

On this, the MappedPort accepts a nat.Port parameter, so you could specify a tcp4/tcp6 port as part of the string representation of the port.

Would it be allowed to only take the tcp4/tcp6 on 'query' side, so e.g. for MappedPort() and the wait methods, and not when creating a container? Then I can at least start a PR for this.

I'm sorry but I'm not following you on this, my bad. Could you please add links to the places where tcp is used 🙏 ? That would help me in understanding the issue with more detail.

Thanks in advance

@mdonkers
Copy link
Contributor Author

mdonkers commented Oct 7, 2022

The wait.ForSQL method e.g. could take an interface / IP parameter next to the port. But since Golang doesn't allow for method overloading, that either means a backwards incompatible change, or different method name. Same for the MappedPort method.

Regarding the SQL host missing, we merged #524 very recently, although not released yet. Would it resolve that part?

I don't think so, as this appears to be the hostname as given by Docker. I think that would still be ambiguous and does not map the interface for which the port is retrieved. See

return nat.NewPort(k.Proto(), p[0].HostPort)
, which just fetches the first port mapping found and does not care for whether the interface is IPv4 or IPv6 for example.

What actually might be easiest, is allow specifying tcp4 / tcp6 next to tcp as network type, as Golang itself supports. Retrieving an existing port mapping should be fairly easy. Difficulty would be how to handle the CreateContainer with the tcp4 / tcp6 names as it probably needs to be filtered and I don't know how to specify to Docker which interface it must bind to for flexible host ports. Next to that if users specify the full mapping explicitly, the interface might not match the network type.

On this, the MappedPort accepts a nat.Port parameter, so you could specify a tcp4/tcp6 port as part of the string representation of the port.

Yes, correct. But currently tcp4 / tcp6 are not supported and at least would need more logic to return the correct port again as in

return nat.NewPort(k.Proto(), p[0].HostPort)

That would be my intend for the PR and guess the simplest solution.

Would it be allowed to only take the tcp4/tcp6 on 'query' side, so e.g. for MappedPort() and the wait methods, and not when creating a container? Then I can at least start a PR for this.

I'm sorry but I'm not following you on this, my bad. Could you please add links to the places where tcp is used pray ? That would help me in understanding the issue with more detail.

Thanks in advance

When you create a container, code looks something like;

	req := testcontainers.ContainerRequest{
		Image:        "clickhouse/clickhouse-server:head-alpine",
		ExposedPorts: []string{"9000/tcp"},
		WaitingFor:   wait.ForSQL(nat.Port("9000/tcp"), "clickhouse", dbURL).Timeout(time.Second * 10),
	}

If you follow where those ExposedPorts is being used, you end up in a different Docker library where 9000/tcp is parsed. It won't accept 9000/tcp4. And even if we'd support, (besides trimming off the 4 or 6) not sure how to handle it.
Docker should support defining mapped ports as something like 9000:0.0.0.0:9000 and 9000:[::1]:9000, but that requires specifying the host port explicitly while most folks I assume want flexible ports.

@settings settings bot removed the type/bug label Oct 26, 2022
@mdelapenya mdelapenya added the bug An issue with the library label Oct 26, 2022
@kiview
Copy link
Member

kiview commented Nov 23, 2022

We observed this behavior in other language implementations as well (see dotnet/Docker.DotNet#565) and it seems the root cause is this upstream bug in Docker/Moby:
moby/moby#42442

@mdonkers
Copy link
Contributor Author

As I'm researching in a bit more I'm finding some more Docker dual-stack networking issues. But also not yet 100% clear to me, if either really a Docker bug or expected behaviour.

So I'll dive in a bit more and see whether the above proposed solution still makes sense, or whether it should be resolved in some other place.

@mdelapenya
Copy link
Member

Thanks for your time here @mdonkers, please let us know about your findings 🙏

@HofmeisterAn
Copy link
Contributor

HofmeisterAn commented Jan 26, 2023

Docker creates two port bindings while creating a container. One for IPv4 and another one for IPv6. This is probably the case when the port binding definition does not contain a host IP. The public assigned ports for IPv4 and IPv6 are not always the same. Probably they are not using the IPV6_V6ONLY socket option.

We do not know which IP address the system resolves for localhost, right? Is it 127.0.0.1 or ::1? Depending on the IP we need to choose the right port mapping.

For .NET (for now) I changed the host resolution to 127.0.0.1 instead of localhost. This seems to run much more reliable. If anyone relies on IPv6, they can still use the host override custom configuration. I am thinking about changing the port resolution in the future to something like:

// Contains two items:
// IsIPv6 ::1
// IsIPv4 127.0.0.1
var ips = Dns.GetHostAddresses("localhost");

var httpPort = 80;
var httpPortKey = httpPort + "/tcp";
var ipv4Binding = new PortBinding { HostIP = "0.0.0.0", HostPort = string.Format(CultureInfo.CurrentCulture, "{0}", ++httpPort) };
var ipv6Binding = new PortBinding { HostIP = "::1", HostPort = string.Format(CultureInfo.CurrentCulture, "{0}", ++httpPort) };

IDictionary<string, IList<PortBinding>> mappedPortBindings = new Dictionary<string, IList<PortBinding>>();
mappedPortBindings.Add(new KeyValuePair<string, IList<PortBinding>>(httpPortKey, new List<PortBinding> { ipv4Binding, ipv6Binding }));
mappedPortBindings.TryGetValue(httpPortKey, out var portBindings);

// Lets imagine Dns.GetHostAddresses(string) returns the preferred order.
var addressFamilies = ips
  .Select(ip => ip.AddressFamily)
  .ToList();

// We can order the port bindings due to their address family, and use the first item.
var portBinding = portBindings
  .Select(portBinding => new IPEndPoint(IPAddress.Parse(portBinding.HostIP), ushort.Parse(portBinding.HostPort, NumberStyles.None, CultureInfo.InvariantCulture)))
  .OrderBy(portBinding => addressFamilies.IndexOf(portBinding.AddressFamily))
  .First();

@mdonkers
Copy link
Contributor Author

I'm going to close this as unfortunately I did not find the time to look into this. And it seems to be a kind of 'special' issue where resolution is not that straightforward.

Feel free to re-open in case someone does really plan to pick this up.

@mdonkers mdonkers closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2023
@mdonkers
Copy link
Contributor Author

Minor update (as I did happen to run into this again):

An easy fix / work-around is to specify exposed ports as

ExposedPorts: []string{"0.0.0.0::9090/tcp"},

Then the container will only bind to IPv4, so no issues of two networks / overlapping ports.

@mitsos1os
Copy link

Minor update (as I did happen to run into this again):

An easy fix / work-around is to specify exposed ports as

ExposedPorts: []string{"0.0.0.0::9090/tcp"},

Then the container will only bind to IPv4, so no issues of two networks / overlapping ports.

@mdonkers Thanks for that! It works correctly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

No branches or pull requests

5 participants