From 0db56de78e2f7d156fe18ea85f0daa9e9e686f83 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Thu, 18 Apr 2024 15:53:49 +0200 Subject: [PATCH] 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 --- libnetwork/cnmallocator/drivers_ipam.go | 8 +++---- libnetwork/controller.go | 2 +- libnetwork/drvregistry/ipams_test.go | 3 +-- libnetwork/ipams/defaultipam/allocator.go | 21 +++++++++++----- libnetwork/ipams/drivers.go | 4 ++-- libnetwork/ipamutils/utils.go | 20 ---------------- libnetwork/ipamutils/utils_test.go | 29 ----------------------- 7 files changed, 22 insertions(+), 65 deletions(-) diff --git a/libnetwork/cnmallocator/drivers_ipam.go b/libnetwork/cnmallocator/drivers_ipam.go index 3688927019..8d0608c846 100644 --- a/libnetwork/cnmallocator/drivers_ipam.go +++ b/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 } diff --git a/libnetwork/controller.go b/libnetwork/controller.go index da76553f5e..dcb552406f 100644 --- a/libnetwork/controller.go +++ b/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 } diff --git a/libnetwork/drvregistry/ipams_test.go b/libnetwork/drvregistry/ipams_test.go index 0b708a72a1..eb8ac498e0 100644 --- a/libnetwork/drvregistry/ipams_test.go +++ b/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 } diff --git a/libnetwork/ipams/defaultipam/allocator.go b/libnetwork/ipams/defaultipam/allocator.go index e6cd587998..9285be91c6 100644 --- a/libnetwork/ipams/defaultipam/allocator.go +++ b/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 } diff --git a/libnetwork/ipams/drivers.go b/libnetwork/ipams/drivers.go index e75008d56a..f9232adc56 100644 --- a/libnetwork/ipams/drivers.go +++ b/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 { diff --git a/libnetwork/ipamutils/utils.go b/libnetwork/ipamutils/utils.go index 107f80755d..ed3a5b278e 100644 --- a/libnetwork/ipamutils/utils.go +++ b/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...) } diff --git a/libnetwork/ipamutils/utils_test.go b/libnetwork/ipamutils/utils_test.go index 6c0334f017..94221b424d 100644 --- a/libnetwork/ipamutils/utils_test.go +++ b/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)) -}