libnet/d/bridge: use fresh PortAllocator in tests

portallocator.PortAllocator holds persistent state on port allocations
in the network namespace. It normal operation it is used as a singleton,
which is a problem in unit tests as every test runs in a clean network
namespace. The singleton "default" PortAllocator remembers all the port
allocations made in other tests---in other network namespaces---and
can result in spurious test failures. Refactor the bridge driver so that
tests can construct driver instances with unique PortAllocators.

Signed-off-by: Cory Snider <csnider@mirantis.com>

libnetwork/drivers/bridge/bridge.go

@@ -22,6 +22,7 @@ import (
 	"github.com/docker/docker/libnetwork/netutils"
 	"github.com/docker/docker/libnetwork/ns"
 	"github.com/docker/docker/libnetwork/options"
+	"github.com/docker/docker/libnetwork/portallocator"
 	"github.com/docker/docker/libnetwork/portmapper"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/sirupsen/logrus"
@@ -155,12 +156,16 @@ type driver struct {
 	store             datastore.DataStore
 	nlh               *netlink.Handle
 	configNetwork     sync.Mutex
+	portAllocator     *portallocator.PortAllocator // Overridable for tests.
 	sync.Mutex
 }
 
 // New constructs a new bridge driver
 func newDriver() *driver {
-	return &driver{networks: map[string]*bridgeNetwork{}}
+	return &driver{
+		networks:      map[string]*bridgeNetwork{},
+		portAllocator: portallocator.Get(),
+	}
 }
 
 // Init registers a new instance of bridge driver
@@ -685,8 +690,8 @@ func (d *driver) createNetwork(config *networkConfiguration) (err error) {
 		id:           config.ID,
 		endpoints:    make(map[string]*bridgeEndpoint),
 		config:       config,
-		portMapper:   portmapper.New(d.config.UserlandProxyPath),
-		portMapperV6: portmapper.New(d.config.UserlandProxyPath),
+		portMapper:   portmapper.NewWithPortAllocator(d.portAllocator, d.config.UserlandProxyPath),
+		portMapperV6: portmapper.NewWithPortAllocator(d.portAllocator, d.config.UserlandProxyPath),
 		bridge:       bridgeIface,
 		driver:       d,
 	}

libnetwork/drivers/bridge/bridge_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/netutils"
 	"github.com/docker/docker/libnetwork/options"
+	"github.com/docker/docker/libnetwork/portallocator"
 	"github.com/docker/docker/libnetwork/testutils"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/vishvananda/netlink"
@@ -607,6 +608,7 @@ func TestQueryEndpointInfoHairpin(t *testing.T) {
 func testQueryEndpointInfo(t *testing.T, ulPxyEnabled bool) {
 	defer testutils.SetupTestOSContext(t)()
 	d := newDriver()
+	d.portAllocator = portallocator.NewInstance()
 
 	config := &configuration{
 		EnableIPTables:      true,
@@ -1079,6 +1081,7 @@ func TestCreateParallel(t *testing.T) {
 	defer c.Cleanup(t)
 
 	d := newDriver()
+	d.portAllocator = portallocator.NewInstance()
 
 	if err := d.configure(nil); err != nil {
 		t.Fatalf("Failed to setup driver config: %v", err)

libnetwork/portallocator/portallocator.go

@@ -83,12 +83,13 @@ func Get() *PortAllocator {
 	// 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(func() {
-		instance = newInstance()
+		instance = NewInstance()
 	})
 	return instance
 }
 
-func newInstance() *PortAllocator {
+// NewInstance is meant for use by libnetwork tests. It is not meant to be called directly.
+func NewInstance() *PortAllocator {
 	start, end, err := getDynamicPortRange()
 	if err != nil {
 		logrus.WithError(err).Infof("falling back to default port range %d-%d", defaultPortRangeStart, defaultPortRangeEnd)

libnetwork/portallocator/portallocator_test.go

@@ -6,7 +6,7 @@ import (
 )
 
 func resetPortAllocator() {
-	instance = newInstance()
+	instance = NewInstance()
 }
 
 func TestRequestNewPort(t *testing.T) {