From 45221d6bfbb76ae500494f0cf2e3b6ea036ddff6 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 Upstream-commit: d8c628cf082a50c0a2a5e381a21da8279a5462b4 Component: engine --- components/engine/api/server/server.go | 4 ++-- components/engine/daemon/daemon.go | 7 ------- .../engine/daemon/networkdriver/bridge/driver.go | 12 +++++++++--- .../daemon/networkdriver/portmapper/mapper.go | 16 ++++++++-------- .../networkdriver/portmapper/mapper_test.go | 2 +- 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/components/engine/api/server/server.go b/components/engine/api/server/server.go index a7d0a58b2f..96abf5c3b5 100644 --- a/components/engine/api/server/server.go +++ b/components/engine/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) } } diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index ba9c73ce64..36cf438bbb 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/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 != "" { diff --git a/components/engine/daemon/networkdriver/bridge/driver.go b/components/engine/daemon/networkdriver/bridge/driver.go index c5f5704f94..21d8c44554 100644 --- a/components/engine/daemon/networkdriver/bridge/driver.go +++ b/components/engine/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 diff --git a/components/engine/daemon/networkdriver/portmapper/mapper.go b/components/engine/daemon/networkdriver/portmapper/mapper.go index 7092352f57..8f79bae3f2 100644 --- a/components/engine/daemon/networkdriver/portmapper/mapper.go +++ b/components/engine/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/components/engine/daemon/networkdriver/portmapper/mapper_test.go b/components/engine/daemon/networkdriver/portmapper/mapper_test.go index d5d10d8cb1..729fe56075 100644 --- a/components/engine/daemon/networkdriver/portmapper/mapper_test.go +++ b/components/engine/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)