Ensure that bridge driver does not use global mappers

This has a few hacks in it but it ensures that the bridge driver does
not use global state in the mappers, atleast as much as possible at this
point without further refactoring.  Some of the exported fields are
hacks to handle the daemon port mapping but this results in a much
cleaner approach and completely remove the global state from the mapper
and allocator.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

api/server/server.go

@@ -27,7 +27,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/daemon/networkdriver/portallocator"
+	"github.com/docker/docker/daemon/networkdriver/bridge"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/pkg/listenbuffer"
 	"github.com/docker/docker/pkg/parsers"
@@ -1542,7 +1542,7 @@ func allocateDaemonPort(addr string) error {
 	}
 
 	for _, hostIP := range hostIPs {
-		if _, err := portallocator.RequestPort(hostIP, "tcp", intPort); err != nil {
+		if _, err := bridge.RequestPort(hostIP, "tcp", intPort); err != nil {
 			return fmt.Errorf("failed to allocate daemon listening port %d (err: %v)", intPort, err)
 		}
 	}

daemon/daemon.go

@@ -25,7 +25,6 @@ import (
 	"github.com/docker/docker/daemon/graphdriver"
 	_ "github.com/docker/docker/daemon/graphdriver/vfs"
 	_ "github.com/docker/docker/daemon/networkdriver/bridge"
-	"github.com/docker/docker/daemon/networkdriver/portallocator"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/graph"
 	"github.com/docker/docker/image"
@@ -818,12 +817,6 @@ func NewDaemonFromDirectory(config *Config, eng *engine.Engine) (*Daemon, error)
 	}
 	config.DisableNetwork = config.BridgeIface == disableNetworkBridge
 
-	// register portallocator release on shutdown
-	eng.OnShutdown(func() {
-		if err := portallocator.ReleaseAll(); err != nil {
-			logrus.Errorf("portallocator.ReleaseAll(): %s", err)
-		}
-	})
 	// Claim the pidfile first, to avoid any and all unexpected race conditions.
 	// Some of the init doesn't need a pidfile lock - but let's not try to be smart.
 	if config.Pidfile != "" {

daemon/networkdriver/bridge/driver.go

@@ -77,6 +77,7 @@ var (
 	bridgeIPv4Network *net.IPNet
 	bridgeIPv6Addr    net.IP
 	globalIPv6Network *net.IPNet
+	portMapper        *portmapper.PortMapper
 
 	defaultBindingIP  = net.ParseIP("0.0.0.0")
 	currentInterfaces = ifaces{c: make(map[string]*networkInterface)}
@@ -99,6 +100,7 @@ func InitDriver(job *engine.Job) error {
 		fixedCIDR      = job.Getenv("FixedCIDR")
 		fixedCIDRv6    = job.Getenv("FixedCIDRv6")
 	)
+	portMapper = portmapper.New()
 
 	if defaultIP := job.Getenv("DefaultBindingIP"); defaultIP != "" {
 		defaultBindingIP = net.ParseIP(defaultIP)
@@ -235,7 +237,7 @@ func InitDriver(job *engine.Job) error {
 		if err != nil {
 			return err
 		}
-		portmapper.SetIptablesChain(chain)
+		portMapper.SetIptablesChain(chain)
 	}
 
 	bridgeIPv4Network = networkv4
@@ -350,6 +352,10 @@ func setupIPTables(addr net.Addr, icc, ipmasq bool) error {
 	return nil
 }
 
+func RequestPort(ip net.IP, proto string, port int) (int, error) {
+	return portMapper.Allocator.RequestPort(ip, proto, port)
+}
+
 // configureBridge attempts to create and configure a network bridge interface named `bridgeIface` on the host
 // If bridgeIP is empty, it will try to find a non-conflicting IP from the Docker-specified private ranges
 // If the bridge `bridgeIface` already exists, it will only perform the IP address association with the existing
@@ -587,7 +593,7 @@ func Release(job *engine.Job) error {
 	}
 
 	for _, nat := range containerInterface.PortMappings {
-		if err := portmapper.Unmap(nat); err != nil {
+		if err := portMapper.Unmap(nat); err != nil {
 			logrus.Infof("Unable to unmap port %s: %s", nat, err)
 		}
 	}
@@ -644,7 +650,7 @@ func AllocatePort(job *engine.Job) error {
 
 	var host net.Addr
 	for i := 0; i < MaxAllocatedPortAttempts; i++ {
-		if host, err = portmapper.Map(container, ip, hostPort); err == nil {
+		if host, err = portMapper.Map(container, ip, hostPort); err == nil {
 			break
 		}
 		// There is no point in immediately retrying to map an explicitly

daemon/networkdriver/portmapper/mapper.go

@@ -33,7 +33,7 @@ type PortMapper struct {
 	currentMappings map[string]*mapping
 	lock            sync.Mutex
 
-	allocator *portallocator.PortAllocator
+	Allocator *portallocator.PortAllocator
 }
 
 func New() *PortMapper {
@@ -43,7 +43,7 @@ func New() *PortMapper {
 func NewWithPortAllocator(allocator *portallocator.PortAllocator) *PortMapper {
 	return &PortMapper{
 		currentMappings: make(map[string]*mapping),
-		allocator:       allocator,
+		Allocator:       allocator,
 	}
 }
 
@@ -65,7 +65,7 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int) (host
 	switch container.(type) {
 	case *net.TCPAddr:
 		proto = "tcp"
-		if allocatedHostPort, err = pm.allocator.RequestPort(hostIP, proto, hostPort); err != nil {
+		if allocatedHostPort, err = pm.Allocator.RequestPort(hostIP, proto, hostPort); err != nil {
 			return nil, err
 		}
 
@@ -78,7 +78,7 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int) (host
 		proxy = NewProxy(proto, hostIP, allocatedHostPort, container.(*net.TCPAddr).IP, container.(*net.TCPAddr).Port)
 	case *net.UDPAddr:
 		proto = "udp"
-		if allocatedHostPort, err = pm.allocator.RequestPort(hostIP, proto, hostPort); err != nil {
+		if allocatedHostPort, err = pm.Allocator.RequestPort(hostIP, proto, hostPort); err != nil {
 			return nil, err
 		}
 
@@ -96,7 +96,7 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int) (host
 	// release the allocated port on any further error during return.
 	defer func() {
 		if err != nil {
-			pm.allocator.ReleasePort(hostIP, proto, allocatedHostPort)
+			pm.Allocator.ReleasePort(hostIP, proto, allocatedHostPort)
 		}
 	}()
 
@@ -114,7 +114,7 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int) (host
 		// need to undo the iptables rules before we return
 		proxy.Stop()
 		pm.forward(iptables.Delete, m.proto, hostIP, allocatedHostPort, containerIP.String(), containerPort)
-		if err := pm.allocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
+		if err := pm.Allocator.ReleasePort(hostIP, m.proto, allocatedHostPort); err != nil {
 			return err
 		}
 
@@ -154,9 +154,9 @@ func (pm *PortMapper) Unmap(host net.Addr) error {
 
 	switch a := host.(type) {
 	case *net.TCPAddr:
-		return pm.allocator.ReleasePort(a.IP, "tcp", a.Port)
+		return pm.Allocator.ReleasePort(a.IP, "tcp", a.Port)
 	case *net.UDPAddr:
-		return pm.allocator.ReleasePort(a.IP, "udp", a.Port)
+		return pm.Allocator.ReleasePort(a.IP, "udp", a.Port)
 	}
 	return nil
 }

daemon/networkdriver/portmapper/mapper_test.go

@@ -125,7 +125,7 @@ func TestMapAllPortsSingleInterface(t *testing.T) {
 	}()
 
 	for i := 0; i < 10; i++ {
-		start, end := pm.allocator.Begin, pm.allocator.End
+		start, end := pm.Allocator.Begin, pm.Allocator.End
 		for i := start; i < end; i++ {
 			if host, err = pm.Map(srcAddr1, dstIp1, 0); err != nil {
 				t.Fatal(err)