Skip to content

Commit

Permalink
daemon: let libnetwork assign default bridge IPAM
Browse files Browse the repository at this point in the history
The netutils.ElectInterfaceAddresses function is only used in one place
outside of tests: in the daemon, to configure the default bridge
network. The function is also messy to reason about as it references the
shared mutable state of ipamutils.PredefinedLocalScopeDefaultNetworks.
It uses the list of predefined default networks to always return an IPv4
address even if the named interface does not exist or does not have any
IPv4 addresses. This list happens to be the same as the one used to
initialize the address pool of the 'builtin' IPAM driver, though that is
far from obvious. (Start with "./libnetwork".initIPAMDrivers and trace
the dataflow of the addressPool value. Surprise! Global state is being
mutated using the value of other global mutable state.)

The daemon does not need the fallback behaviour of
ElectInterfaceAddresses. In fact, the daemon does not have to configure
an address pool for the network at all! libnetwork will acquire one of
the available address ranges from the network's IPAM driver when the
preferred-pool configuration is unset. It will do so using the same list
of address ranges and the exact same logic
(netutils.FindAvailableNetworks) as ElectInterfaceAddresses. So unless
the daemon needs to force the network to use a specific address range
because the bridge interface already exists, it can leave the details
up to libnetwork.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit cc19eba)
Signed-off-by: Albin Kerouanton <albinker@gmail.com>
  • Loading branch information
corhere authored and akerouanton committed Mar 31, 2023
1 parent 219f21b commit 063d3a6
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 18 deletions.
41 changes: 41 additions & 0 deletions daemon/daemon_linux.go
Expand Up @@ -4,16 +4,19 @@ import (
"bufio"
"fmt"
"io"
"net"
"os"
"regexp"
"strings"

"github.com/docker/docker/daemon/config"
"github.com/docker/docker/libnetwork/ns"
"github.com/docker/docker/libnetwork/resolvconf"
"github.com/moby/sys/mount"
"github.com/moby/sys/mountinfo"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/vishvananda/netlink"
)

// On Linux, plugins use a static path for storing execution state,
Expand Down Expand Up @@ -141,3 +144,41 @@ func setupResolvConf(config *config.Config) {
}
config.ResolvConf = resolvconf.Path()
}

// ifaceAddrs returns the IPv4 and IPv6 addresses assigned to the network
// interface with name linkName.
//
// No error is returned if the named interface does not exist.
func ifaceAddrs(linkName string) (v4, v6 []*net.IPNet, err error) {
nl := ns.NlHandle()
link, err := nl.LinkByName(linkName)
if err != nil {
if !errors.As(err, new(netlink.LinkNotFoundError)) {
return nil, nil, err
}
return nil, nil, nil
}

get := func(family int) ([]*net.IPNet, error) {
addrs, err := nl.AddrList(link, family)
if err != nil {
return nil, err
}

ipnets := make([]*net.IPNet, len(addrs))
for i := range addrs {
ipnets[i] = addrs[i].IPNet
}
return ipnets, nil
}

v4, err = get(netlink.FAMILY_V4)
if err != nil {
return nil, nil, err
}
v6, err = get(netlink.FAMILY_V6)
if err != nil {
return nil, nil, err
}
return v4, v6, nil
}
68 changes: 68 additions & 0 deletions daemon/daemon_linux_test.go
Expand Up @@ -4,15 +4,20 @@
package daemon // import "github.com/docker/docker/daemon"

import (
"net"
"os"
"path/filepath"
"strings"
"testing"

containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/daemon/config"
"github.com/docker/docker/libnetwork/testutils"
"github.com/docker/docker/libnetwork/types"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/moby/sys/mount"
"github.com/moby/sys/mountinfo"
"github.com/vishvananda/netlink"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
Expand Down Expand Up @@ -343,3 +348,66 @@ func TestRootMountCleanup(t *testing.T) {
assert.Assert(t, d.cleanupMounts())
})
}

func TestIfaceAddrs(t *testing.T) {
CIDR := func(cidr string) *net.IPNet {
t.Helper()
nw, err := types.ParseCIDR(cidr)
assert.NilError(t, err)
return nw
}

for _, tt := range []struct {
name string
nws []*net.IPNet
}{
{
name: "Single",
nws: []*net.IPNet{CIDR("172.101.202.254/16")},
},
{
name: "Multiple",
nws: []*net.IPNet{
CIDR("172.101.202.254/16"),
CIDR("172.102.202.254/16"),
},
},
} {
t.Run(tt.name, func(t *testing.T) {
defer testutils.SetupTestOSContext(t)()

createBridge(t, "test", tt.nws...)

ipv4Nw, ipv6Nw, err := ifaceAddrs("test")
if err != nil {
t.Fatal(err)
}

assert.Check(t, is.DeepEqual(tt.nws, ipv4Nw,
cmpopts.SortSlices(func(a, b *net.IPNet) bool { return a.String() < b.String() })))
// IPv6 link-local address
assert.Check(t, is.Len(ipv6Nw, 1))
})
}
}

func createBridge(t *testing.T, name string, bips ...*net.IPNet) {
t.Helper()

link := &netlink.Bridge{
LinkAttrs: netlink.LinkAttrs{
Name: name,
},
}
if err := netlink.LinkAdd(link); err != nil {
t.Fatalf("Failed to create interface via netlink: %v", err)
}
for _, bip := range bips {
if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
t.Fatal(err)
}
}
if err := netlink.LinkSetUp(link); err != nil {
t.Fatal(err)
}
}
42 changes: 24 additions & 18 deletions daemon/daemon_unix.go
Expand Up @@ -34,7 +34,6 @@ import (
nwconfig "github.com/docker/docker/libnetwork/config"
"github.com/docker/docker/libnetwork/drivers/bridge"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/netutils"
"github.com/docker/docker/libnetwork/options"
lntypes "github.com/docker/docker/libnetwork/types"
"github.com/docker/docker/opts"
Expand Down Expand Up @@ -950,30 +949,37 @@ func initBridgeDriver(controller libnetwork.NetworkController, config *config.Co

ipamV4Conf := &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}

nwList, nw6List, err := netutils.ElectInterfaceAddresses(bridgeName)
// By default, libnetwork will request an arbitrary available address
// pool for the network from the configured IPAM allocator.
// Configure it to use the IPv4 network ranges of the existing bridge
// interface if one exists with IPv4 addresses assigned to it.

nwList, nw6List, err := ifaceAddrs(bridgeName)
if err != nil {
return errors.Wrap(err, "list bridge addresses failed")
}

nw := nwList[0]
if len(nwList) > 1 && config.BridgeConfig.FixedCIDR != "" {
_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
if err != nil {
return errors.Wrap(err, "parse CIDR failed")
}
// Iterate through in case there are multiple addresses for the bridge
for _, entry := range nwList {
if fCIDR.Contains(entry.IP) {
nw = entry
break
if len(nwList) > 0 {
nw := nwList[0]
if len(nwList) > 1 && config.BridgeConfig.FixedCIDR != "" {
_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
if err != nil {
return errors.Wrap(err, "parse CIDR failed")
}
// Iterate through in case there are multiple addresses for the bridge
for _, entry := range nwList {
if fCIDR.Contains(entry.IP) {
nw = entry
break
}
}
}
}

ipamV4Conf.PreferredPool = lntypes.GetIPNetCanonical(nw).String()
hip, _ := lntypes.GetHostPartIP(nw.IP, nw.Mask)
if hip.IsGlobalUnicast() {
ipamV4Conf.Gateway = nw.IP.String()
ipamV4Conf.PreferredPool = lntypes.GetIPNetCanonical(nw).String()
hip, _ := lntypes.GetHostPartIP(nw.IP, nw.Mask)
if hip.IsGlobalUnicast() {
ipamV4Conf.Gateway = nw.IP.String()
}
}

if config.BridgeConfig.IP != "" {
Expand Down

0 comments on commit 063d3a6

Please sign in to comment.