Merge pull request #12175 from dqminh/fix-allocate-port

#12148: sort ports mapping before allocating

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

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)

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
+}

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)
+	}
+}