diff --git a/daemon/container.go b/daemon/container.go index 82446a8d1b..b78c24642f 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -647,7 +647,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 8849a2d1b1..c87b7cf3fe 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2213,6 +2213,39 @@ func (s *DockerSuite) TestRunPortInUse(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) + } +}