Skip to content

Commit

Permalink
libnet/ipamutils: no more global state
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
akerouanton committed Apr 26, 2024
1 parent 3c97181 commit 0db56de
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 65 deletions.
8 changes: 3 additions & 5 deletions libnetwork/cnmallocator/drivers_ipam.go
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion libnetwork/controller.go
Expand Up @@ -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
}

Expand Down
3 changes: 1 addition & 2 deletions libnetwork/drvregistry/ipams_test.go
Expand Up @@ -7,15 +7,14 @@ 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"
)

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
}
Expand Down
21 changes: 15 additions & 6 deletions libnetwork/ipams/defaultipam/allocator.go
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/ipams/drivers.go
Expand Up @@ -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 {
Expand Down
20 changes: 0 additions & 20 deletions libnetwork/ipamutils/utils.go
Expand Up @@ -4,7 +4,6 @@ package ipamutils
import (
"fmt"
"net"
"sync"
)

var (
Expand All @@ -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},
Expand Down Expand Up @@ -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...)
}

Expand Down
29 changes: 0 additions & 29 deletions libnetwork/ipamutils/utils_test.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}

0 comments on commit 0db56de

Please sign in to comment.