From 3e6a889cd602386f3b4878528c0e9cf6fe674de2 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Thu, 16 Apr 2015 15:16:11 -0700 Subject: [PATCH] Port Allocator as a libnetwork package DESCRIPTION: As part of bringing libnetwork bridge driver features in parity with docker/daemon/network/driver/bridge features (Issue #46), this commit addresses the bridge.RequestPort() API. Currenlty docker/api/server.go needs an hold of port allocator in order to reserve a transport port which will be used by the http server on the host machine, so that portallocator does not give out that port when queried by portmapper as part of network driver operations. ISSUE: Current implementation in docker is server.go directly access portmapper and then portallocator from bridge pkg calling bridge.RequestPort(). This also forces that function to trigger portmapper initialization (in case bridge init() was not executed yet), while portmapper life cycle should only be controlled by bridge network driver. We cannot mantain this behavior with libnetwrok as this violates the modularization of networking code which libnetwork is bringing in. FIX: Make portallocator a singleton, now both docker core and portmapper code can initialize it and get the only one instance (Change in docker core code will happen when docker code will migrate to use libnetwork), given it is being used for host specific needs. NOTE: Long term fix is having multiple portallocator instances (so no more singleton) each capable to be in sync with OS regarding current port allocation. When this change comes, no change whould be required on portallocator' clients side, changes will be confined to portallocator package. Signed-off-by: Alessandro Boch --- libnetwork/drivers/bridge/bridge.go | 9 +-------- libnetwork/{ => pkg}/portallocator/portallocator.go | 13 +++++++++++++ .../{ => pkg}/portallocator/portallocator_test.go | 13 +++++++++++++ libnetwork/portmapper/mapper.go | 2 +- 4 files changed, 28 insertions(+), 9 deletions(-) rename libnetwork/{ => pkg}/portallocator/portallocator.go (90%) rename libnetwork/{ => pkg}/portallocator/portallocator_test.go (95%) diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 7fcf179b05..81230a9ddc 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -23,17 +23,10 @@ const ( ) var ( - once sync.Once ipAllocator *ipallocator.IPAllocator portMapper *portmapper.PortMapper ) -func initPortMapper() { - once.Do(func() { - portMapper = portmapper.New() - }) -} - // Configuration info for the "simplebridge" driver. type Configuration struct { BridgeName string @@ -70,7 +63,7 @@ type driver struct { func init() { ipAllocator = ipallocator.New() - initPortMapper() + portMapper = portmapper.New() } // New provides a new instance of bridge driver instance diff --git a/libnetwork/portallocator/portallocator.go b/libnetwork/pkg/portallocator/portallocator.go similarity index 90% rename from libnetwork/portallocator/portallocator.go rename to libnetwork/pkg/portallocator/portallocator.go index 51b60aa68d..df605d7fb8 100644 --- a/libnetwork/portallocator/portallocator.go +++ b/libnetwork/pkg/portallocator/portallocator.go @@ -26,6 +26,9 @@ var ( // ErrUnknownProtocol is returned when an unknown protocol was specified ErrUnknownProtocol = errors.New("unknown protocol") defaultIP = net.ParseIP("0.0.0.0") + once sync.Once + instance *PortAllocator + createInstance = func() { instance = newInstance() } ) // ErrPortAlreadyAllocated is the returned error information when a requested port is already being used @@ -79,6 +82,16 @@ type ( // New returns a new instance of PortAllocator func New() *PortAllocator { + // Port Allocator is a singleton + // Note: Long term solution will be each PortAllocator will have access to + // the OS so that it can have up to date view of the OS port allocation. + // When this happens singleton behavior will be removed. Clients do not + // need to worry about this, they will not see a change in behavior. + once.Do(createInstance) + return instance +} + +func newInstance() *PortAllocator { start, end, err := getDynamicPortRange() if err != nil { logrus.Warn(err) diff --git a/libnetwork/portallocator/portallocator_test.go b/libnetwork/pkg/portallocator/portallocator_test.go similarity index 95% rename from libnetwork/portallocator/portallocator_test.go rename to libnetwork/pkg/portallocator/portallocator_test.go index 17201235e0..76ba37601d 100644 --- a/libnetwork/portallocator/portallocator_test.go +++ b/libnetwork/pkg/portallocator/portallocator_test.go @@ -5,8 +5,13 @@ import ( "testing" ) +func resetPortAllocator() { + instance = newInstance() +} + func TestRequestNewPort(t *testing.T) { p := New() + defer resetPortAllocator() port, err := p.RequestPort(defaultIP, "tcp", 0) if err != nil { @@ -20,11 +25,13 @@ func TestRequestNewPort(t *testing.T) { func TestRequestSpecificPort(t *testing.T) { p := New() + defer resetPortAllocator() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { t.Fatal(err) } + if port != 5000 { t.Fatalf("Expected port 5000 got %d", port) } @@ -48,6 +55,7 @@ func TestReleasePort(t *testing.T) { func TestReuseReleasedPort(t *testing.T) { p := New() + defer resetPortAllocator() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -69,6 +77,7 @@ func TestReuseReleasedPort(t *testing.T) { func TestReleaseUnreadledPort(t *testing.T) { p := New() + defer resetPortAllocator() port, err := p.RequestPort(defaultIP, "tcp", 5000) if err != nil { @@ -95,6 +104,7 @@ func TestUnknowProtocol(t *testing.T) { func TestAllocateAllPorts(t *testing.T) { p := New() + defer resetPortAllocator() for i := 0; i <= p.End-p.Begin; i++ { port, err := p.RequestPort(defaultIP, "tcp", 0) @@ -145,6 +155,7 @@ func TestAllocateAllPorts(t *testing.T) { func BenchmarkAllocatePorts(b *testing.B) { p := New() + defer resetPortAllocator() for i := 0; i < b.N; i++ { for i := 0; i <= p.End-p.Begin; i++ { @@ -163,6 +174,7 @@ func BenchmarkAllocatePorts(b *testing.B) { func TestPortAllocation(t *testing.T) { p := New() + defer resetPortAllocator() ip := net.ParseIP("192.168.0.1") ip2 := net.ParseIP("192.168.0.2") @@ -224,6 +236,7 @@ func TestPortAllocation(t *testing.T) { func TestNoDuplicateBPR(t *testing.T) { p := New() + defer resetPortAllocator() if port, err := p.RequestPort(defaultIP, "tcp", p.Begin); err != nil { t.Fatal(err) diff --git a/libnetwork/portmapper/mapper.go b/libnetwork/portmapper/mapper.go index f0083e32ea..3661b5e462 100644 --- a/libnetwork/portmapper/mapper.go +++ b/libnetwork/portmapper/mapper.go @@ -8,7 +8,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/iptables" - "github.com/docker/libnetwork/portallocator" + "github.com/docker/libnetwork/pkg/portallocator" ) type mapping struct {