0ct0pu5/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #46051 from thaJeztah/ipam_clean

Browse source 
libnetwork/ipam: assorted cleanup and refactor
This commit is contained in:
Sebastiaan van Stijn 2023-08-08 18:39:00 +02:00 • committed by GitHub
commit 2884018e7f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 153 additions and 123 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
libnetwork/ipam/allocator.go
View file

@ -64,29 +64,29 @@ func (a *Allocator) GetDefaultAddressSpaces() (string, string, error) {
// RequestPool returns an address pool along with its unique id.
// addressSpace must be a valid address space name and must not be the empty string.
// If pool is the empty string then the default predefined pool for addressSpace will be used, otherwise pool must be a valid IP address and length in CIDR notation.
// If subPool is not empty, it must be a valid IP address and length in CIDR notation which is a sub-range of pool.
// subPool must be empty if pool is empty.
func (a *Allocator) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
log.G(context.TODO()).Debugf("RequestPool(%s, %s, %s, %v, %t)", addressSpace, pool, subPool, options, v6)
// If requestedPool is the empty string then the default predefined pool for addressSpace will be used, otherwise pool must be a valid IP address and length in CIDR notation.
// If requestedSubPool is not empty, it must be a valid IP address and length in CIDR notation which is a sub-range of requestedPool.
// requestedSubPool must be empty if requestedPool is empty.
func (a *Allocator) RequestPool(addressSpace, requestedPool, requestedSubPool string, _ map[string]string, v6 bool) (poolID string, pool *net.IPNet, meta map[string]string, err error) {
log.G(context.TODO()).Debugf("RequestPool(%s, %s, %s, _, %t)", addressSpace, requestedPool, requestedSubPool, v6)
parseErr := func(err error) (string, *net.IPNet, map[string]string, error) {
return "", nil, nil, types.InternalErrorf("failed to parse pool request for address space %q pool %q subpool %q: %v", addressSpace, pool, subPool, err)
parseErr := func(err error) error {
return types.InternalErrorf("failed to parse pool request for address space %q pool %q subpool %q: %v", addressSpace, requestedPool, requestedSubPool, err)
}
if addressSpace == "" {
return parseErr(ipamapi.ErrInvalidAddressSpace)
return "", nil, nil, parseErr(ipamapi.ErrInvalidAddressSpace)
}
aSpace, err := a.getAddrSpace(addressSpace)
if err != nil {
return "", nil, nil, err
}
k := PoolID{AddressSpace: addressSpace}
if requestedPool == "" && requestedSubPool != "" {
return "", nil, nil, parseErr(ipamapi.ErrInvalidSubPool)
}
if pool == "" {
if subPool != "" {
return parseErr(ipamapi.ErrInvalidSubPool)
}
k := PoolID{AddressSpace: addressSpace}
if requestedPool == "" {
k.Subnet, err = aSpace.allocatePredefinedPool(v6)
if err != nil {
return "", nil, nil, err
@ -94,15 +94,14 @@ func (a *Allocator) RequestPool(addressSpace, pool, subPool string, options map[
return k.String(), toIPNet(k.Subnet), nil, nil
}
if k.Subnet, err = netip.ParsePrefix(pool); err != nil {
return parseErr(ipamapi.ErrInvalidPool)
if k.Subnet, err = netip.ParsePrefix(requestedPool); err != nil {
return "", nil, nil, parseErr(ipamapi.ErrInvalidPool)
}
if subPool != "" {
var err error
k.ChildSubnet, err = netip.ParsePrefix(subPool)
if requestedSubPool != "" {
k.ChildSubnet, err = netip.ParsePrefix(requestedSubPool)
if err != nil {
return parseErr(ipamapi.ErrInvalidSubPool)
return "", nil, nil, parseErr(ipamapi.ErrInvalidSubPool)
}
}
@ -118,8 +117,8 @@ func (a *Allocator) RequestPool(addressSpace, pool, subPool string, options map[
// ReleasePool releases the address pool identified by the passed id
func (a *Allocator) ReleasePool(poolID string) error {
log.G(context.TODO()).Debugf("ReleasePool(%s)", poolID)
k := PoolID{}
if err := k.FromString(poolID); err != nil {
k, err := PoolIDFromString(poolID)
if err != nil {
return types.BadRequestErrorf("invalid pool id: %s", poolID)
}
@ -227,8 +226,8 @@ func (aSpace *addrSpace) allocatePredefinedPool(ipV6 bool) (netip.Prefix, error)
// RequestAddress returns an address from the specified pool ID
func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[string]string) (*net.IPNet, map[string]string, error) {
log.G(context.TODO()).Debugf("RequestAddress(%s, %v, %v)", poolID, prefAddress, opts)
k := PoolID{}
if err := k.FromString(poolID); err != nil {
k, err := PoolIDFromString(poolID)
if err != nil {
return nil, nil, types.BadRequestErrorf("invalid pool id: %s", poolID)
}
@ -287,8 +286,8 @@ func (aSpace *addrSpace) requestAddress(nw, sub netip.Prefix, prefAddress netip.
// ReleaseAddress releases the address from the specified pool ID
func (a *Allocator) ReleaseAddress(poolID string, address net.IP) error {
log.G(context.TODO()).Debugf("ReleaseAddress(%s, %v)", poolID, address)
k := PoolID{}
if err := k.FromString(poolID); err != nil {
k, err := PoolIDFromString(poolID)
if err != nil {
return types.BadRequestErrorf("invalid pool id: %s", poolID)
}

37
libnetwork/ipam/allocator_test.go
View file

@ -29,8 +29,7 @@ func TestKeyString(t *testing.T) {
t.Fatalf("Unexpected key string: %s", k.String())
}
k2 := &PoolID{}
err := k2.FromString(expected)
k2, err := PoolIDFromString(expected)
if err != nil {
t.Fatal(err)
}
@ -44,7 +43,7 @@ func TestKeyString(t *testing.T) {
t.Fatalf("Unexpected key string: %s", k.String())
}
err = k2.FromString(expected)
k2, err = PoolIDFromString(expected)
if err != nil {
t.Fatal(err)
}
@ -133,7 +132,6 @@ func TestAddReleasePoolID(t *testing.T) {
a, err := NewAllocator(ipamutils.GetLocalScopeDefaultNetworks(), ipamutils.GetGlobalScopeDefaultNetworks())
assert.NilError(t, err)
var k0, k1 PoolID
_, err = a.getAddrSpace(localAddressSpace)
if err != nil {
t.Fatal(err)
@ -143,7 +141,8 @@ func TestAddReleasePoolID(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected failure in adding pool: %v", err)
}
if err := k0.FromString(pid0); err != nil {
k0, err := PoolIDFromString(pid0)
if err != nil {
t.Fatal(err)
}
@ -160,7 +159,8 @@ func TestAddReleasePoolID(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected failure in adding sub pool: %v", err)
}
if err := k1.FromString(pid1); err != nil {
k1, err := PoolIDFromString(pid1)
if err != nil {
t.Fatal(err)
}
@ -1276,3 +1276,28 @@ func TestParallelPredefinedRequest4(t *testing.T) {
func TestParallelPredefinedRequest5(t *testing.T) {
runParallelTests(t, 4)
}
func BenchmarkPoolIDToString(b *testing.B) {
const poolIDString = "default/172.27.0.0/16/172.27.3.0/24"
k, err := PoolIDFromString(poolIDString)
if err != nil {
b.Fatal(err)
}
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_ = k.String()
}
}
func BenchmarkPoolIDFromString(b *testing.B) {
const poolIDString = "default/172.27.0.0/16/172.27.3.0/24"
b.ReportAllocs()
for i := 0; i < b.N; i++ {
_, err := PoolIDFromString(poolIDString)
if err != nil {
b.Fatal(err)
}
}
}

46
libnetwork/ipam/structures.go
View file

@ -43,45 +43,39 @@ type addrSpace struct {
sync.Mutex
}
// String returns the string form of the SubnetKey object
func (s *PoolID) String() string {
k := fmt.Sprintf("%s/%s", s.AddressSpace, s.Subnet)
if s.ChildSubnet != (netip.Prefix{}) {
k = fmt.Sprintf("%s/%s", k, s.ChildSubnet)
}
return k
}
// FromString populates the SubnetKey object reading it from string
func (s *PoolID) FromString(str string) error {
if str == "" || !strings.Contains(str, "/") {
return types.BadRequestErrorf("invalid string form for subnetkey: %s", str)
// PoolIDFromString creates a new PoolID and populates the SubnetKey object
// reading it from the given string.
func PoolIDFromString(str string) (pID PoolID, err error) {
if str == "" {
return pID, types.BadRequestErrorf("invalid string form for subnetkey: %s", str)
}
p := strings.Split(str, "/")
if len(p) != 3 && len(p) != 5 {
return types.BadRequestErrorf("invalid string form for subnetkey: %s", str)
return pID, types.BadRequestErrorf("invalid string form for subnetkey: %s", str)
}
sub, err := netip.ParsePrefix(p[1] + "/" + p[2])
pID.AddressSpace = p[0]
pID.Subnet, err = netip.ParsePrefix(p[1] + "/" + p[2])
if err != nil {
return types.BadRequestErrorf("%v", err)
return pID, types.BadRequestErrorf("%v", err)
}
var child netip.Prefix
if len(p) == 5 {
child, err = netip.ParsePrefix(p[3] + "/" + p[4])
pID.ChildSubnet, err = netip.ParsePrefix(p[3] + "/" + p[4])
if err != nil {
return types.BadRequestErrorf("%v", err)
return pID, types.BadRequestErrorf("%v", err)
}
}
*s = PoolID{
AddressSpace: p[0],
SubnetKey: SubnetKey{
Subnet: sub,
ChildSubnet: child,
},
return pID, nil
}
// String returns the string form of the SubnetKey object
func (s *PoolID) String() string {
if s.ChildSubnet == (netip.Prefix{}) {
return s.AddressSpace + "/" + s.Subnet.String()
} else {
return s.AddressSpace + "/" + s.Subnet.String() + "/" + s.ChildSubnet.String()
}
return nil
}
// String returns the string form of the PoolData object

8
libnetwork/ipamapi/contract.go
View file

@ -51,12 +51,12 @@ type Ipam interface {
// GetDefaultAddressSpaces returns the default local and global address spaces for this ipam
GetDefaultAddressSpaces() (string, string, error)
// RequestPool returns an address pool along with its unique id. Address space is a mandatory field
// which denotes a set of non-overlapping pools. pool describes the pool of addresses in CIDR notation.
// subpool indicates a smaller range of addresses from the pool, for now it is specified in CIDR notation.
// Both pool and subpool are non mandatory fields. When they are not specified, Ipam driver may choose to
// which denotes a set of non-overlapping pools. requestedPool describes the pool of addresses in CIDR notation.
// requestedSubPool indicates a smaller range of addresses from the pool, for now it is specified in CIDR notation.
// Both requestedPool and requestedSubPool are non-mandatory fields. When they are not specified, Ipam driver may choose to
// return a self chosen pool for this request. In such case the v6 flag needs to be set appropriately so
// that the driver would return the expected ip version pool.
RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error)
RequestPool(addressSpace, requestedPool, requestedSubPool string, options map[string]string, v6 bool) (poolID string, pool *net.IPNet, meta map[string]string, err error)
// ReleasePool releases the address pool identified by the passed id
ReleasePool(poolID string) error
// RequestAddress request an address from the specified pool ID. Input options or required IP can be passed.

21
libnetwork/ipams/null/null.go
View file

@ -3,33 +3,34 @@
package null
import (
"fmt"
"net"
"github.com/docker/docker/libnetwork/ipamapi"
"github.com/docker/docker/libnetwork/types"
)
var (
defaultAS = "null"
defaultPool, _ = types.ParseCIDR("0.0.0.0/0")
defaultPoolID = fmt.Sprintf("%s/%s", defaultAS, defaultPool.String())
const (
defaultAddressSpace = "null"
defaultPoolCIDR = "0.0.0.0/0"
defaultPoolID = defaultAddressSpace + "/" + defaultPoolCIDR
)
var defaultPool, _ = types.ParseCIDR(defaultPoolCIDR)
type allocator struct{}
func (a *allocator) GetDefaultAddressSpaces() (string, string, error) {
return defaultAS, defaultAS, nil
return defaultAddressSpace, defaultAddressSpace, nil
}
func (a *allocator) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
if addressSpace != defaultAS {
func (a *allocator) RequestPool(addressSpace, requestedPool, requestedSubPool string, _ map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
if addressSpace != defaultAddressSpace {
return "", nil, nil, types.BadRequestErrorf("unknown address space: %s", addressSpace)
}
if pool != "" {
if requestedPool != "" {
return "", nil, nil, types.BadRequestErrorf("null ipam driver does not handle specific address pool requests")
}
if subPool != "" {
if requestedSubPool != "" {
return "", nil, nil, types.BadRequestErrorf("null ipam driver does not handle specific address subpool requests")
}
if v6 {

8
libnetwork/ipams/null/null_test.go
View file

@ -9,7 +9,7 @@ import (
func TestPoolRequest(t *testing.T) {
a := allocator{}
pid, pool, _, err := a.RequestPool(defaultAS, "", "", nil, false)
pid, pool, _, err := a.RequestPool(defaultAddressSpace, "", "", nil, false)
if err != nil {
t.Fatal(err)
}
@ -25,17 +25,17 @@ func TestPoolRequest(t *testing.T) {
t.Fatal("Unexpected success")
}
_, _, _, err = a.RequestPool(defaultAS, "192.168.0.0/16", "", nil, false)
_, _, _, err = a.RequestPool(defaultAddressSpace, "192.168.0.0/16", "", nil, false)
if err == nil {
t.Fatal("Unexpected success")
}
_, _, _, err = a.RequestPool(defaultAS, "", "192.168.0.0/24", nil, false)
_, _, _, err = a.RequestPool(defaultAddressSpace, "", "192.168.0.0/24", nil, false)
if err == nil {
t.Fatal("Unexpected success")
}
_, _, _, err = a.RequestPool(defaultAS, "", "", nil, true)
_, _, _, err = a.RequestPool(defaultAddressSpace, "", "", nil, true)
if err == nil {
t.Fatal("Unexpected success")
}

4
libnetwork/ipams/remote/remote.go
View file

@ -116,8 +116,8 @@ func (a *allocator) GetDefaultAddressSpaces() (string, string, error) {
}
// RequestPool requests an address pool in the specified address space
func (a *allocator) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
req := &api.RequestPoolRequest{AddressSpace: addressSpace, Pool: pool, SubPool: subPool, Options: options, V6: v6}
func (a *allocator) RequestPool(addressSpace, requestedPool, requestedSubPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
req := &api.RequestPoolRequest{AddressSpace: addressSpace, Pool: requestedPool, SubPool: requestedSubPool, Options: options, V6: v6}
res := &api.RequestPoolResponse{}
if err := a.call("RequestPool", req, res); err != nil {
return "", nil, nil, err

10
libnetwork/ipams/windowsipam/windowsipam.go
View file

@ -32,17 +32,17 @@ func (a *allocator) GetDefaultAddressSpaces() (string, string, error) {
// RequestPool returns an address pool along with its unique id. This is a null ipam driver. It allocates the
// subnet user asked and does not validate anything. Doesn't support subpool allocation
func (a *allocator) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
log.G(context.TODO()).Debugf("RequestPool(%s, %s, %s, %v, %t)", addressSpace, pool, subPool, options, v6)
if subPool != "" || v6 {
func (a *allocator) RequestPool(addressSpace, requestedPool, requestedSubPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
log.G(context.TODO()).Debugf("RequestPool(%s, %s, %s, %v, %t)", addressSpace, requestedPool, requestedSubPool, options, v6)
if requestedSubPool != "" || v6 {
return "", nil, nil, types.InternalErrorf("This request is not supported by null ipam driver")
}
var ipNet *net.IPNet
var err error
if pool != "" {
_, ipNet, err = net.ParseCIDR(pool)
if requestedPool != "" {
_, ipNet, err = net.ParseCIDR(requestedPool)
if err != nil {
return "", nil, nil, err
}

93
libnetwork/network.go
View file

@ -1524,16 +1524,43 @@ func (n *Network) ipamAllocate() error {
return err
}
func (n *Network) requestPoolHelper(ipam ipamapi.Ipam, addressSpace, preferredPool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
func (n *Network) requestPoolHelper(ipam ipamapi.Ipam, addressSpace, requestedPool, requestedSubPool string, options map[string]string, v6 bool) (poolID string, pool *net.IPNet, meta map[string]string, err error) {
var tmpPoolLeases []string
defer func() {
// Prevent repeated lock/unlock in the loop.
nwName := n.Name()
// Release all pools we held on to.
for _, pID := range tmpPoolLeases {
if err := ipam.ReleasePool(pID); err != nil {
log.G(context.TODO()).Warnf("Failed to release overlapping pool %s while returning from pool request helper for network %s", pool, nwName)
}
}
}()
for {
poolID, pool, meta, err := ipam.RequestPool(addressSpace, preferredPool, subPool, options, v6)
poolID, pool, meta, err = ipam.RequestPool(addressSpace, requestedPool, requestedSubPool, options, v6)
if err != nil {
return "", nil, nil, err
}
// If the network belongs to global scope or the pool was
// explicitly chosen or it is invalid, do not perform the overlap check.
if n.Scope() == scope.Global || preferredPool != "" || !types.IsIPNetValid(pool) {
// If the network pool was explicitly chosen, the network belongs to
// global scope, or it is invalid ("0.0.0.0/0"), then we don't perform
// check for overlaps.
//
// FIXME(thaJeztah): why are we ignoring invalid pools here?
//
// The "invalid" conditions was added in [libnetwork#1095][1], which
// moved code to reduce os-specific dependencies in the ipam package,
// but also introduced a types.IsIPNetValid() function, which considers
// "0.0.0.0/0" invalid, and added it to the conditions below.
//
// Unfortunately review does not mention this change, so there's no
// context why. Possibly this was done to prevent errors further down
// the line (when checking for overlaps), but returning an error here
// instead would likely have avoided that as well, so we can only guess.
//
// [1]: https://github.com/moby/libnetwork/commit/5ca79d6b87873264516323a7b76f0af7d0298492#diff-bdcd879439d041827d334846f9aba01de6e3683ed8fdd01e63917dae6df23846
if requestedPool != "" || n.Scope() == scope.Global || pool.String() == "0.0.0.0/0" {
return poolID, pool, meta, nil
}
@ -1542,26 +1569,12 @@ func (n *Network) requestPoolHelper(ipam ipamapi.Ipam, addressSpace, preferredPo
return poolID, pool, meta, nil
}
// Pool obtained in this iteration is
// overlapping. Hold onto the pool and don't release
// it yet, because we don't want ipam to give us back
// the same pool over again. But make sure we still do
// a deferred release when we have either obtained a
// non-overlapping pool or ran out of pre-defined
// pools.
defer func() {
if err := ipam.ReleasePool(poolID); err != nil {
log.G(context.TODO()).Warnf("Failed to release overlapping pool %s while returning from pool request helper for network %s", pool, n.Name())
}
}()
// If this is a preferred pool request and the network
// is local scope and there is an overlap, we fail the
// network creation right here. The pool will be
// released in the defer.
if preferredPool != "" {
return "", nil, nil, fmt.Errorf("requested subnet %s overlaps in the host", preferredPool)
}
// Pool obtained in this iteration is overlapping. Hold onto the pool
// and don't release it yet, because we don't want IPAM to give us back
// the same pool over again. But make sure we still do a deferred release
// when we have either obtained a non-overlapping pool or ran out of
// pre-defined pools.
tmpPoolLeases = append(tmpPoolLeases, poolID)
}
}
@ -1794,56 +1807,54 @@ func (n *Network) Scope() string {
return n.scope
}
func (n *Network) IpamConfig() (string, map[string]string, []*IpamConf, []*IpamConf) {
func (n *Network) IpamConfig() (ipamType string, ipamOptions map[string]string, ipamV4Config []*IpamConf, ipamV6Config []*IpamConf) {
n.mu.Lock()
defer n.mu.Unlock()
v4L := make([]*IpamConf, len(n.ipamV4Config))
v6L := make([]*IpamConf, len(n.ipamV6Config))
ipamV4Config = make([]*IpamConf, len(n.ipamV4Config))
for i, c := range n.ipamV4Config {
cc := &IpamConf{}
if err := c.CopyTo(cc); err != nil {
log.G(context.TODO()).WithError(err).Error("Error copying ipam ipv4 config")
}
v4L[i] = cc
ipamV4Config[i] = cc
}
ipamV6Config = make([]*IpamConf, len(n.ipamV6Config))
for i, c := range n.ipamV6Config {
cc := &IpamConf{}
if err := c.CopyTo(cc); err != nil {
log.G(context.TODO()).WithError(err).Debug("Error copying ipam ipv6 config")
}
v6L[i] = cc
ipamV6Config[i] = cc
}
return n.ipamType, n.ipamOptions, v4L, v6L
return n.ipamType, n.ipamOptions, ipamV4Config, ipamV6Config
}
func (n *Network) IpamInfo() ([]*IpamInfo, []*IpamInfo) {
func (n *Network) IpamInfo() (ipamV4Info []*IpamInfo, ipamV6Info []*IpamInfo) {
n.mu.Lock()
defer n.mu.Unlock()
v4Info := make([]*IpamInfo, len(n.ipamV4Info))
v6Info := make([]*IpamInfo, len(n.ipamV6Info))
ipamV4Info = make([]*IpamInfo, len(n.ipamV4Info))
for i, info := range n.ipamV4Info {
ic := &IpamInfo{}
if err := info.CopyTo(ic); err != nil {
log.G(context.TODO()).WithError(err).Error("Error copying ipv4 ipam config")
log.G(context.TODO()).WithError(err).Error("Error copying IPv4 IPAM config")
}
v4Info[i] = ic
ipamV4Info[i] = ic
}
ipamV6Info = make([]*IpamInfo, len(n.ipamV6Info))
for i, info := range n.ipamV6Info {
ic := &IpamInfo{}
if err := info.CopyTo(ic); err != nil {
log.G(context.TODO()).WithError(err).Error("Error copying ipv6 ipam config")
log.G(context.TODO()).WithError(err).Error("Error copying IPv6 IPAM config")
}
v6Info[i] = ic
ipamV6Info[i] = ic
}
return v4Info, v6Info
return ipamV4Info, ipamV6Info
}
func (n *Network) Internal() bool {