1. docker
  2. Commit facb2323a014e30891fe554ab033452c69a6b86b
Browse code

Add tests for IPAM Config of default bridge

Signed-off-by: Rob Murray <rob.murray@docker.com>

Rob Murray authored on 2024/08/12 00:53:39
Showing 2 changed files
daemon/daemon_unix.go
History View file @ facb232
... ... 
@@ -927,10 +927,7 @@ func driverOptions(config *config.Config) nwconfig.Option {
927 927 
 }
928 928
929 929 
 func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig) error {
930 
-	bridgeName := bridge.DefaultBridgeName
931 
-	if cfg.Iface != "" {
932 
-		bridgeName = cfg.Iface
933 
-	}
930 
+	bridgeName, userManagedBridge := getDefaultBridgeName(cfg)
934 931 
 	netOption := map[string]string{
935 932 
 		bridge.BridgeName:         bridgeName,
936 933 
 		bridge.DefaultBridge:      strconv.FormatBool(true),
... ... 
@@ -938,7 +935,6 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
938 938 
 		bridge.EnableIPMasquerade: strconv.FormatBool(cfg.EnableIPMasq),
939 939 
 		bridge.EnableICC:          strconv.FormatBool(cfg.InterContainerCommunication),
940 940 
 	}
941 
-
942 941 
 	// --ip processing
943 942 
 	if cfg.DefaultIP != nil {
944 943 
 		netOption[bridge.DefaultBindingIP] = cfg.DefaultIP.String()
... ... 
@@ -986,7 +982,7 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
986 986 
 		}
987 987 
 		ipamV4Conf.PreferredPool = ipNet.String()
988 988 
 		ipamV4Conf.Gateway = ip.String()
989 
-	} else if bridgeName == bridge.DefaultBridgeName && ipamV4Conf.PreferredPool != "" {
989 
+	} else if !userManagedBridge && ipamV4Conf.PreferredPool != "" {
990 990 
 		log.G(context.TODO()).Infof("Default bridge (%s) is assigned with an IP address %s. Daemon option --bip can be used to set a preferred IP address", bridgeName, ipamV4Conf.PreferredPool)
991 991 
 	}
992 992
... ... 
@@ -1069,6 +1065,28 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
1069 1069 
 	return nil
1070 1070 
 }
1071 1071
1072 
+func getDefaultBridgeName(cfg config.BridgeConfig) (bridgeName string, userManagedBridge bool) {
1073 
+	// cfg.Iface is --bridge, the option to supply a user-managed bridge.
1074 
+	if cfg.Iface != "" {
1075 
+		// The default network will use a user-managed bridge, the daemon will not
1076 
+		// create it, and it is not possible to supply an address using --bip.
1077 
+		return cfg.Iface, true
1078 
+	}
1079 
+	// Without a --bridge, the bridge is "docker0", created and managed by the
1080 
+	// daemon. A --bip (cidr) can be supplied to define the bridge's IP address
1081 
+	// and the network's subnet.
1082 
+	//
1083 
+	// Env var DOCKER_TEST_CREATE_DEFAULT_BRIDGE env var modifies the default
1084 
+	// bridge name. Unlike '--bridge', the bridge does not need to be created
1085 
+	// outside the daemon, and it's still possible to use '--bip'. It is
1086 
+	// intended only for use in moby tests; it may be removed, its behaviour
1087 
+	// may be modified, and it may not do what you want anyway!
1088 
+	if bn := os.Getenv("DOCKER_TEST_CREATE_DEFAULT_BRIDGE"); bn != "" {
1089 
+		return bn, false
1090 
+	}
1091 
+	return bridge.DefaultBridgeName, false
1092 
+}
1093 
+
1072 1094 
 // Remove default bridge interface if present (--bridge=none use case)
1073 1095 
 func removeDefaultBridgeInterface() {
1074 1096 
 	if lnk, err := nlwrap.LinkByName(bridge.DefaultBridgeName); err == nil {
integration/daemon/daemon_linux_test.go
History View file @ facb232
... ... 
@@ -1,11 +1,18 @@
1 1 
 package daemon // import "github.com/docker/docker/integration/daemon"
2 2
3 3 
 import (
4 
+	"context"
5 
+	"net"
4 6 
 	"testing"
5 7
8 
+	"github.com/docker/docker/api/types/network"
6 9 
 	"github.com/docker/docker/testutil"
7 10 
 	"github.com/docker/docker/testutil/daemon"
11 
+	"github.com/vishvananda/netlink"
12 
+	"gotest.tools/v3/assert"
13 
+	is "gotest.tools/v3/assert/cmp"
8 14 
 	"gotest.tools/v3/icmd"
15 
+	"gotest.tools/v3/skip"
9 16 
 )
10 17
11 18 
 func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
... ... 
@@ -29,6 +36,289 @@ func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
29 29 
 	d.StartWithBusybox(ctx, t, "--bridge", bridgeName, "--fixed-cidr", "192.168.130.0/24")
30 30 
 }
31 31
32 
+// Test fixed-cidr and bip options, with various addresses on the bridge
33 
+// before the daemon starts.
34 
+func TestDaemonDefaultBridgeIPAM_Docker0(t *testing.T) {
35 
+	skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace")
36 
+	ctx := testutil.StartSpan(baseContext, t)
37 
+
38 
+	testcases := []defaultBridgeIPAMTestCase{
39 
+		{
40 
+			name:       "fixed-cidr only",
41 
+			daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
42 
+			expIPAMConfig: []network.IPAMConfig{
43 
+				{
44 
+					Subnet:  "192.168.176.0/24",
45 
+					IPRange: "192.168.176.0/24",
46 
+				},
47 
+			},
48 
+		},
49 
+		{
50 
+			name:       "bip only",
51 
+			daemonArgs: []string{"--bip", "192.168.176.88/24"},
52 
+			expIPAMConfig: []network.IPAMConfig{
53 
+				{
54 
+					Subnet:  "192.168.176.0/24",
55 
+					Gateway: "192.168.176.88",
56 
+				},
57 
+			},
58 
+		},
59 
+		{
60 
+			name:               "existing bridge address only",
61 
+			initialBridgeAddrs: []string{"192.168.176.88/24"},
62 
+			expIPAMConfig: []network.IPAMConfig{
63 
+				{
64 
+					Subnet:  "192.168.176.0/24",
65 
+					Gateway: "192.168.176.88",
66 
+				},
67 
+			},
68 
+		},
69 
+		{
70 
+			name:               "fixed-cidr within old bridge subnet",
71 
+			initialBridgeAddrs: []string{"192.168.176.88/20"},
72 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/24"},
73 
+			// There's no --bip to dictate the subnet, so it's derived from an
74 
+			// existing bridge address. If fixed-cidr's subnet is made smaller
75 
+			// following a daemon restart, a user might reasonably expect the
76 
+			// default bridge network's subnet to shrink to match. However,
77 
+			// that has not been the behaviour - instead, only the allocatable
78 
+			// range is reduced (as would happen with a user-managed bridge).
79 
+			// In this case, if the user wants a smaller subnet, their options
80 
+			// are to delete docker0, or supply a --bip. A change in this subtle
81 
+			// behaviour might be best. But, it's probably not causing problems,
82 
+			// and it'd be a breaking change for anyone relying on the existing
83 
+			// behaviour.
84 
+			expIPAMConfig: []network.IPAMConfig{
85 
+				{
86 
+					Subnet:  "192.168.176.0/20",
87 
+					IPRange: "192.168.176.0/24",
88 
+					Gateway: "192.168.176.88",
89 
+				},
90 
+			},
91 
+		},
92 
+		{
93 
+			name:               "fixed-cidr within old bridge subnet with new bip",
94 
+			initialBridgeAddrs: []string{"192.168.176.88/20"},
95 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/24", "--bip", "192.168.176.99/24"},
96 
+			expIPAMConfig: []network.IPAMConfig{
97 
+				{
98 
+					Subnet:  "192.168.176.0/24",
99 
+					IPRange: "192.168.176.0/24",
100 
+					Gateway: "192.168.176.99",
101 
+				},
102 
+			},
103 
+		},
104 
+		{
105 
+			name:               "old bridge subnet within fixed-cidr",
106 
+			initialBridgeAddrs: []string{"192.168.176.88/24"},
107 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/20"},
108 
+			expIPAMConfig: []network.IPAMConfig{
109 
+				{
110 
+					// FIXME(robmry) - subnet didn't change, allocatable range
111 
+					//   is bigger than the subnet.
112 
+					Subnet:  "192.168.176.0/24",
113 
+					IPRange: "192.168.176.0/20",
114 
+					Gateway: "192.168.176.88",
115 
+				},
116 
+			},
117 
+		},
118 
+		{
119 
+			name:               "old bridge subnet outside fixed-cidr",
120 
+			initialBridgeAddrs: []string{"192.168.176.88/24"},
121 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.177.0/24"},
122 
+			expIPAMConfig: []network.IPAMConfig{
123 
+				{
124 
+					// FIXME(robmry) - subnet and bridge address haven't changed,
125 
+					//   and the allocatable range is outside the subnet.
126 
+					Subnet:  "192.168.176.0/24",
127 
+					IPRange: "192.168.177.0/24",
128 
+					Gateway: "192.168.176.88",
129 
+				},
130 
+			},
131 
+		},
132 
+		{
133 
+			name:               "old bridge subnet outside fixed-cidr with bip",
134 
+			initialBridgeAddrs: []string{"192.168.176.88/24"},
135 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.177.0/24", "--bip", "192.168.177.99/24"},
136 
+			expIPAMConfig: []network.IPAMConfig{
137 
+				{
138 
+					Subnet:  "192.168.177.0/24",
139 
+					IPRange: "192.168.177.0/24",
140 
+					Gateway: "192.168.177.99",
141 
+				},
142 
+			},
143 
+		},
144 
+	}
145 
+	for _, tc := range testcases {
146 
+		testDefaultBridgeIPAM(ctx, t, tc)
147 
+	}
148 
+}
149 
+
150 
+// Like TestDaemonUserDefaultBridgeIPAMDocker0, but with a user-defined/supplied
151 
+// bridge, instead of docker0.
152 
+func TestDaemonDefaultBridgeIPAM_UserBr(t *testing.T) {
153 
+	skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace")
154 
+	ctx := testutil.StartSpan(baseContext, t)
155 
+
156 
+	testcases := []defaultBridgeIPAMTestCase{
157 
+		{
158 
+			name:               "bridge only",
159 
+			initialBridgeAddrs: []string{"192.168.176.88/20"},
160 
+			expIPAMConfig: []network.IPAMConfig{
161 
+				{
162 
+					Subnet:  "192.168.176.0/20",
163 
+					Gateway: "192.168.176.88",
164 
+				},
165 
+			},
166 
+		},
167 
+		{
168 
+			name:       "fixed-cidr only",
169 
+			daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
170 
+			expIPAMConfig: []network.IPAMConfig{
171 
+				{
172 
+					Subnet:  "192.168.176.0/24",
173 
+					IPRange: "192.168.176.0/24",
174 
+				},
175 
+			},
176 
+		},
177 
+		{
178 
+			name:               "fcidr in bridge subnet and bridge ip in fcidr",
179 
+			initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"},
180 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/24"},
181 
+			// Selected bip should be the one within fixed-cidr
182 
+			expIPAMConfig: []network.IPAMConfig{
183 
+				{
184 
+					Subnet:  "192.168.176.0/20",
185 
+					IPRange: "192.168.176.0/24",
186 
+					Gateway: "192.168.176.88",
187 
+				},
188 
+			},
189 
+		},
190 
+		{
191 
+			name:               "fcidr in bridge subnet and bridge ip not in fcidr",
192 
+			initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"},
193 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.177.0/24"},
194 
+			expIPAMConfig: []network.IPAMConfig{
195 
+				{
196 
+					// FIXME(robmry) - selected subnet should be the one that encompasses
197 
+					//  fixed-cidr, allocatable range is outside the subnet.
198 
+					Subnet:  "192.168.160.0/20",
199 
+					IPRange: "192.168.177.0/24",
200 
+					Gateway: "192.168.160.88",
201 
+				},
202 
+			},
203 
+		},
204 
+		{
205 
+			name:               "fixed-cidr bigger than bridge subnet",
206 
+			initialBridgeAddrs: []string{"192.168.176.88/24"},
207 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/20"},
208 
+			expIPAMConfig: []network.IPAMConfig{
209 
+				{
210 
+					Subnet: "192.168.176.0/24",
211 
+					// FIXME(robmry) - allocatable range is bigger than the subnet.
212 
+					IPRange: "192.168.176.0/20",
213 
+					Gateway: "192.168.176.88",
214 
+				},
215 
+			},
216 
+		},
217 
+		{
218 
+			name:               "no bridge ip within fixed-cidr",
219 
+			initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.192.88/20"},
220 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/24"},
221 
+			// Selected bip should be the one within fixed-cidr
222 
+			expIPAMConfig: []network.IPAMConfig{
223 
+				{
224 
+					// FIXME(robmry) - allocatable range outside subnet.
225 
+					Subnet:  "192.168.160.0/20",
226 
+					IPRange: "192.168.176.0/24",
227 
+					Gateway: "192.168.160.88",
228 
+				},
229 
+			},
230 
+		},
231 
+		{
232 
+			name:               "fixed-cidr contains bridge subnet",
233 
+			initialBridgeAddrs: []string{"192.168.177.1/24"},
234 
+			daemonArgs:         []string{"--fixed-cidr", "192.168.176.0/20"},
235 
+			expIPAMConfig: []network.IPAMConfig{
236 
+				{
237 
+					Subnet: "192.168.177.0/24",
238 
+					// FIXME(robmry) - allocatable range is bigger than subnet
239 
+					IPRange: "192.168.176.0/20",
240 
+					Gateway: "192.168.177.1",
241 
+				},
242 
+			},
243 
+		},
244 
+	}
245 
+	for _, tc := range testcases {
246 
+		tc.userDefinedBridge = true
247 
+		testDefaultBridgeIPAM(ctx, t, tc)
248 
+	}
249 
+}
250 
+
251 
+type defaultBridgeIPAMTestCase struct {
252 
+	name               string
253 
+	userDefinedBridge  bool
254 
+	initialBridgeAddrs []string
255 
+	daemonArgs         []string
256 
+	expIPAMConfig      []network.IPAMConfig
257 
+}
258 
+
259 
+func testDefaultBridgeIPAM(ctx context.Context, t *testing.T, tc defaultBridgeIPAMTestCase) {
260 
+	t.Run(tc.name, func(t *testing.T) {
261 
+		ctx := testutil.StartSpan(ctx, t)
262 
+		const bridgeName = "br-dbi"
263 
+
264 
+		createBridge(t, bridgeName, tc.initialBridgeAddrs)
265 
+		defer deleteInterface(t, bridgeName)
266 
+
267 
+		var dOpts []daemon.Option
268 
+		dArgs := tc.daemonArgs
269 
+		if tc.userDefinedBridge {
270 
+			// If a bridge is supplied by the user, the daemon should use its addresses
271 
+			// to infer --bip (which cannot be specified).
272 
+			dArgs = append(dArgs, []string{"--bridge", bridgeName}...)
273 
+		} else {
274 
+			// The bridge is created and managed by docker, it's always called "docker0",
275 
+			// unless this test-only env var is set - to avoid conflict with the docker0
276 
+			// belonging to the daemon started in CI runs.
277 
+			dOpts = append(dOpts, daemon.WithEnvVars("DOCKER_TEST_CREATE_DEFAULT_BRIDGE="+bridgeName))
278 
+		}
279 
+
280 
+		d := daemon.New(t, dOpts...)
281 
+		defer func() {
282 
+			d.Stop(t)
283 
+			d.Cleanup(t)
284 
+		}()
285 
+		d.StartWithBusybox(ctx, t, dArgs...)
286 
+		c := d.NewClientT(t)
287 
+		defer c.Close()
288 
+
289 
+		insp, err := c.NetworkInspect(ctx, network.NetworkBridge, network.InspectOptions{})
290 
+		assert.NilError(t, err)
291 
+		assert.Check(t, is.DeepEqual(insp.IPAM.Config, tc.expIPAMConfig))
292 
+	})
293 
+}
294 
+
295 
+func createBridge(t *testing.T, ifName string, addrs []string) {
296 
+	t.Helper()
297 
+
298 
+	link := &netlink.Bridge{
299 
+		LinkAttrs: netlink.LinkAttrs{
300 
+			Name: ifName,
301 
+		},
302 
+	}
303 
+
304 
+	err := netlink.LinkAdd(link)
305 
+	assert.NilError(t, err)
306 
+	for _, addr := range addrs {
307 
+		ip, ipNet, err := net.ParseCIDR(addr)
308 
+		assert.NilError(t, err)
309 
+		ipNet.IP = ip
310 
+		err = netlink.AddrAdd(link, &netlink.Addr{IPNet: ipNet})
311 
+		assert.NilError(t, err)
312 
+	}
313 
+}
314 
+
32 315 
 func deleteInterface(t *testing.T, ifName string) {
33 316 
 	icmd.RunCommand("ip", "link", "delete", ifName).Assert(t, icmd.Success)
34 317 
 	icmd.RunCommand("iptables", "-t", "nat", "--flush").Assert(t, icmd.Success)