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 <dqminh89@gmail.com>

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

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)

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