From b41e2d4dc1d940aed60a49bac5b9f7dcfada7373 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 29 Jan 2021 17:28:46 +0100 Subject: [PATCH 1/3] integration/container: wrap some long lines for readability Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 54ca929a709553bb911661abfc60901093ad3eab) Signed-off-by: Sebastiaan van Stijn --- integration/container/nat_test.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/integration/container/nat_test.go b/integration/container/nat_test.go index f33c904480..a8d1134e4d 100644 --- a/integration/container/nat_test.go +++ b/integration/container/nat_test.go @@ -70,7 +70,11 @@ func TestNetworkLoopbackNat(t *testing.T) { client := testEnv.APIClient() ctx := context.Background() - cID := container.Run(ctx, t, client, container.WithCmd("sh", "-c", fmt.Sprintf("stty raw && nc -w 1 %s 8080", endpoint.String())), container.WithTty(true), container.WithNetworkMode("container:"+serverContainerID)) + cID := container.Run(ctx, t, client, + container.WithCmd("sh", "-c", fmt.Sprintf("stty raw && nc -w 1 %s 8080", endpoint.String())), + container.WithTty(true), + container.WithNetworkMode("container:"+serverContainerID), + ) poll.WaitOn(t, container.IsStopped(ctx, client, cID), poll.WithDelay(100*time.Millisecond)) @@ -92,15 +96,19 @@ func startServerContainer(t *testing.T, msg string, port int) string { client := testEnv.APIClient() ctx := context.Background() - cID := container.Run(ctx, t, client, container.WithName("server-"+t.Name()), container.WithCmd("sh", "-c", fmt.Sprintf("echo %q | nc -lp %d", msg, port)), container.WithExposedPorts(fmt.Sprintf("%d/tcp", port)), func(c *container.TestContainerConfig) { - c.HostConfig.PortBindings = nat.PortMap{ - nat.Port(fmt.Sprintf("%d/tcp", port)): []nat.PortBinding{ - { - HostPort: fmt.Sprintf("%d", port), + cID := container.Run(ctx, t, client, + container.WithName("server-"+t.Name()), + container.WithCmd("sh", "-c", fmt.Sprintf("echo %q | nc -lp %d", msg, port)), + container.WithExposedPorts(fmt.Sprintf("%d/tcp", port)), + func(c *container.TestContainerConfig) { + c.HostConfig.PortBindings = nat.PortMap{ + nat.Port(fmt.Sprintf("%d/tcp", port)): []nat.PortBinding{ + { + HostPort: fmt.Sprintf("%d", port), + }, }, - }, - } - }) + } + }) poll.WaitOn(t, container.IsInState(ctx, client, cID, "running"), poll.WithDelay(100*time.Millisecond)) From 86d98f5711ea4350dce6398339df5906eeaf62fe Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 10 Feb 2021 11:31:18 +0100 Subject: [PATCH 2/3] integration: update getExternalAddress to prefer IPv4 Rootlesskit doesn't currently handle IPv6 addresses, causing TestNetworkLoopbackNat and TestNetworkNat to fail; Error starting userland proxy: error while calling PortManager.AddPort(): listen tcp: address :::8080: too many colons in address This patch: - Updates `getExternalAddress()` to pick IPv4 address if both IPv6 and IPv4 are found - Update TestNetworkNat to net.JoinHostPort(), so that square brackets are used for IPv6 addresses (e.g. `[::]:8080`) Signed-off-by: Sebastiaan van Stijn (cherry picked from commit f845b98ca675c9e848a27f135eac9cd31aac78e5) Signed-off-by: Sebastiaan van Stijn --- integration/container/nat_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/integration/container/nat_test.go b/integration/container/nat_test.go index a8d1134e4d..aad5175fd5 100644 --- a/integration/container/nat_test.go +++ b/integration/container/nat_test.go @@ -30,7 +30,7 @@ func TestNetworkNat(t *testing.T) { startServerContainer(t, msg, 8080) endpoint := getExternalAddress(t) - conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", endpoint.String(), 8080)) + conn, err := net.Dial("tcp", net.JoinHostPort(endpoint.String(), "8080")) assert.NilError(t, err) defer conn.Close() @@ -115,6 +115,9 @@ func startServerContainer(t *testing.T, msg string, port int) string { return cID } +// getExternalAddress() returns the external IP-address from eth0. If eth0 has +// multiple IP-addresses, it returns the first IPv4 IP-address; if no IPv4 +// address is present, it returns the first IP-address found. func getExternalAddress(t *testing.T) net.IP { t.Helper() iface, err := net.InterfaceByName("eth0") @@ -124,6 +127,17 @@ func getExternalAddress(t *testing.T) net.IP { assert.NilError(t, err) assert.Check(t, 0 != len(ifaceAddrs)) + if len(ifaceAddrs) > 1 { + // Prefer IPv4 address if multiple addresses found, as rootlesskit + // does not handle IPv6 currently https://github.com/moby/moby/pull/41908#issuecomment-774200001 + for _, a := range ifaceAddrs { + ifaceIP, _, err := net.ParseCIDR(a.String()) + assert.NilError(t, err) + if ifaceIP.To4() != nil { + return ifaceIP + } + } + } ifaceIP, _, err := net.ParseCIDR(ifaceAddrs[0].String()) assert.NilError(t, err) From ef2351b416146ead18e4026a6fd2e43da70b87c7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 11 Feb 2021 21:45:16 +0100 Subject: [PATCH 3/3] integration-cli: rely less on "docker port" output format Also re-formatting some lines for readability. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit c6038b488406ca3b7cc62ddcc0438cf678694bf7) Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_api_exec_test.go | 2 +- integration-cli/docker_cli_port_test.go | 115 +++++++++++++----------- integration-cli/docker_cli_ps_test.go | 16 ++-- integration-cli/docker_cli_run_test.go | 16 ++-- 4 files changed, 77 insertions(+), 72 deletions(-) diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go index e0e0973ea0..02b33ac0fb 100644 --- a/integration-cli/docker_api_exec_test.go +++ b/integration-cli/docker_api_exec_test.go @@ -297,7 +297,7 @@ func waitForExec(c *testing.T, id string) { } func inspectContainer(c *testing.T, id string, out interface{}) { - resp, body, err := request.Get(fmt.Sprintf("/containers/%s/json", id)) + resp, body, err := request.Get("/containers/" + id + "/json") assert.NilError(c, err) defer body.Close() assert.Equal(c, resp.StatusCode, http.StatusOK) diff --git a/integration-cli/docker_cli_port_test.go b/integration-cli/docker_cli_port_test.go index f1bd7904cc..fbef608288 100644 --- a/integration-cli/docker_cli_port_test.go +++ b/integration-cli/docker_cli_port_test.go @@ -1,8 +1,8 @@ package main import ( + "context" "fmt" - "net" "regexp" "sort" "strconv" @@ -51,7 +51,8 @@ func (s *DockerSuite) TestPortList(c *testing.T) { err = assertPortList(c, out, []string{ "80/tcp -> 0.0.0.0:9876", "81/tcp -> 0.0.0.0:9877", - "82/tcp -> 0.0.0.0:9878"}) + "82/tcp -> 0.0.0.0:9878", + }) // Port list is not correct assert.NilError(c, err) @@ -78,7 +79,8 @@ func (s *DockerSuite) TestPortList(c *testing.T) { "80/tcp -> 0.0.0.0:9876", "80/tcp -> 0.0.0.0:9999", "81/tcp -> 0.0.0.0:9877", - "82/tcp -> 0.0.0.0:9878"}) + "82/tcp -> 0.0.0.0:9878", + }) // Port list is not correct assert.NilError(c, err) dockerCmd(c, "rm", "-f", ID) @@ -87,9 +89,7 @@ func (s *DockerSuite) TestPortList(c *testing.T) { // host port ranges used IDs := make([]string, 3) for i := 0; i < 3; i++ { - out, _ = dockerCmd(c, "run", "-d", - "-p", "9090-9092:80", - "busybox", "top") + out, _ = dockerCmd(c, "run", "-d", "-p", "9090-9092:80", "busybox", "top") IDs[i] = strings.TrimSpace(out) out, _ = dockerCmd(c, "port", IDs[i]) @@ -100,9 +100,7 @@ func (s *DockerSuite) TestPortList(c *testing.T) { } // test port range exhaustion - out, _, err = dockerCmdWithError("run", "-d", - "-p", "9090-9092:80", - "busybox", "top") + out, _, err = dockerCmdWithError("run", "-d", "-p", "9090-9092:80", "busybox", "top") // Exhausted port range did not return an error assert.Assert(c, err != nil, "out: %s", out) @@ -116,17 +114,13 @@ func (s *DockerSuite) TestPortList(c *testing.T) { // test invalid port ranges for _, invalidRange := range []string{"9090-9089:80", "9090-:80", "-9090:80"} { - out, _, err = dockerCmdWithError("run", "-d", - "-p", invalidRange, - "busybox", "top") + out, _, err = dockerCmdWithError("run", "-d", "-p", invalidRange, "busybox", "top") // Port range should have returned an error assert.Assert(c, err != nil, "out: %s", out) } // test host range:container range spec. - out, _ = dockerCmd(c, "run", "-d", - "-p", "9800-9803:80-83", - "busybox", "top") + out, _ = dockerCmd(c, "run", "-d", "-p", "9800-9803:80-83", "busybox", "top") ID = strings.TrimSpace(out) out, _ = dockerCmd(c, "port", ID) @@ -135,28 +129,27 @@ func (s *DockerSuite) TestPortList(c *testing.T) { "80/tcp -> 0.0.0.0:9800", "81/tcp -> 0.0.0.0:9801", "82/tcp -> 0.0.0.0:9802", - "83/tcp -> 0.0.0.0:9803"}) + "83/tcp -> 0.0.0.0:9803", + }) // Port list is not correct assert.NilError(c, err) dockerCmd(c, "rm", "-f", ID) // test mixing protocols in same port range - out, _ = dockerCmd(c, "run", "-d", - "-p", "8000-8080:80", - "-p", "8000-8080:80/udp", - "busybox", "top") + out, _ = dockerCmd(c, "run", "-d", "-p", "8000-8080:80", "-p", "8000-8080:80/udp", "busybox", "top") ID = strings.TrimSpace(out) out, _ = dockerCmd(c, "port", ID) // Running this test multiple times causes the TCP port to increment. - err = assertPortRange(c, out, []int{8000, 8080}, []int{8000, 8080}) + err = assertPortRange(ID, []int{8000, 8080}, []int{8000, 8080}) // Port list is not correct assert.NilError(c, err) dockerCmd(c, "rm", "-f", ID) } func assertPortList(c *testing.T, out string, expected []string) error { + c.Helper() lines := strings.Split(strings.Trim(out, "\n "), "\n") if len(lines) != len(expected) { return fmt.Errorf("different size lists %s, %d, %d", out, len(lines), len(expected)) @@ -164,8 +157,20 @@ func assertPortList(c *testing.T, out string, expected []string) error { sort.Strings(lines) sort.Strings(expected) + // "docker port" does not yet have a "--format" flag, and older versions + // of the CLI used an incorrect output format for mappings on IPv6 addresses + // for example, "80/tcp -> :::80" instead of "80/tcp -> [::]:80". + oldFormat := func(mapping string) string { + old := strings.Replace(mapping, "-> [", "-> ", 1) + old = strings.Replace(old, "]:", ":", 1) + return old + } + for i := 0; i < len(expected); i++ { - if lines[i] != expected[i] { + if lines[i] == expected[i] { + continue + } + if lines[i] != oldFormat(expected[i]) { return fmt.Errorf("|" + lines[i] + "!=" + expected[i] + "|") } } @@ -173,27 +178,40 @@ func assertPortList(c *testing.T, out string, expected []string) error { return nil } -func assertPortRange(c *testing.T, out string, expectedTCP, expectedUDP []int) error { - lines := strings.Split(strings.Trim(out, "\n "), "\n") +func assertPortRange(id string, expectedTCP, expectedUDP []int) error { + client := testEnv.APIClient() + inspect, err := client.ContainerInspect(context.TODO(), id) + if err != nil { + return err + } var validTCP, validUDP bool - for _, l := range lines { - // 80/tcp -> 0.0.0.0:8015 - port, err := strconv.Atoi(strings.Split(l, ":")[1]) - if err != nil { - return err + for portAndProto, binding := range inspect.NetworkSettings.Ports { + if portAndProto.Proto() == "tcp" && len(expectedTCP) == 0 { + continue } - if strings.Contains(l, "tcp") && expectedTCP != nil { - if port < expectedTCP[0] || port > expectedTCP[1] { - return fmt.Errorf("tcp port (%d) not in range expected range %d-%d", port, expectedTCP[0], expectedTCP[1]) - } - validTCP = true + if portAndProto.Proto() == "udp" && len(expectedTCP) == 0 { + continue } - if strings.Contains(l, "udp") && expectedUDP != nil { - if port < expectedUDP[0] || port > expectedUDP[1] { - return fmt.Errorf("udp port (%d) not in range expected range %d-%d", port, expectedUDP[0], expectedUDP[1]) + + for _, b := range binding { + port, err := strconv.Atoi(b.HostPort) + if err != nil { + return err + } + + if len(expectedTCP) > 0 { + if port < expectedTCP[0] || port > expectedTCP[1] { + return fmt.Errorf("tcp port (%d) not in range expected range %d-%d", port, expectedTCP[0], expectedTCP[1]) + } + validTCP = true + } + if len(expectedUDP) > 0 { + if port < expectedUDP[0] || port > expectedUDP[1] { + return fmt.Errorf("udp port (%d) not in range expected range %d-%d", port, expectedUDP[0], expectedUDP[1]) + } + validUDP = true } - validUDP = true } } if !validTCP { @@ -282,8 +300,7 @@ func (s *DockerSuite) TestUnpublishedPortsInPsOutput(c *testing.T) { func (s *DockerSuite) TestPortHostBinding(c *testing.T) { testRequires(c, DaemonIsLinux, NotUserNamespace) - out, _ := dockerCmd(c, "run", "-d", "-p", "9876:80", "busybox", - "nc", "-l", "-p", "80") + out, _ := dockerCmd(c, "run", "-d", "-p", "9876:80", "busybox", "nc", "-l", "-p", "80") firstID := strings.TrimSpace(out) out, _ = dockerCmd(c, "port", firstID, "80") @@ -292,8 +309,7 @@ func (s *DockerSuite) TestPortHostBinding(c *testing.T) { // Port list is not correct assert.NilError(c, err) - dockerCmd(c, "run", "--net=host", "busybox", - "nc", "localhost", "9876") + dockerCmd(c, "run", "--net=host", "busybox", "nc", "localhost", "9876") dockerCmd(c, "rm", "-f", firstID) @@ -304,22 +320,17 @@ func (s *DockerSuite) TestPortHostBinding(c *testing.T) { func (s *DockerSuite) TestPortExposeHostBinding(c *testing.T) { testRequires(c, DaemonIsLinux, NotUserNamespace) - out, _ := dockerCmd(c, "run", "-d", "-P", "--expose", "80", "busybox", - "nc", "-l", "-p", "80") + out, _ := dockerCmd(c, "run", "-d", "-P", "--expose", "80", "busybox", "nc", "-l", "-p", "80") firstID := strings.TrimSpace(out) - out, _ = dockerCmd(c, "port", firstID, "80") + out, _ = dockerCmd(c, "inspect", "--format", `{{index .NetworkSettings.Ports "80/tcp" 0 "HostPort" }}`, firstID) - _, exposedPort, err := net.SplitHostPort(out) - assert.Assert(c, err == nil, "out: %s", out) - - dockerCmd(c, "run", "--net=host", "busybox", - "nc", "localhost", strings.TrimSpace(exposedPort)) + exposedPort := strings.TrimSpace(out) + dockerCmd(c, "run", "--net=host", "busybox", "nc", "127.0.0.1", exposedPort) dockerCmd(c, "rm", "-f", firstID) - out, _, err = dockerCmdWithError("run", "--net=host", "busybox", - "nc", "localhost", strings.TrimSpace(exposedPort)) + out, _, err := dockerCmdWithError("run", "--net=host", "busybox", "nc", "127.0.0.1", exposedPort) // Port is still bound after the Container is removed assert.Assert(c, err != nil, "out: %s", out) } diff --git a/integration-cli/docker_cli_ps_test.go b/integration-cli/docker_cli_ps_test.go index ee0a67867a..1677fc00ea 100644 --- a/integration-cli/docker_cli_ps_test.go +++ b/integration-cli/docker_cli_ps_test.go @@ -594,20 +594,16 @@ func (s *DockerSuite) TestPsImageIDAfterUpdate(c *testing.T) { func (s *DockerSuite) TestPsNotShowPortsOfStoppedContainer(c *testing.T) { testRequires(c, DaemonIsLinux) - dockerCmd(c, "run", "--name=foo", "-d", "-p", "5000:5000", "busybox", "top") + dockerCmd(c, "run", "--name=foo", "-d", "-p", "6000:5000", "busybox", "top") assert.Assert(c, waitRun("foo") == nil) - out, _ := dockerCmd(c, "ps") - lines := strings.Split(strings.TrimSpace(out), "\n") - expected := "0.0.0.0:5000->5000/tcp" - fields := strings.Fields(lines[1]) - assert.Equal(c, fields[len(fields)-2], expected, fmt.Sprintf("Expected: %v, got: %v", expected, fields[len(fields)-2])) + ports, _ := dockerCmd(c, "ps", "--format", "{{ .Ports }}", "--filter", "name=foo") + expected := ":6000->5000/tcp" + assert.Assert(c, is.Contains(ports, expected), "Expected: %v, got: %v", expected, ports) dockerCmd(c, "kill", "foo") dockerCmd(c, "wait", "foo") - out, _ = dockerCmd(c, "ps", "-l") - lines = strings.Split(strings.TrimSpace(out), "\n") - fields = strings.Fields(lines[1]) - assert.Assert(c, fields[len(fields)-2] != expected, "Should not got %v", expected) + ports, _ = dockerCmd(c, "ps", "--format", "{{ .Ports }}", "--filter", "name=foo") + assert.Equal(c, ports, "", "Should not got %v", expected) } func (s *DockerSuite) TestPsShowMounts(c *testing.T) { diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 2fdc887188..1856643ea8 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2063,12 +2063,11 @@ func (s *DockerSuite) TestRunAllocatePortInReservedRange(c *testing.T) { out, _ := dockerCmd(c, "run", "-d", "-P", "-p", "80", "busybox", "top") id := strings.TrimSpace(out) - out, _ = dockerCmd(c, "port", id, "80") - - strPort := strings.Split(strings.TrimSpace(out), ":")[1] - port, err := strconv.ParseInt(strPort, 10, 64) + out, _ = dockerCmd(c, "inspect", "--format", `{{index .NetworkSettings.Ports "80/tcp" 0 "HostPort" }}`, id) + out = strings.TrimSpace(out) + port, err := strconv.ParseInt(out, 10, 64) if err != nil { - c.Fatalf("invalid port, got: %s, error: %s", strPort, err) + c.Fatalf("invalid port, got: %s, error: %s", out, err) } // allocate a static port and a dynamic port together, with static port @@ -2278,7 +2277,7 @@ func (s *DockerSuite) TestRunAllowPortRangeThroughExpose(c *testing.T) { if portnum < 3000 || portnum > 3003 { c.Fatalf("Port %d is out of range ", portnum) } - if binding == nil || len(binding) != 1 || len(binding[0].HostPort) == 0 { + if len(binding) == 0 || len(binding[0].HostPort) == 0 { c.Fatalf("Port is not mapped for the port %s", port) } } @@ -2497,13 +2496,12 @@ func (s *DockerSuite) TestRunPortFromDockerRangeInUse(c *testing.T) { out, _ := dockerCmd(c, "run", "-d", "-p", ":80", "busybox", "top") id := strings.TrimSpace(out) - out, _ = dockerCmd(c, "port", id) + out, _ = dockerCmd(c, "inspect", "--format", `{{index .NetworkSettings.Ports "80/tcp" 0 "HostPort" }}`, id) out = strings.TrimSpace(out) if out == "" { c.Fatal("docker port command output is empty") } - out = strings.Split(out, ":")[1] lastPort, err := strconv.Atoi(out) if err != nil { c.Fatal(err) @@ -2637,7 +2635,7 @@ func (s *DockerSuite) TestRunAllowPortRangeThroughPublish(c *testing.T) { if portnum < 3000 || portnum > 3003 { c.Fatalf("Port %d is out of range ", portnum) } - if binding == nil || len(binding) != 1 || len(binding[0].HostPort) == 0 { + if len(binding) == 0 || len(binding[0].HostPort) == 0 { c.Fatal("Port is not mapped for the port "+port, out) } }