From cd2b019214eb1978ae267786668dc7a8a3702679 Mon Sep 17 00:00:00 2001 From: "Daniel, Dao Quang Minh" Date: Wed, 8 Apr 2015 05:31:47 +0000 Subject: [PATCH] sort ports mapping before allocating prioritize the ports with static mapping before dynamic mapping. This removes the port conflicts when we allocate static port in the reserved range together with dynamic ones. When static port is allocated first, Docker will skip those when determining free ports for dynamic ones. Signed-off-by: Daniel, Dao Quang Minh --- daemon/container.go | 9 +++- integration-cli/docker_cli_run_test.go | 33 +++++++++++++ nat/sort.go | 66 +++++++++++++++++++++++++- nat/sort_test.go | 44 +++++++++++++++++ 4 files changed, 150 insertions(+), 2 deletions(-) diff --git a/daemon/container.go b/daemon/container.go index ef42295344..ffae6ff2cd 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -645,7 +645,14 @@ func (container *Container) AllocateNetwork() error { container.NetworkSettings.PortMapping = nil - for port := range portSpecs { + ports := make([]nat.Port, len(portSpecs)) + var i int + for p := range portSpecs { + ports[i] = p + i++ + } + nat.SortPortMap(ports, bindings) + for _, port := range ports { if err = container.allocatePort(port, bindings); err != nil { bridge.Release(container.ID) return err diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 0cf5c31eee..1ed84545ae 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2243,6 +2243,39 @@ func (s *DockerSuite) TestRunPortProxy(c *check.C) { } } +// https://github.com/docker/docker/issues/12148 +func (s *DockerSuite) TestRunAllocatePortInReservedRange(c *check.C) { + // allocate a dynamic port to get the most recent + cmd := exec.Command(dockerBinary, "run", "-d", "-P", "-p", "80", "busybox", "top") + out, _, err := runCommandWithOutput(cmd) + if err != nil { + c.Fatalf("Failed to run, output: %s, error: %s", out, err) + } + id := strings.TrimSpace(out) + + cmd = exec.Command(dockerBinary, "port", id, "80") + out, _, err = runCommandWithOutput(cmd) + if err != nil { + c.Fatalf("Failed to get port, output: %s, error: %s", out, err) + } + strPort := strings.Split(strings.TrimSpace(out), ":")[1] + port, err := strconv.ParseInt(strPort, 10, 64) + if err != nil { + c.Fatalf("invalid port, got: %s, error: %s", strPort, err) + } + + // allocate a static port and a dynamic port together, with static port + // takes the next recent port in dynamic port range. + cmd = exec.Command(dockerBinary, "run", "-d", "-P", + "-p", "80", + "-p", fmt.Sprintf("%d:8080", port+1), + "busybox", "top") + out, _, err = runCommandWithOutput(cmd) + if err != nil { + c.Fatalf("Failed to run, output: %s, error: %s", out, err) + } +} + // Regression test for #7792 func (s *DockerSuite) TestRunMountOrdering(c *check.C) { testRequires(c, SameHostDaemon) diff --git a/nat/sort.go b/nat/sort.go index f36c12f7bb..6441936ff9 100644 --- a/nat/sort.go +++ b/nat/sort.go @@ -1,6 +1,10 @@ package nat -import "sort" +import ( + "sort" + "strconv" + "strings" +) type portSorter struct { ports []Port @@ -26,3 +30,63 @@ func Sort(ports []Port, predicate func(i, j Port) bool) { s := &portSorter{ports, predicate} sort.Sort(s) } + +type portMapEntry struct { + port Port + binding PortBinding +} + +type portMapSorter []portMapEntry + +func (s portMapSorter) Len() int { return len(s) } +func (s portMapSorter) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + +// sort the port so that the order is: +// 1. port with larger specified bindings +// 2. larger port +// 3. port with tcp protocol +func (s portMapSorter) Less(i, j int) bool { + pi, pj := s[i].port, s[j].port + hpi, hpj := toInt(s[i].binding.HostPort), toInt(s[j].binding.HostPort) + return hpi > hpj || pi.Int() > pj.Int() || (pi.Int() == pj.Int() && strings.ToLower(pi.Proto()) == "tcp") +} + +// SortPortMap sorts the list of ports and their respected mapping. The ports +// will explicit HostPort will be placed first. +func SortPortMap(ports []Port, bindings PortMap) { + s := portMapSorter{} + for _, p := range ports { + if binding, ok := bindings[p]; ok { + for _, b := range binding { + s = append(s, portMapEntry{port: p, binding: b}) + } + } else { + s = append(s, portMapEntry{port: p}) + } + bindings[p] = []PortBinding{} + } + + sort.Sort(s) + var ( + i int + pm = make(map[Port]struct{}) + ) + // reorder ports + for _, entry := range s { + if _, ok := pm[entry.port]; !ok { + ports[i] = entry.port + pm[entry.port] = struct{}{} + i++ + } + // reorder bindings for this port + bindings[entry.port] = append(bindings[entry.port], entry.binding) + } +} + +func toInt(s string) int64 { + i, err := strconv.ParseInt(s, 10, 64) + if err != nil { + i = 0 + } + return i +} diff --git a/nat/sort_test.go b/nat/sort_test.go index 5d490e321b..ba24cdbcb9 100644 --- a/nat/sort_test.go +++ b/nat/sort_test.go @@ -2,6 +2,7 @@ package nat import ( "fmt" + "reflect" "testing" ) @@ -39,3 +40,46 @@ func TestSortSamePortWithDifferentProto(t *testing.T) { t.Fail() } } + +func TestSortPortMap(t *testing.T) { + ports := []Port{ + Port("22/tcp"), + Port("22/udp"), + Port("8000/tcp"), + Port("6379/tcp"), + Port("9999/tcp"), + } + + portMap := PortMap{ + Port("22/tcp"): []PortBinding{ + {}, + }, + Port("8000/tcp"): []PortBinding{ + {}, + }, + Port("6379/tcp"): []PortBinding{ + {}, + {HostIp: "0.0.0.0", HostPort: "32749"}, + }, + Port("9999/tcp"): []PortBinding{ + {HostIp: "0.0.0.0", HostPort: "40000"}, + }, + } + + SortPortMap(ports, portMap) + if !reflect.DeepEqual(ports, []Port{ + Port("9999/tcp"), + Port("6379/tcp"), + Port("8000/tcp"), + Port("22/tcp"), + Port("22/udp"), + }) { + t.Errorf("failed to prioritize port with explicit mappings, got %v", ports) + } + if pm := portMap[Port("6379/tcp")]; !reflect.DeepEqual(pm, []PortBinding{ + {HostIp: "0.0.0.0", HostPort: "32749"}, + {}, + }) { + t.Errorf("failed to prioritize bindings with explicit mappings, got %v", pm) + } +}