diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 31a1d3f47d..011739f6d9 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -927,10 +927,7 @@ func driverOptions(config *config.Config) nwconfig.Option { } func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig) error { - bridgeName := bridge.DefaultBridgeName - if cfg.Iface != "" { - bridgeName = cfg.Iface - } + bridgeName, userManagedBridge := getDefaultBridgeName(cfg) netOption := map[string]string{ bridge.BridgeName: bridgeName, bridge.DefaultBridge: strconv.FormatBool(true), @@ -938,7 +935,6 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig bridge.EnableIPMasquerade: strconv.FormatBool(cfg.EnableIPMasq), bridge.EnableICC: strconv.FormatBool(cfg.InterContainerCommunication), } - // --ip processing if cfg.DefaultIP != nil { netOption[bridge.DefaultBindingIP] = cfg.DefaultIP.String() @@ -986,7 +982,7 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig } ipamV4Conf.PreferredPool = ipNet.String() ipamV4Conf.Gateway = ip.String() - } else if bridgeName == bridge.DefaultBridgeName && ipamV4Conf.PreferredPool != "" { + } else if !userManagedBridge && ipamV4Conf.PreferredPool != "" { 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) } @@ -1069,6 +1065,28 @@ func initBridgeDriver(controller *libnetwork.Controller, cfg config.BridgeConfig return nil } +func getDefaultBridgeName(cfg config.BridgeConfig) (bridgeName string, userManagedBridge bool) { + // cfg.Iface is --bridge, the option to supply a user-managed bridge. + if cfg.Iface != "" { + // The default network will use a user-managed bridge, the daemon will not + // create it, and it is not possible to supply an address using --bip. + return cfg.Iface, true + } + // Without a --bridge, the bridge is "docker0", created and managed by the + // daemon. A --bip (cidr) can be supplied to define the bridge's IP address + // and the network's subnet. + // + // Env var DOCKER_TEST_CREATE_DEFAULT_BRIDGE env var modifies the default + // bridge name. Unlike '--bridge', the bridge does not need to be created + // outside the daemon, and it's still possible to use '--bip'. It is + // intended only for use in moby tests; it may be removed, its behaviour + // may be modified, and it may not do what you want anyway! + if bn := os.Getenv("DOCKER_TEST_CREATE_DEFAULT_BRIDGE"); bn != "" { + return bn, false + } + return bridge.DefaultBridgeName, false +} + // Remove default bridge interface if present (--bridge=none use case) func removeDefaultBridgeInterface() { if lnk, err := nlwrap.LinkByName(bridge.DefaultBridgeName); err == nil { diff --git a/integration/daemon/daemon_linux_test.go b/integration/daemon/daemon_linux_test.go index 591cba531d..6406cfc1d9 100644 --- a/integration/daemon/daemon_linux_test.go +++ b/integration/daemon/daemon_linux_test.go @@ -1,11 +1,18 @@ package daemon // import "github.com/docker/docker/integration/daemon" import ( + "context" + "net" "testing" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" + "github.com/vishvananda/netlink" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" + "gotest.tools/v3/skip" ) func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) { @@ -29,6 +36,289 @@ func TestDaemonDefaultBridgeWithFixedCidrButNoBip(t *testing.T) { d.StartWithBusybox(ctx, t, "--bridge", bridgeName, "--fixed-cidr", "192.168.130.0/24") } +// Test fixed-cidr and bip options, with various addresses on the bridge +// before the daemon starts. +func TestDaemonDefaultBridgeIPAM_Docker0(t *testing.T) { + skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace") + ctx := testutil.StartSpan(baseContext, t) + + testcases := []defaultBridgeIPAMTestCase{ + { + name: "fixed-cidr only", + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + IPRange: "192.168.176.0/24", + }, + }, + }, + { + name: "bip only", + daemonArgs: []string{"--bip", "192.168.176.88/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "existing bridge address only", + initialBridgeAddrs: []string{"192.168.176.88/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "fixed-cidr within old bridge subnet", + initialBridgeAddrs: []string{"192.168.176.88/20"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"}, + // There's no --bip to dictate the subnet, so it's derived from an + // existing bridge address. If fixed-cidr's subnet is made smaller + // following a daemon restart, a user might reasonably expect the + // default bridge network's subnet to shrink to match. However, + // that has not been the behaviour - instead, only the allocatable + // range is reduced (as would happen with a user-managed bridge). + // In this case, if the user wants a smaller subnet, their options + // are to delete docker0, or supply a --bip. A change in this subtle + // behaviour might be best. But, it's probably not causing problems, + // and it'd be a breaking change for anyone relying on the existing + // behaviour. + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/20", + IPRange: "192.168.176.0/24", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "fixed-cidr within old bridge subnet with new bip", + initialBridgeAddrs: []string{"192.168.176.88/20"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24", "--bip", "192.168.176.99/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + IPRange: "192.168.176.0/24", + Gateway: "192.168.176.99", + }, + }, + }, + { + name: "old bridge subnet within fixed-cidr", + initialBridgeAddrs: []string{"192.168.176.88/24"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"}, + expIPAMConfig: []network.IPAMConfig{ + { + // FIXME(robmry) - subnet didn't change, allocatable range + // is bigger than the subnet. + Subnet: "192.168.176.0/24", + IPRange: "192.168.176.0/20", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "old bridge subnet outside fixed-cidr", + initialBridgeAddrs: []string{"192.168.176.88/24"}, + daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + // FIXME(robmry) - subnet and bridge address haven't changed, + // and the allocatable range is outside the subnet. + Subnet: "192.168.176.0/24", + IPRange: "192.168.177.0/24", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "old bridge subnet outside fixed-cidr with bip", + initialBridgeAddrs: []string{"192.168.176.88/24"}, + daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24", "--bip", "192.168.177.99/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.177.0/24", + IPRange: "192.168.177.0/24", + Gateway: "192.168.177.99", + }, + }, + }, + } + for _, tc := range testcases { + testDefaultBridgeIPAM(ctx, t, tc) + } +} + +// Like TestDaemonUserDefaultBridgeIPAMDocker0, but with a user-defined/supplied +// bridge, instead of docker0. +func TestDaemonDefaultBridgeIPAM_UserBr(t *testing.T) { + skip.If(t, testEnv.IsRootless, "can't create test bridge in rootless namespace") + ctx := testutil.StartSpan(baseContext, t) + + testcases := []defaultBridgeIPAMTestCase{ + { + name: "bridge only", + initialBridgeAddrs: []string{"192.168.176.88/20"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/20", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "fixed-cidr only", + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + IPRange: "192.168.176.0/24", + }, + }, + }, + { + name: "fcidr in bridge subnet and bridge ip in fcidr", + initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"}, + // Selected bip should be the one within fixed-cidr + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/20", + IPRange: "192.168.176.0/24", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "fcidr in bridge subnet and bridge ip not in fcidr", + initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.176.88/20", "192.168.192.88/20"}, + daemonArgs: []string{"--fixed-cidr", "192.168.177.0/24"}, + expIPAMConfig: []network.IPAMConfig{ + { + // FIXME(robmry) - selected subnet should be the one that encompasses + // fixed-cidr, allocatable range is outside the subnet. + Subnet: "192.168.160.0/20", + IPRange: "192.168.177.0/24", + Gateway: "192.168.160.88", + }, + }, + }, + { + name: "fixed-cidr bigger than bridge subnet", + initialBridgeAddrs: []string{"192.168.176.88/24"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.176.0/24", + // FIXME(robmry) - allocatable range is bigger than the subnet. + IPRange: "192.168.176.0/20", + Gateway: "192.168.176.88", + }, + }, + }, + { + name: "no bridge ip within fixed-cidr", + initialBridgeAddrs: []string{"192.168.160.88/20", "192.168.192.88/20"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/24"}, + // Selected bip should be the one within fixed-cidr + expIPAMConfig: []network.IPAMConfig{ + { + // FIXME(robmry) - allocatable range outside subnet. + Subnet: "192.168.160.0/20", + IPRange: "192.168.176.0/24", + Gateway: "192.168.160.88", + }, + }, + }, + { + name: "fixed-cidr contains bridge subnet", + initialBridgeAddrs: []string{"192.168.177.1/24"}, + daemonArgs: []string{"--fixed-cidr", "192.168.176.0/20"}, + expIPAMConfig: []network.IPAMConfig{ + { + Subnet: "192.168.177.0/24", + // FIXME(robmry) - allocatable range is bigger than subnet + IPRange: "192.168.176.0/20", + Gateway: "192.168.177.1", + }, + }, + }, + } + for _, tc := range testcases { + tc.userDefinedBridge = true + testDefaultBridgeIPAM(ctx, t, tc) + } +} + +type defaultBridgeIPAMTestCase struct { + name string + userDefinedBridge bool + initialBridgeAddrs []string + daemonArgs []string + expIPAMConfig []network.IPAMConfig +} + +func testDefaultBridgeIPAM(ctx context.Context, t *testing.T, tc defaultBridgeIPAMTestCase) { + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + const bridgeName = "br-dbi" + + createBridge(t, bridgeName, tc.initialBridgeAddrs) + defer deleteInterface(t, bridgeName) + + var dOpts []daemon.Option + dArgs := tc.daemonArgs + if tc.userDefinedBridge { + // If a bridge is supplied by the user, the daemon should use its addresses + // to infer --bip (which cannot be specified). + dArgs = append(dArgs, []string{"--bridge", bridgeName}...) + } else { + // The bridge is created and managed by docker, it's always called "docker0", + // unless this test-only env var is set - to avoid conflict with the docker0 + // belonging to the daemon started in CI runs. + dOpts = append(dOpts, daemon.WithEnvVars("DOCKER_TEST_CREATE_DEFAULT_BRIDGE="+bridgeName)) + } + + d := daemon.New(t, dOpts...) + defer func() { + d.Stop(t) + d.Cleanup(t) + }() + d.StartWithBusybox(ctx, t, dArgs...) + c := d.NewClientT(t) + defer c.Close() + + insp, err := c.NetworkInspect(ctx, network.NetworkBridge, network.InspectOptions{}) + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(insp.IPAM.Config, tc.expIPAMConfig)) + }) +} + +func createBridge(t *testing.T, ifName string, addrs []string) { + t.Helper() + + link := &netlink.Bridge{ + LinkAttrs: netlink.LinkAttrs{ + Name: ifName, + }, + } + + err := netlink.LinkAdd(link) + assert.NilError(t, err) + for _, addr := range addrs { + ip, ipNet, err := net.ParseCIDR(addr) + assert.NilError(t, err) + ipNet.IP = ip + err = netlink.AddrAdd(link, &netlink.Addr{IPNet: ipNet}) + assert.NilError(t, err) + } +} + func deleteInterface(t *testing.T, ifName string) { icmd.RunCommand("ip", "link", "delete", ifName).Assert(t, icmd.Success) icmd.RunCommand("iptables", "-t", "nat", "--flush").Assert(t, icmd.Success)