libnet/ipamutils: no more global state

Prior to this change, cnmallocator would call
`ConfigGlobalScopeDefaultNetworks` right before initializing its
IPAM drivers. This function was mutating some global state used
during drivers init.

This change just remove the global state, and adds an arg to
ipams.Register and defaultipam.Register to pass the global pools
by arguments instead.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

libnetwork/cnmallocator/drivers_ipam.go

@@ -32,14 +32,12 @@ func initIPAMDrivers(r ipamapi.Registerer, netConfig *networkallocator.Config) e
 		str.WriteString(strconv.Itoa(int(netConfig.SubnetSize)))
 
 	}
-	if err := ipamutils.ConfigGlobalScopeDefaultNetworks(addressPool); err != nil {
-		return err
-	}
-	if addressPool != nil {
+
+	if len(addressPool) > 0 {
 		log.G(context.TODO()).Infof("Swarm initialized global default address pool to: " + str.String())
 	}
 
-	if err := ipams.Register(r, nil, []*ipamutils.NetworkToSplit(nil)); err != nil {
+	if err := ipams.Register(r, nil, nil, addressPool); err != nil {
 		return err
 	}
 

libnetwork/controller.go

@@ -133,7 +133,7 @@ func New(cfgOptions ...config.Option) (*Controller, error) {
 		return nil, err
 	}
 
-	if err := ipams.Register(&c.ipamRegistry, c.cfg.PluginGetter, c.cfg.DefaultAddressPool); err != nil {
+	if err := ipams.Register(&c.ipamRegistry, c.cfg.PluginGetter, c.cfg.DefaultAddressPool, nil); err != nil {
 		return nil, err
 	}
 

libnetwork/drvregistry/ipams_test.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/ipams"
-	"github.com/docker/docker/libnetwork/ipamutils"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -15,7 +14,7 @@ import (
 func getNewIPAMs(t *testing.T) *IPAMs {
 	r := &IPAMs{}
 
-	assert.Assert(t, ipams.Register(r, nil, []*ipamutils.NetworkToSplit(nil)))
+	assert.Assert(t, ipams.Register(r, nil, nil, nil))
 
 	return r
 }

libnetwork/ipams/defaultipam/allocator.go

@@ -21,19 +21,28 @@ const (
 )
 
 // Register registers the default ipam driver with libnetwork. It takes
-// an optional addressPools containing the list of user-defined address pools
-// used by the local address space (ie. daemon's default-address-pools parameter).
-func Register(ic ipamapi.Registerer, addressPools []*ipamutils.NetworkToSplit) error {
+// two optional address pools respectively containing the list of user-defined
+// address pools for 'local' and 'global' address spaces.
+func Register(ic ipamapi.Registerer, lAddrPools, gAddrPools []*ipamutils.NetworkToSplit) error {
 	localAddressPools := ipamutils.GetLocalScopeDefaultNetworks()
-	if len(addressPools) > 0 {
+	if len(lAddrPools) > 0 {
 		var err error
-		localAddressPools, err = ipamutils.SplitNetworks(addressPools)
+		localAddressPools, err = ipamutils.SplitNetworks(lAddrPools)
 		if err != nil {
 			return err
 		}
 	}
 
-	a, err := NewAllocator(localAddressPools, ipamutils.GetGlobalScopeDefaultNetworks())
+	globalAddressPools := ipamutils.GetGlobalScopeDefaultNetworks()
+	if len(gAddrPools) > 0 {
+		var err error
+		globalAddressPools, err = ipamutils.SplitNetworks(gAddrPools)
+		if err != nil {
+			return err
+		}
+	}
+
+	a, err := NewAllocator(localAddressPools, globalAddressPools)
 	if err != nil {
 		return err
 	}

libnetwork/ipams/drivers.go

@@ -12,8 +12,8 @@ import (
 
 // Register registers all the builtin drivers (ie. default, windowsipam, null
 // and remote). If 'pg' is nil, the remote driver won't be registered.
-func Register(r ipamapi.Registerer, pg plugingetter.PluginGetter, addressPools []*ipamutils.NetworkToSplit) error {
-	if err := defaultipam.Register(r, addressPools); err != nil {
+func Register(r ipamapi.Registerer, pg plugingetter.PluginGetter, lAddrPools, gAddrPools []*ipamutils.NetworkToSplit) error {
+	if err := defaultipam.Register(r, lAddrPools, gAddrPools); err != nil {
 		return err
 	}
 	if err := windowsipam.Register(r); err != nil {

libnetwork/ipamutils/utils.go

@@ -4,7 +4,6 @@ package ipamutils
 import (
 	"fmt"
 	"net"
-	"sync"
 )
 
 var (
@@ -14,7 +13,6 @@ var (
 	// predefinedGlobalScopeDefaultNetworks contains a list of 64K IPv4 private networks with host size 8
 	// (10.x.x.x/24) which do not overlap with the networks in `PredefinedLocalScopeDefaultNetworks`
 	predefinedGlobalScopeDefaultNetworks []*net.IPNet
-	mutex                                sync.Mutex
 	localScopeDefaultNetworks            = []*NetworkToSplit{
 		{"172.17.0.0/16", 16},
 		{"172.18.0.0/16", 16},
@@ -48,26 +46,8 @@ func init() {
 	}
 }
 
-// ConfigGlobalScopeDefaultNetworks configures global default pool.
-// Ideally this will be called from SwarmKit as part of swarm init
-func ConfigGlobalScopeDefaultNetworks(defaultAddressPool []*NetworkToSplit) error {
-	if defaultAddressPool == nil {
-		return nil
-	}
-	mutex.Lock()
-	defer mutex.Unlock()
-	defaultNetworks, err := SplitNetworks(defaultAddressPool)
-	if err != nil {
-		return err
-	}
-	predefinedGlobalScopeDefaultNetworks = defaultNetworks
-	return nil
-}
-
 // GetGlobalScopeDefaultNetworks returns a copy of the global-sopce network list.
 func GetGlobalScopeDefaultNetworks() []*net.IPNet {
-	mutex.Lock()
-	defer mutex.Unlock()
 	return append([]*net.IPNet(nil), predefinedGlobalScopeDefaultNetworks...)
 }
 

libnetwork/ipamutils/utils_test.go

@@ -32,17 +32,6 @@ func initGranularPredefinedNetworks() []*net.IPNet {
 	return pl
 }
 
-func initGlobalScopeNetworks() []*net.IPNet {
-	pl := make([]*net.IPNet, 0, 256*256)
-	mask := []byte{255, 255, 255, 0}
-	for i := 0; i < 256; i++ {
-		for j := 0; j < 256; j++ {
-			pl = append(pl, &net.IPNet{IP: []byte{30, byte(i), byte(j), 0}, Mask: mask})
-		}
-	}
-	return pl
-}
-
 func TestDefaultNetwork(t *testing.T) {
 	for _, nw := range GetGlobalScopeDefaultNetworks() {
 		if ones, bits := nw.Mask.Size(); bits != 32 || ones != 24 {
@@ -83,21 +72,3 @@ func TestDefaultNetwork(t *testing.T) {
 
 	assert.Check(t, is.Len(m, 0))
 }
-
-func TestConfigGlobalScopeDefaultNetworks(t *testing.T) {
-	err := ConfigGlobalScopeDefaultNetworks([]*NetworkToSplit{{"30.0.0.0/8", 24}})
-	assert.NilError(t, err)
-
-	originalGlobalScopeNetworks := initGlobalScopeNetworks()
-	m := make(map[string]bool)
-	for _, v := range originalGlobalScopeNetworks {
-		m[v.String()] = true
-	}
-	for _, nw := range GetGlobalScopeDefaultNetworks() {
-		_, ok := m[nw.String()]
-		assert.Check(t, ok)
-		delete(m, nw.String())
-	}
-
-	assert.Check(t, is.Len(m, 0))
-}