Skip to content

Commit

Permalink
Merge pull request moby#47727 from akerouanton/libnet-ipam-cleanup
Browse files Browse the repository at this point in the history
libnet/ipam: Various clean-ups
  • Loading branch information
akerouanton authored Apr 26, 2024
2 parents 16b2c22 + c5376e5 commit 48d769b
Show file tree
Hide file tree
Showing 37 changed files with 850 additions and 955 deletions.
2 changes: 2 additions & 0 deletions integration-cli/docker_api_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,8 @@ func getNetworkResource(c *testing.T, id string) *types.NetworkResource {
}

func createNetwork(c *testing.T, config types.NetworkCreateRequest, expectedStatusCode int) string {
c.Helper()

resp, body, err := request.Post(testutil.GetContext(c), "/networks/create", request.JSONBody(config))
assert.NilError(c, err)
defer resp.Body.Close()
Expand Down
18 changes: 5 additions & 13 deletions libnetwork/cnmallocator/drivers_ipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import (

"github.com/containerd/log"
"github.com/docker/docker/libnetwork/ipamapi"
builtinIpam "github.com/docker/docker/libnetwork/ipams/builtin"
nullIpam "github.com/docker/docker/libnetwork/ipams/null"
"github.com/docker/docker/libnetwork/ipams"
"github.com/docker/docker/libnetwork/ipamutils"
"github.com/moby/swarmkit/v2/manager/allocator/networkallocator"
)
Expand All @@ -33,20 +32,13 @@ 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())
}

for _, fn := range [](func(ipamapi.Registerer) error){
builtinIpam.Register,
nullIpam.Register,
} {
if err := fn(r); err != nil {
return err
}
if err := ipams.Register(r, nil, nil, addressPool); err != nil {
return err
}

return nil
Expand Down
18 changes: 12 additions & 6 deletions libnetwork/cnmallocator/networkallocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/docker/docker/libnetwork/drivers/remote"
"github.com/docker/docker/libnetwork/drvregistry"
"github.com/docker/docker/libnetwork/ipamapi"
"github.com/docker/docker/libnetwork/ipams/defaultipam"
remoteipam "github.com/docker/docker/libnetwork/ipams/remote"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/scope"
Expand Down Expand Up @@ -803,7 +804,7 @@ func (na *cnmNetworkAllocator) loadDriver(name string) error {

// Resolve the IPAM driver
func (na *cnmNetworkAllocator) resolveIPAM(n *api.Network) (ipamapi.Ipam, string, map[string]string, error) {
dName := ipamapi.DefaultIPAM
dName := defaultipam.DriverName
if n.Spec.IPAM != nil && n.Spec.IPAM.Driver != nil && n.Spec.IPAM.Driver.Name != "" {
dName = n.Spec.IPAM.Driver.Name
}
Expand Down Expand Up @@ -884,13 +885,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.
Expand All @@ -902,7 +908,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)
}
Expand All @@ -917,7 +923,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)
Expand All @@ -926,7 +932,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 == "" {
Expand Down
14 changes: 9 additions & 5 deletions libnetwork/cnmallocator/networkallocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion libnetwork/cnmallocator/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/docker/docker/libnetwork/driverapi"
"github.com/docker/docker/libnetwork/drivers/overlay/overlayutils"
"github.com/docker/docker/libnetwork/ipamapi"
"github.com/docker/docker/libnetwork/ipams/defaultipam"
"github.com/docker/docker/pkg/plugingetter"
"github.com/moby/swarmkit/v2/api"
"github.com/moby/swarmkit/v2/manager/allocator/networkallocator"
Expand Down Expand Up @@ -35,7 +36,7 @@ func (p *Provider) ValidateIPAMDriver(driver *api.Driver) error {
if driver.Name == "" {
return status.Errorf(codes.InvalidArgument, "driver name: if driver is specified name is required")
}
if strings.ToLower(driver.Name) == ipamapi.DefaultIPAM {
if strings.ToLower(driver.Name) == defaultipam.DriverName {
return nil
}
return p.validatePluginDriver(driver, ipamapi.PluginEndpointType)
Expand Down
3 changes: 2 additions & 1 deletion libnetwork/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import (
remotedriver "github.com/docker/docker/libnetwork/drivers/remote"
"github.com/docker/docker/libnetwork/drvregistry"
"github.com/docker/docker/libnetwork/ipamapi"
"github.com/docker/docker/libnetwork/ipams"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/osl"
"github.com/docker/docker/libnetwork/scope"
Expand Down Expand Up @@ -132,7 +133,7 @@ func New(cfgOptions ...config.Option) (*Controller, error) {
return nil, err
}

if err := initIPAMDrivers(&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
30 changes: 0 additions & 30 deletions libnetwork/drivers_ipam.go

This file was deleted.

8 changes: 2 additions & 6 deletions libnetwork/drvregistry/ipams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@ import (
"testing"

"github.com/docker/docker/libnetwork/ipamapi"
builtinIpam "github.com/docker/docker/libnetwork/ipams/builtin"
nullIpam "github.com/docker/docker/libnetwork/ipams/null"
remoteIpam "github.com/docker/docker/libnetwork/ipams/remote"
"github.com/docker/docker/libnetwork/ipams"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)

func getNewIPAMs(t *testing.T) *IPAMs {
r := &IPAMs{}

assert.Assert(t, builtinIpam.Register(r))
assert.Assert(t, remoteIpam.Register(r, nil))
assert.Assert(t, nullIpam.Register(r))
assert.Assert(t, ipams.Register(r, nil, nil, nil))

return r
}
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/endpoint_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"testing"

"github.com/docker/docker/internal/testutils/netnsutils"
"github.com/docker/docker/libnetwork/ipamapi"
"github.com/docker/docker/libnetwork/ipams/defaultipam"
"github.com/docker/docker/libnetwork/osl"
)

Expand All @@ -24,7 +24,7 @@ ff02::2 ip6-allrouters
fe90::2 somehost.example.com somehost
`

opts := []NetworkOption{NetworkOptionEnableIPv6(true), NetworkOptionIpam(ipamapi.DefaultIPAM, "",
opts := []NetworkOption{NetworkOptionEnableIPv6(true), NetworkOptionIpam(defaultipam.DriverName, "",
[]*IpamConf{{PreferredPool: "192.168.222.0/24", Gateway: "192.168.222.1"}},
[]*IpamConf{{PreferredPool: "fe90::/64", Gateway: "fe90::1"}},
nil)}
Expand Down
Loading

0 comments on commit 48d769b

Please sign in to comment.