Skip to content

Commit facb232

Browse files
robmryrobmry
committed
Add tests for IPAM Config of default bridge
Signed-off-by: Rob Murray <[email protected]>
1 parent 4a2bd10 commit facb232

2 files changed

Lines changed: 314 additions & 6 deletions

File tree

‎daemon/daemon_unix.go‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -927,18 +927,14 @@ func driverOptions(config *config.Config) nwconfig.Option {
927927
}
928928

929929
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)
934931
netOption := map[string]string{
935932
bridge.BridgeName: bridgeName,
936933
bridge.DefaultBridge: strconv.FormatBool(true),
937934
netlabel.DriverMTU: strconv.Itoa(cfg.MTU),
938935
bridge.EnableIPMasquerade: strconv.FormatBool(cfg.EnableIPMasq),
939936
bridge.EnableICC: strconv.FormatBool(cfg.InterContainerCommunication),
940937
}
941-
942938
// --ip processing
943939
if cfg.DefaultIP != nil {
944940
netOption[bridge.DefaultBindingIP] = cfg.DefaultIP.String()
@@ -986,7 +982,7 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
986982
}
987983
ipamV4Conf.PreferredPool = ipNet.String()
988984
ipamV4Conf.Gateway = ip.String()
989-
} else if bridgeName == bridge.DefaultBridgeName && ipamV4Conf.PreferredPool != "" {
985+
} else if !userManagedBridge && ipamV4Conf.PreferredPool != "" {
990986
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)
991987
}
992988

@@ -1069,6 +1065,28 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig
10691065
return nil
10701066
}
10711067

1068+
func getDefaultBridgeName(cfg config.BridgeConfig) (bridgeName string, userManagedBridge bool) {
1069+
// cfg.Iface is --bridge, the option to supply a user-managed bridge.
1070+
if cfg.Iface != "" {
1071+
// The default network will use a user-managed bridge, the daemon will not
1072+
// create it, and it is not possible to supply an address using --bip.
1073+
return cfg.Iface, true
1074+
}
1075+
// Without a --bridge, the bridge is "docker0", created and managed by the
1076+
// daemon. A --bip (cidr) can be supplied to define the bridge's IP address
1077+
// and the network's subnet.
1078+
//
1079+
// Env var DOCKER_TEST_CREATE_DEFAULT_BRIDGE env var modifies the default
1080+
// bridge name. Unlike '--bridge', the bridge does not need to be created
1081+
// outside the daemon, and it's still possible to use '--bip'. It is
1082+
// intended only for use in moby tests; it may be removed, its behaviour
1083+
// may be modified, and it may not do what you want anyway!
1084+
if bn := os.Getenv("DOCKER_TEST_CREATE_DEFAULT_BRIDGE"); bn != "" {
1085+
return bn, false
1086+
}
1087+
return bridge.DefaultBridgeName, false
1088+
}
1089+
10721090
// Remove default bridge interface if present (--bridge=none use case)
10731091
func removeDefaultBridgeInterface() {
10741092
if lnk, err := nlwrap.LinkByName(bridge.DefaultBridgeName); err == nil {

‎integration/daemon/daemon_linux_test.go‎

Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
package daemon // import "github.com/docker/docker/integration/daemon"
22

33
import (
4+
"context"
5+
"net"
46
"testing"
57

8+
"github.com/docker/docker/api/types/network"
69
"github.com/docker/docker/testutil"
710
"github.com/docker/docker/testutil/daemon"
11+
"github.com/vishvananda/netlink"
12+
"gotest.tools/v3/assert"
13+
is "gotest.tools/v3/assert/cmp"
814
"gotest.tools/v3/icmd"
15+
"gotest.tools/v3/skip"
916
)
1017

1118
func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
@@ -29,6 +36,289 @@ func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) {
2936
d.StartWithBusybox(ctx, t, "--bridge", bridgeName, "--fixed-cidr", "192.168.130.0/24")
3037
}
3138

39+
// Test fixed-cidr and bip options, with various addresses on the bridge
40+
// before the daemon starts.
41+
func TestDaemonDefaultBridgeIPAM_Docker0(t *testing.T) {
42+
skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace")
43+
ctx := testutil.StartSpan(baseContext, t)
44+
45+
testcases := []defaultBridgeIPAMTestCase{
46+
{
47+
name: "fixed-cidr only",
48+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
49+
expIPAMConfig: []network.IPAMConfig{
50+
{
51+
Subnet: "192.168.176.0/24",
52+
IPRange: "192.168.176.0/24",
53+
},
54+
},
55+
},
56+
{
57+
name: "bip only",
58+
daemonArgs: []string{"--bip", "192.168.176.88/24"},
59+
expIPAMConfig: []network.IPAMConfig{
60+
{
61+
Subnet: "192.168.176.0/24",
62+
Gateway: "192.168.176.88",
63+
},
64+
},
65+
},
66+
{
67+
name: "existing bridge address only",
68+
initialBridgeAddrs: []string{"192.168.176.88/24"},
69+
expIPAMConfig: []network.IPAMConfig{
70+
{
71+
Subnet: "192.168.176.0/24",
72+
Gateway: "192.168.176.88",
73+
},
74+
},
75+
},
76+
{
77+
name: "fixed-cidr within old bridge subnet",
78+
initialBridgeAddrs: []string{"192.168.176.88/20"},
79+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
80+
// There's no --bip to dictate the subnet, so it's derived from an
81+
// existing bridge address. If fixed-cidr's subnet is made smaller
82+
// following a daemon restart, a user might reasonably expect the
83+
// default bridge network's subnet to shrink to match. However,
84+
// that has not been the behaviour - instead, only the allocatable
85+
// range is reduced (as would happen with a user-managed bridge).
86+
// In this case, if the user wants a smaller subnet, their options
87+
// are to delete docker0, or supply a --bip. A change in this subtle
88+
// behaviour might be best. But, it's probably not causing problems,
89+
// and it'd be a breaking change for anyone relying on the existing
90+
// behaviour.
91+
expIPAMConfig: []network.IPAMConfig{
92+
{
93+
Subnet: "192.168.176.0/20",
94+
IPRange: "192.168.176.0/24",
95+
Gateway: "192.168.176.88",
96+
},
97+
},
98+
},
99+
{
100+
name: "fixed-cidr within old bridge subnet with new bip",
101+
initialBridgeAddrs: []string{"192.168.176.88/20"},
102+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24", "--bip", "192.168.176.99/24"},
103+
expIPAMConfig: []network.IPAMConfig{
104+
{
105+
Subnet: "192.168.176.0/24",
106+
IPRange: "192.168.176.0/24",
107+
Gateway: "192.168.176.99",
108+
},
109+
},
110+
},
111+
{
112+
name: "old bridge subnet within fixed-cidr",
113+
initialBridgeAddrs: []string{"192.168.176.88/24"},
114+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"},
115+
expIPAMConfig: []network.IPAMConfig{
116+
{
117+
// FIXME(robmry) - subnet didn't change, allocatable range
118+
// is bigger than the subnet.
119+
Subnet: "192.168.176.0/24",
120+
IPRange: "192.168.176.0/20",
121+
Gateway: "192.168.176.88",
122+
},
123+
},
124+
},
125+
{
126+
name: "old bridge subnet outside fixed-cidr",
127+
initialBridgeAddrs: []string{"192.168.176.88/24"},
128+
daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24"},
129+
expIPAMConfig: []network.IPAMConfig{
130+
{
131+
// FIXME(robmry) - subnet and bridge address haven't changed,
132+
// and the allocatable range is outside the subnet.
133+
Subnet: "192.168.176.0/24",
134+
IPRange: "192.168.177.0/24",
135+
Gateway: "192.168.176.88",
136+
},
137+
},
138+
},
139+
{
140+
name: "old bridge subnet outside fixed-cidr with bip",
141+
initialBridgeAddrs: []string{"192.168.176.88/24"},
142+
daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24", "--bip", "192.168.177.99/24"},
143+
expIPAMConfig: []network.IPAMConfig{
144+
{
145+
Subnet: "192.168.177.0/24",
146+
IPRange: "192.168.177.0/24",
147+
Gateway: "192.168.177.99",
148+
},
149+
},
150+
},
151+
}
152+
for _, tc := range testcases {
153+
testDefaultBridgeIPAM(ctx, t, tc)
154+
}
155+
}
156+
157+
// Like TestDaemonUserDefaultBridgeIPAMDocker0, but with a user-defined/supplied
158+
// bridge, instead of docker0.
159+
func TestDaemonDefaultBridgeIPAM_UserBr(t *testing.T) {
160+
skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace")
161+
ctx := testutil.StartSpan(baseContext, t)
162+
163+
testcases := []defaultBridgeIPAMTestCase{
164+
{
165+
name: "bridge only",
166+
initialBridgeAddrs: []string{"192.168.176.88/20"},
167+
expIPAMConfig: []network.IPAMConfig{
168+
{
169+
Subnet: "192.168.176.0/20",
170+
Gateway: "192.168.176.88",
171+
},
172+
},
173+
},
174+
{
175+
name: "fixed-cidr only",
176+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
177+
expIPAMConfig: []network.IPAMConfig{
178+
{
179+
Subnet: "192.168.176.0/24",
180+
IPRange: "192.168.176.0/24",
181+
},
182+
},
183+
},
184+
{
185+
name: "fcidr in bridge subnet and bridge ip in fcidr",
186+
initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"},
187+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
188+
// Selected bip should be the one within fixed-cidr
189+
expIPAMConfig: []network.IPAMConfig{
190+
{
191+
Subnet: "192.168.176.0/20",
192+
IPRange: "192.168.176.0/24",
193+
Gateway: "192.168.176.88",
194+
},
195+
},
196+
},
197+
{
198+
name: "fcidr in bridge subnet and bridge ip not in fcidr",
199+
initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"},
200+
daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24"},
201+
expIPAMConfig: []network.IPAMConfig{
202+
{
203+
// FIXME(robmry) - selected subnet should be the one that encompasses
204+
// fixed-cidr, allocatable range is outside the subnet.
205+
Subnet: "192.168.160.0/20",
206+
IPRange: "192.168.177.0/24",
207+
Gateway: "192.168.160.88",
208+
},
209+
},
210+
},
211+
{
212+
name: "fixed-cidr bigger than bridge subnet",
213+
initialBridgeAddrs: []string{"192.168.176.88/24"},
214+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"},
215+
expIPAMConfig: []network.IPAMConfig{
216+
{
217+
Subnet: "192.168.176.0/24",
218+
// FIXME(robmry) - allocatable range is bigger than the subnet.
219+
IPRange: "192.168.176.0/20",
220+
Gateway: "192.168.176.88",
221+
},
222+
},
223+
},
224+
{
225+
name: "no bridge ip within fixed-cidr",
226+
initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.192.88/20"},
227+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"},
228+
// Selected bip should be the one within fixed-cidr
229+
expIPAMConfig: []network.IPAMConfig{
230+
{
231+
// FIXME(robmry) - allocatable range outside subnet.
232+
Subnet: "192.168.160.0/20",
233+
IPRange: "192.168.176.0/24",
234+
Gateway: "192.168.160.88",
235+
},
236+
},
237+
},
238+
{
239+
name: "fixed-cidr contains bridge subnet",
240+
initialBridgeAddrs: []string{"192.168.177.1/24"},
241+
daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"},
242+
expIPAMConfig: []network.IPAMConfig{
243+
{
244+
Subnet: "192.168.177.0/24",
245+
// FIXME(robmry) - allocatable range is bigger than subnet
246+
IPRange: "192.168.176.0/20",
247+
Gateway: "192.168.177.1",
248+
},
249+
},
250+
},
251+
}
252+
for _, tc := range testcases {
253+
tc.userDefinedBridge = true
254+
testDefaultBridgeIPAM(ctx, t, tc)
255+
}
256+
}
257+
258+
type defaultBridgeIPAMTestCase struct {
259+
name string
260+
userDefinedBridge bool
261+
initialBridgeAddrs []string
262+
daemonArgs []string
263+
expIPAMConfig []network.IPAMConfig
264+
}
265+
266+
func testDefaultBridgeIPAM(ctx context.Context, t *testing.T, tc defaultBridgeIPAMTestCase) {
267+
t.Run(tc.name, func(t *testing.T) {
268+
ctx := testutil.StartSpan(ctx, t)
269+
const bridgeName = "br-dbi"
270+
271+
createBridge(t, bridgeName, tc.initialBridgeAddrs)
272+
defer deleteInterface(t, bridgeName)
273+
274+
var dOpts []daemon.Option
275+
dArgs := tc.daemonArgs
276+
if tc.userDefinedBridge {
277+
// If a bridge is supplied by the user, the daemon should use its addresses
278+
// to infer --bip (which cannot be specified).
279+
dArgs = append(dArgs, []string{"--bridge", bridgeName}...)
280+
} else {
281+
// The bridge is created and managed by docker, it's always called "docker0",
282+
// unless this test-only env var is set - to avoid conflict with the docker0
283+
// belonging to the daemon started in CI runs.
284+
dOpts = append(dOpts, daemon.WithEnvVars("DOCKER_TEST_CREATE_DEFAULT_BRIDGE="+bridgeName))
285+
}
286+
287+
d := daemon.New(t, dOpts...)
288+
defer func() {
289+
d.Stop(t)
290+
d.Cleanup(t)
291+
}()
292+
d.StartWithBusybox(ctx, t, dArgs...)
293+
c := d.NewClientT(t)
294+
defer c.Close()
295+
296+
insp, err := c.NetworkInspect(ctx, network.NetworkBridge, network.InspectOptions{})
297+
assert.NilError(t, err)
298+
assert.Check(t, is.DeepEqual(insp.IPAM.Config, tc.expIPAMConfig))
299+
})
300+
}
301+
302+
func createBridge(t *testing.T, ifName string, addrs []string) {
303+
t.Helper()
304+
305+
link := &netlink.Bridge{
306+
LinkAttrs: netlink.LinkAttrs{
307+
Name: ifName,
308+
},
309+
}
310+
311+
err := netlink.LinkAdd(link)
312+
assert.NilError(t, err)
313+
for _, addr := range addrs {
314+
ip, ipNet, err := net.ParseCIDR(addr)
315+
assert.NilError(t, err)
316+
ipNet.IP = ip
317+
err = netlink.AddrAdd(link, &netlink.Addr{IPNet: ipNet})
318+
assert.NilError(t, err)
319+
}
320+
}
321+
32322
func deleteInterface(t *testing.T, ifName string) {
33323
icmd.RunCommand("ip", "link", "delete", ifName).Assert(t, icmd.Success)
34324
icmd.RunCommand("iptables", "-t", "nat", "--flush").Assert(t, icmd.Success)

0 commit comments

Comments
 (0)