From f115c32f6b2e23b8efacb8140f65824ab50c8752 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Mon, 30 Mar 2015 18:06:16 -0700 Subject: [PATCH] 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 (cherry picked from commit d8c628cf082a50c0a2a5e381a21da8279a5462b4) Docker-DCO-1.1-Signed-off-by: Jessie Frazelle (github: jfrazelle) Docker-DCO-1.1-Signed-off-by: Jessie Frazelle (github: jfrazelle) --- api/server/server.go | 4 ++-- daemon/daemon.go | 7 ------- daemon/networkdriver/bridge/driver.go | 12 +++++++++--- daemon/networkdriver/portmapper/mapper.go | 16 ++++++++-------- daemon/networkdriver/portmapper/mapper_test.go | 2 +- 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/api/server/server.go b/api/server/server.go index a1c57a8e4..453b4c69a 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -27,7 +27,7 @@ import ( log "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" @@ -1527,7 +1527,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) } } diff --git a/daemon/daemon.go b/daemon/daemon.go index 056566632..f51a7b165 100644 --- a/daemon/daemon.go +++ b/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" @@ -827,12 +826,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 { - log.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 != "" { diff --git a/daemon/networkdriver/bridge/driver.go b/daemon/networkdriver/bridge/driver.go index aa139b9a3..c2582701a 100644 --- a/daemon/networkdriver/bridge/driver.go +++ b/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)} @@ -98,6 +99,7 @@ func InitDriver(job *engine.Job) engine.Status { fixedCIDR = job.Getenv("FixedCIDR") fixedCIDRv6 = job.Getenv("FixedCIDRv6") ) + portMapper = portmapper.New() if defaultIP := job.Getenv("DefaultBindingIP"); defaultIP != "" { defaultBindingIP = net.ParseIP(defaultIP) @@ -234,7 +236,7 @@ func InitDriver(job *engine.Job) engine.Status { if err != nil { return job.Error(err) } - portmapper.SetIptablesChain(chain) + portMapper.SetIptablesChain(chain) } bridgeIPv4Network = networkv4 @@ -349,6 +351,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 @@ -586,7 +592,7 @@ func Release(job *engine.Job) engine.Status { } for _, nat := range containerInterface.PortMappings { - if err := portmapper.Unmap(nat); err != nil { + if err := portMapper.Unmap(nat); err != nil { log.Infof("Unable to unmap port %s: %s", nat, err) } } @@ -643,7 +649,7 @@ func AllocatePort(job *engine.Job) engine.Status { 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 diff --git a/daemon/networkdriver/portmapper/mapper.go b/daemon/networkdriver/portmapper/mapper.go index 98f5d5dae..4a98d4a79 100644 --- a/daemon/networkdriver/portmapper/mapper.go +++ b/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 } diff --git a/daemon/networkdriver/portmapper/mapper_test.go b/daemon/networkdriver/portmapper/mapper_test.go index d5d10d8cb..729fe5607 100644 --- a/daemon/networkdriver/portmapper/mapper_test.go +++ b/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)