libnet/ipamapi: add in/out structs for RequestPool

The `RequestPool` method has many args and named returns. This
makes the code hard to follow at times. This commit adds one struct,
`PoolRequest`, to replace these args, and one struct, `AllocatedPool`,
to replace these named returns.

Both structs' fields are properly documented to better define their
semantics, and their relationship with address allocation.

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

libnetwork/cnmallocator/networkallocator.go

@@ -884,13 +884,18 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string,
 	}
 
 	for i, ic := range ipamConfigs {
-		poolID, poolIP, meta, err := ipam.RequestPool(asName, ic.Subnet, ic.Range, dOptions, false)
+		alloc, err := ipam.RequestPool(ipamapi.PoolRequest{
+			AddressSpace: asName,
+			Pool:         ic.Subnet,
+			SubPool:      ic.Range,
+			Options:      dOptions,
+		})
 		if err != nil {
 			// Rollback by releasing all the resources allocated so far.
 			releasePools(ipam, ipamConfigs[:i], pools)
 			return nil, err
 		}
-		pools[poolIP.String()] = poolID
+		pools[alloc.Pool.String()] = alloc.PoolID
 
 		// The IPAM contract allows the IPAM driver to autonomously
 		// provide a network gateway in response to the pool request.
@@ -902,7 +907,7 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string,
 			gwIP *net.IPNet
 			ip   net.IP
 		)
-		if gws, ok := meta[netlabel.Gateway]; ok {
+		if gws, ok := alloc.Meta[netlabel.Gateway]; ok {
 			if ip, gwIP, err = net.ParseCIDR(gws); err != nil {
 				return nil, fmt.Errorf("failed to parse gateway address (%v) returned by ipam driver: %v", gws, err)
 			}
@@ -917,7 +922,7 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string,
 		defer delete(dOptions, ipamapi.RequestAddressType)
 
 		if ic.Gateway != "" || gwIP == nil {
-			gwIP, _, err = ipam.RequestAddress(poolID, net.ParseIP(ic.Gateway), dOptions)
+			gwIP, _, err = ipam.RequestAddress(alloc.PoolID, net.ParseIP(ic.Gateway), dOptions)
 			if err != nil {
 				// Rollback by releasing all the resources allocated so far.
 				releasePools(ipam, ipamConfigs[:i], pools)
@@ -926,7 +931,7 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string,
 		}
 
 		if ic.Subnet == "" {
-			ic.Subnet = poolIP.String()
+			ic.Subnet = alloc.Pool.String()
 		}
 
 		if ic.Gateway == "" {

libnetwork/cnmallocator/networkallocator_test.go

@@ -3,9 +3,10 @@ package cnmallocator
 import (
 	"fmt"
 	"net"
+	"net/netip"
 	"testing"
 
-	"github.com/docker/docker/libnetwork/types"
+	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/moby/swarmkit/v2/api"
 	"github.com/moby/swarmkit/v2/manager/allocator/networkallocator"
 	"gotest.tools/v3/assert"
@@ -728,11 +729,14 @@ func (a *mockIpam) GetDefaultAddressSpaces() (string, string, error) {
 	return "defaultAS", "defaultAS", nil
 }
 
-func (a *mockIpam) RequestPool(addressSpace, pool, subPool string, options map[string]string, v6 bool) (string, *net.IPNet, map[string]string, error) {
-	a.actualIpamOptions = options
+func (a *mockIpam) RequestPool(req ipamapi.PoolRequest) (ipamapi.AllocatedPool, error) {
+	a.actualIpamOptions = req.Options
 
-	poolCidr, _ := types.ParseCIDR(pool)
-	return fmt.Sprintf("%s/%s", "defaultAS", pool), poolCidr, nil, nil
+	poolCidr := netip.MustParsePrefix(req.Pool)
+	return ipamapi.AllocatedPool{
+		PoolID: fmt.Sprintf("defaultAS/%s", req.Pool),
+		Pool:   poolCidr,
+	}, nil
 }
 
 func (a *mockIpam) ReleasePool(poolID string) error {

libnetwork/ipam/allocator.go

@@ -91,41 +91,39 @@ func (a *Allocator) GetDefaultAddressSpaces() (string, string, error) {
 // 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)
+func (a *Allocator) RequestPool(req ipamapi.PoolRequest) (ipamapi.AllocatedPool, error) {
+	log.G(context.TODO()).Debugf("RequestPool: %+v", req)
 
 	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)
+		return types.InternalErrorf("failed to parse pool request for address space %q pool %q subpool %q: %v", req.AddressSpace, req.Pool, req.SubPool, err)
 	}
 
-	if addressSpace == "" {
-		return "", nil, nil, parseErr(ipamapi.ErrInvalidAddressSpace)
+	if req.AddressSpace == "" {
+		return ipamapi.AllocatedPool{}, parseErr(ipamapi.ErrInvalidAddressSpace)
 	}
-	aSpace, err := a.getAddrSpace(addressSpace, v6)
+	aSpace, err := a.getAddrSpace(req.AddressSpace, req.V6)
 	if err != nil {
-		return "", nil, nil, err
+		return ipamapi.AllocatedPool{}, err
 	}
-	if requestedPool == "" && requestedSubPool != "" {
-		return "", nil, nil, parseErr(ipamapi.ErrInvalidSubPool)
+	if req.Pool == "" && req.SubPool != "" {
+		return ipamapi.AllocatedPool{}, parseErr(ipamapi.ErrInvalidSubPool)
 	}
 
-	k := PoolID{AddressSpace: addressSpace}
-	if requestedPool == "" {
-		k.Subnet, err = aSpace.allocatePredefinedPool(v6)
-		if err != nil {
-			return "", nil, nil, err
+	k := PoolID{AddressSpace: req.AddressSpace}
+	if req.Pool == "" {
+		if k.Subnet, err = aSpace.allocatePredefinedPool(req.V6); err != nil {
+			return ipamapi.AllocatedPool{}, err
 		}
-		return k.String(), netiputil.ToIPNet(k.Subnet), nil, nil
+		return ipamapi.AllocatedPool{PoolID: k.String(), Pool: k.Subnet}, nil
 	}
 
-	if k.Subnet, err = netip.ParsePrefix(requestedPool); err != nil {
-		return "", nil, nil, parseErr(ipamapi.ErrInvalidPool)
+	if k.Subnet, err = netip.ParsePrefix(req.Pool); err != nil {
+		return ipamapi.AllocatedPool{}, parseErr(ipamapi.ErrInvalidPool)
 	}
 
-	if requestedSubPool != "" {
-		k.ChildSubnet, err = netip.ParsePrefix(requestedSubPool)
-		if err != nil {
-			return "", nil, nil, parseErr(ipamapi.ErrInvalidSubPool)
+	if req.SubPool != "" {
+		if k.ChildSubnet, err = netip.ParsePrefix(req.SubPool); err != nil {
+			return ipamapi.AllocatedPool{}, types.InternalErrorf("invalid pool request: %v", ipamapi.ErrInvalidSubPool)
 		}
 	}
 
@@ -140,10 +138,10 @@ func (a *Allocator) RequestPool(addressSpace, requestedPool, requestedSubPool st
 
 	err = aSpace.allocateSubnet(k.Subnet, k.ChildSubnet)
 	if err != nil {
-		return "", nil, nil, err
+		return ipamapi.AllocatedPool{}, types.ForbiddenErrorf("invalid pool request: %v", err)
 	}
 
-	return k.String(), netiputil.ToIPNet(k.Subnet), nil, nil
+	return ipamapi.AllocatedPool{PoolID: k.String(), Pool: k.Subnet}, nil
 }
 
 // ReleasePool releases the address pool identified by the passed id