Browse code

daemon: let libnetwork assign default bridge IPAM

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>

Cory Snider authored on 2023/01/17 07:25:13
Showing 3 changed files
... ...
@@ -4,16 +4,19 @@ import (
4 4
 	"bufio"
5 5
 	"fmt"
6 6
 	"io"
7
+	"net"
7 8
 	"os"
8 9
 	"regexp"
9 10
 	"strings"
10 11
 
11 12
 	"github.com/docker/docker/daemon/config"
13
+	"github.com/docker/docker/libnetwork/ns"
12 14
 	"github.com/docker/docker/libnetwork/resolvconf"
13 15
 	"github.com/moby/sys/mount"
14 16
 	"github.com/moby/sys/mountinfo"
15 17
 	"github.com/pkg/errors"
16 18
 	"github.com/sirupsen/logrus"
19
+	"github.com/vishvananda/netlink"
17 20
 )
18 21
 
19 22
 // On Linux, plugins use a static path for storing execution state,
... ...
@@ -141,3 +144,41 @@ func setupResolvConf(config *config.Config) {
141 141
 	}
142 142
 	config.ResolvConf = resolvconf.Path()
143 143
 }
144
+
145
+// ifaceAddrs returns the IPv4 and IPv6 addresses assigned to the network
146
+// interface with name linkName.
147
+//
148
+// No error is returned if the named interface does not exist.
149
+func ifaceAddrs(linkName string) (v4, v6 []*net.IPNet, err error) {
150
+	nl := ns.NlHandle()
151
+	link, err := nl.LinkByName(linkName)
152
+	if err != nil {
153
+		if !errors.As(err, new(netlink.LinkNotFoundError)) {
154
+			return nil, nil, err
155
+		}
156
+		return nil, nil, nil
157
+	}
158
+
159
+	get := func(family int) ([]*net.IPNet, error) {
160
+		addrs, err := nl.AddrList(link, family)
161
+		if err != nil {
162
+			return nil, err
163
+		}
164
+
165
+		ipnets := make([]*net.IPNet, len(addrs))
166
+		for i := range addrs {
167
+			ipnets[i] = addrs[i].IPNet
168
+		}
169
+		return ipnets, nil
170
+	}
171
+
172
+	v4, err = get(netlink.FAMILY_V4)
173
+	if err != nil {
174
+		return nil, nil, err
175
+	}
176
+	v6, err = get(netlink.FAMILY_V6)
177
+	if err != nil {
178
+		return nil, nil, err
179
+	}
180
+	return v4, v6, nil
181
+}
... ...
@@ -4,6 +4,7 @@
4 4
 package daemon // import "github.com/docker/docker/daemon"
5 5
 
6 6
 import (
7
+	"net"
7 8
 	"os"
8 9
 	"path/filepath"
9 10
 	"strings"
... ...
@@ -11,8 +12,12 @@ import (
11 11
 
12 12
 	containertypes "github.com/docker/docker/api/types/container"
13 13
 	"github.com/docker/docker/daemon/config"
14
+	"github.com/docker/docker/libnetwork/testutils"
15
+	"github.com/docker/docker/libnetwork/types"
16
+	"github.com/google/go-cmp/cmp/cmpopts"
14 17
 	"github.com/moby/sys/mount"
15 18
 	"github.com/moby/sys/mountinfo"
19
+	"github.com/vishvananda/netlink"
16 20
 	"gotest.tools/v3/assert"
17 21
 	is "gotest.tools/v3/assert/cmp"
18 22
 )
... ...
@@ -343,3 +348,66 @@ func TestRootMountCleanup(t *testing.T) {
343 343
 		assert.Assert(t, d.cleanupMounts())
344 344
 	})
345 345
 }
346
+
347
+func TestIfaceAddrs(t *testing.T) {
348
+	CIDR := func(cidr string) *net.IPNet {
349
+		t.Helper()
350
+		nw, err := types.ParseCIDR(cidr)
351
+		assert.NilError(t, err)
352
+		return nw
353
+	}
354
+
355
+	for _, tt := range []struct {
356
+		name string
357
+		nws  []*net.IPNet
358
+	}{
359
+		{
360
+			name: "Single",
361
+			nws:  []*net.IPNet{CIDR("172.101.202.254/16")},
362
+		},
363
+		{
364
+			name: "Multiple",
365
+			nws: []*net.IPNet{
366
+				CIDR("172.101.202.254/16"),
367
+				CIDR("172.102.202.254/16"),
368
+			},
369
+		},
370
+	} {
371
+		t.Run(tt.name, func(t *testing.T) {
372
+			defer testutils.SetupTestOSContext(t)()
373
+
374
+			createBridge(t, "test", tt.nws...)
375
+
376
+			ipv4Nw, ipv6Nw, err := ifaceAddrs("test")
377
+			if err != nil {
378
+				t.Fatal(err)
379
+			}
380
+
381
+			assert.Check(t, is.DeepEqual(tt.nws, ipv4Nw,
382
+				cmpopts.SortSlices(func(a, b *net.IPNet) bool { return a.String() < b.String() })))
383
+			// IPv6 link-local address
384
+			assert.Check(t, is.Len(ipv6Nw, 1))
385
+		})
386
+	}
387
+}
388
+
389
+func createBridge(t *testing.T, name string, bips ...*net.IPNet) {
390
+	t.Helper()
391
+
392
+	link := &netlink.Bridge{
393
+		LinkAttrs: netlink.LinkAttrs{
394
+			Name: name,
395
+		},
396
+	}
397
+	if err := netlink.LinkAdd(link); err != nil {
398
+		t.Fatalf("Failed to create interface via netlink: %v", err)
399
+	}
400
+	for _, bip := range bips {
401
+		if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: bip}); err != nil {
402
+			t.Fatal(err)
403
+		}
404
+	}
405
+	if err := netlink.LinkSetUp(link); err != nil {
406
+		t.Fatal(err)
407
+	}
408
+}
... ...
@@ -35,7 +35,6 @@ import (
35 35
 	nwconfig "github.com/docker/docker/libnetwork/config"
36 36
 	"github.com/docker/docker/libnetwork/drivers/bridge"
37 37
 	"github.com/docker/docker/libnetwork/netlabel"
38
-	"github.com/docker/docker/libnetwork/netutils"
39 38
 	"github.com/docker/docker/libnetwork/options"
40 39
 	lntypes "github.com/docker/docker/libnetwork/types"
41 40
 	"github.com/docker/docker/opts"
... ...
@@ -950,30 +949,37 @@ func initBridgeDriver(controller *libnetwork.Controller, config *config.Config)
950 950
 
951 951
 	ipamV4Conf := &libnetwork.IpamConf{AuxAddresses: make(map[string]string)}
952 952
 
953
-	nwList, nw6List, err := netutils.ElectInterfaceAddresses(bridgeName)
953
+	// By default, libnetwork will request an arbitrary available address
954
+	// pool for the network from the configured IPAM allocator.
955
+	// Configure it to use the IPv4 network ranges of the existing bridge
956
+	// interface if one exists with IPv4 addresses assigned to it.
957
+
958
+	nwList, nw6List, err := ifaceAddrs(bridgeName)
954 959
 	if err != nil {
955 960
 		return errors.Wrap(err, "list bridge addresses failed")
956 961
 	}
957 962
 
958
-	nw := nwList[0]
959
-	if len(nwList) > 1 && config.BridgeConfig.FixedCIDR != "" {
960
-		_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
961
-		if err != nil {
962
-			return errors.Wrap(err, "parse CIDR failed")
963
-		}
964
-		// Iterate through in case there are multiple addresses for the bridge
965
-		for _, entry := range nwList {
966
-			if fCIDR.Contains(entry.IP) {
967
-				nw = entry
968
-				break
963
+	if len(nwList) > 0 {
964
+		nw := nwList[0]
965
+		if len(nwList) > 1 && config.BridgeConfig.FixedCIDR != "" {
966
+			_, fCIDR, err := net.ParseCIDR(config.BridgeConfig.FixedCIDR)
967
+			if err != nil {
968
+				return errors.Wrap(err, "parse CIDR failed")
969
+			}
970
+			// Iterate through in case there are multiple addresses for the bridge
971
+			for _, entry := range nwList {
972
+				if fCIDR.Contains(entry.IP) {
973
+					nw = entry
974
+					break
975
+				}
969 976
 			}
970 977
 		}
971
-	}
972 978
 
973
-	ipamV4Conf.PreferredPool = lntypes.GetIPNetCanonical(nw).String()
974
-	hip, _ := lntypes.GetHostPartIP(nw.IP, nw.Mask)
975
-	if hip.IsGlobalUnicast() {
976
-		ipamV4Conf.Gateway = nw.IP.String()
979
+		ipamV4Conf.PreferredPool = lntypes.GetIPNetCanonical(nw).String()
980
+		hip, _ := lntypes.GetHostPartIP(nw.IP, nw.Mask)
981
+		if hip.IsGlobalUnicast() {
982
+			ipamV4Conf.Gateway = nw.IP.String()
983
+		}
977 984
 	}
978 985
 
979 986
 	if config.BridgeConfig.IP != "" {