diff --git a/api/types/network/endpoint.go b/api/types/network/endpoint.go index 4b3c06a52b..9edd1c38d9 100644 --- a/api/types/network/endpoint.go +++ b/api/types/network/endpoint.go @@ -14,6 +14,9 @@ type EndpointSettings struct { IPAMConfig *EndpointIPAMConfig Links []string Aliases []string // Aliases holds the list of extra, user-specified DNS names for this endpoint. + // MacAddress may be used to specify a MAC address when the container is created. + // Once the container is running, it becomes operational data (it may contain a + // generated address). MacAddress string // Operational data NetworkID string diff --git a/daemon/container_operations.go b/daemon/container_operations.go index f4ccb14fdb..e5bf1eb87d 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -445,6 +445,11 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai for name, epConfig := range endpointsConfig { container.NetworkSettings.Networks[name] = &network.EndpointSettings{ EndpointSettings: epConfig, + // At this point, during container creation, epConfig.MacAddress is the + // configured value from the API. If there is no configured value, the + // same field will later be used to store a generated MAC address. So, + // remember the requested address now. + DesiredMacAddress: epConfig.MacAddress, } } } @@ -511,7 +516,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C defaultNetName := runconfig.DefaultDaemonNetworkMode().NetworkName() if nConf, ok := container.NetworkSettings.Networks[defaultNetName]; ok { cleanOperationalData(nConf) - if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf.EndpointSettings, updateSettings); err != nil { + if err := daemon.connectToNetwork(cfg, container, defaultNetName, nConf, updateSettings); err != nil { return err } } @@ -528,7 +533,7 @@ func (daemon *Daemon) allocateNetwork(cfg *config.Config, container *container.C for netName, epConf := range networks { cleanOperationalData(epConf) - if err := daemon.connectToNetwork(cfg, container, netName, epConf.EndpointSettings, updateSettings); err != nil { + if err := daemon.connectToNetwork(cfg, container, netName, epConf, updateSettings); err != nil { return err } } @@ -637,12 +642,10 @@ func cleanOperationalData(es *network.EndpointSettings) { es.IPv6Gateway = "" es.GlobalIPv6Address = "" es.GlobalIPv6PrefixLen = 0 + es.MacAddress = "" if es.IPAMOperational { es.IPAMConfig = nil } - if es.MACOperational { - es.MacAddress = "" - } } func (daemon *Daemon) updateNetworkConfig(container *container.Container, n *libnetwork.Network, endpointConfig *networktypes.EndpointSettings, updateSettings bool) error { @@ -685,7 +688,7 @@ func buildEndpointDNSNames(ctr *container.Container, aliases []string) []string return sliceutil.Dedup(dnsNames) } -func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *networktypes.EndpointSettings, updateSettings bool) (retErr error) { +func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container.Container, idOrName string, endpointConfig *network.EndpointSettings, updateSettings bool) (retErr error) { start := time.Now() if container.HostConfig.NetworkMode.IsContainer() { return runconfig.ErrConflictSharedNetwork @@ -695,10 +698,12 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. return nil } if endpointConfig == nil { - endpointConfig = &networktypes.EndpointSettings{} + endpointConfig = &network.EndpointSettings{ + EndpointSettings: &networktypes.EndpointSettings{}, + } } - n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig) + n, nwCfg, err := daemon.findAndAttachNetwork(container, idOrName, endpointConfig.EndpointSettings) if err != nil { return err } @@ -713,26 +718,20 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. } } - var operIPAM bool - operMAC := true + endpointConfig.IPAMOperational = false if nwCfg != nil { if epConfig, ok := nwCfg.EndpointsConfig[nwName]; ok { if endpointConfig.IPAMConfig == nil || (endpointConfig.IPAMConfig.IPv4Address == "" && endpointConfig.IPAMConfig.IPv6Address == "" && len(endpointConfig.IPAMConfig.LinkLocalIPs) == 0) { - operIPAM = true + endpointConfig.IPAMOperational = true } // copy IPAMConfig and NetworkID from epConfig via AttachNetwork endpointConfig.IPAMConfig = epConfig.IPAMConfig endpointConfig.NetworkID = epConfig.NetworkID - - // Work out whether the MAC address is user-configured. - operMAC = endpointConfig.MacAddress == "" - // Copy the configured MAC address (which may be empty). - endpointConfig.MacAddress = epConfig.MacAddress } } - if err := daemon.updateNetworkConfig(container, n, endpointConfig, updateSettings); err != nil { + if err := daemon.updateNetworkConfig(container, n, endpointConfig.EndpointSettings, updateSettings); err != nil { return err } @@ -755,11 +754,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. } } }() - container.NetworkSettings.Networks[nwName] = &network.EndpointSettings{ - EndpointSettings: endpointConfig, - IPAMOperational: operIPAM, - MACOperational: operMAC, - } + container.NetworkSettings.Networks[nwName] = endpointConfig delete(container.NetworkSettings.Networks, n.ID()) @@ -1063,7 +1058,10 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName } } } else { - if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, endpointConfig, true); err != nil { + epc := &network.EndpointSettings{ + EndpointSettings: endpointConfig, + } + if err := daemon.connectToNetwork(&daemon.config().Config, container, idOrName, epc, true); err != nil { return err } } diff --git a/daemon/inspect.go b/daemon/inspect.go index 179c7264c9..db2d0f7d65 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -165,8 +165,12 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai // unversioned API endpoints. if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. if nwm := hostConfig.NetworkMode; nwm.IsDefault() || nwm.IsBridge() || nwm.IsUserDefined() { - if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok { - container.Config.MacAddress = epConf.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + name := nwm.NetworkName() + if nwm.IsDefault() { + name = daemon.netController.Config().DefaultNetwork + } + if epConf, ok := container.NetworkSettings.Networks[name]; ok { + container.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. } } } diff --git a/daemon/network.go b/daemon/network.go index c4b0c93c1d..fa58df1aed 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -788,7 +788,7 @@ func (daemon *Daemon) clearAttachableNetworks() { } // buildCreateEndpointOptions builds endpoint options from a given network. -func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *network.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) { +func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, epConfig *internalnetwork.EndpointSettings, sb *libnetwork.Sandbox, daemonDNS []string) ([]libnetwork.EndpointOption, error) { var createOptions []libnetwork.EndpointOption var genericOptions = make(options.Generic) @@ -824,8 +824,8 @@ func buildCreateEndpointOptions(c *container.Container, n *libnetwork.Network, e createOptions = append(createOptions, libnetwork.EndpointOptionGeneric(options.Generic{k: v})) } - if epConfig.MacAddress != "" { - mac, err := net.ParseMAC(epConfig.MacAddress) + if epConfig.DesiredMacAddress != "" { + mac, err := net.ParseMAC(epConfig.DesiredMacAddress) if err != nil { return nil, err } diff --git a/daemon/network/settings.go b/daemon/network/settings.go index ec44ba0e31..8236d16ac0 100644 --- a/daemon/network/settings.go +++ b/daemon/network/settings.go @@ -33,8 +33,9 @@ type Settings struct { type EndpointSettings struct { *networktypes.EndpointSettings IPAMOperational bool - // MACOperational is false if EndpointSettings.MacAddress is a user-configured value. - MACOperational bool + // DesiredMacAddress is the configured value, it's copied from MacAddress (the + // API param field) when the container is created. + DesiredMacAddress string } // AttachmentStore stores the load balancer IP address for a network id. diff --git a/integration/networking/mac_addr_test.go b/integration/networking/mac_addr_test.go index 497b4f96dc..92c24d4db4 100644 --- a/integration/networking/mac_addr_test.go +++ b/integration/networking/mac_addr_test.go @@ -4,8 +4,11 @@ import ( "testing" containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/network" + "github.com/docker/docker/libnetwork/drivers/bridge" + "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -35,7 +38,7 @@ func TestMACAddrOnRestart(t *testing.T) { const netName = "testmacaddrs" network.CreateNoError(ctx, t, c, netName, network.WithDriver("bridge"), - network.WithOption("com.docker.network.bridge.name", netName)) + network.WithOption(bridge.BridgeName, netName)) defer network.RemoveNoError(ctx, t, c, netName) const ctr1Name = "ctr1" @@ -77,3 +80,154 @@ func TestMACAddrOnRestart(t *testing.T) { assert.Check(t, ctr1MAC != ctr2MAC, "expected containers to have different MAC addresses; got %q for both", ctr1MAC) } + +// Check that a configured MAC address is restored after a container restart, +// and after a daemon restart. +func TestCfgdMACAddrOnRestart(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + + ctx := setupTest(t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + const netName = "testcfgmacaddr" + network.CreateNoError(ctx, t, c, netName, + network.WithDriver("bridge"), + network.WithOption(bridge.BridgeName, netName)) + defer network.RemoveNoError(ctx, t, c, netName) + + const wantMAC = "02:42:ac:11:00:42" + const ctr1Name = "ctr1" + id1 := container.Run(ctx, t, c, + container.WithName(ctr1Name), + container.WithImage("busybox:latest"), + container.WithCmd("top"), + container.WithNetworkMode(netName), + container.WithMacAddress(netName, wantMAC)) + defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{ + Force: true, + }) + + inspect := container.Inspect(ctx, t, c, ctr1Name) + gotMAC := inspect.NetworkSettings.Networks[netName].MacAddress + assert.Check(t, is.Equal(wantMAC, gotMAC)) + + startAndCheck := func() { + t.Helper() + err := c.ContainerStart(ctx, ctr1Name, containertypes.StartOptions{}) + assert.Assert(t, is.Nil(err)) + inspect = container.Inspect(ctx, t, c, ctr1Name) + gotMAC = inspect.NetworkSettings.Networks[netName].MacAddress + assert.Check(t, is.Equal(wantMAC, gotMAC)) + } + + // Restart the container, check that the MAC address is restored. + err := c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{}) + assert.Assert(t, is.Nil(err)) + startAndCheck() + + // Restart the daemon, check that the MAC address is restored. + err = c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{}) + assert.Assert(t, is.Nil(err)) + d.Restart(t) + startAndCheck() +} + +// Regression test for https://github.com/moby/moby/issues/47228 - check that a +// generated MAC address is not included in the Config section of 'inspect' +// output, but a configured address is. +func TestInspectCfgdMAC(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows") + + ctx := setupTest(t) + + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + testcases := []struct { + name string + desiredMAC string + netName string + ctrWide bool + }{ + { + name: "generated address default bridge", + netName: "bridge", + }, + { + name: "configured address default bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "bridge", + }, + { + name: "generated address custom bridge", + netName: "testnet", + }, + { + name: "configured address custom bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "testnet", + }, + { + name: "ctr-wide address default bridge", + desiredMAC: "02:42:ac:11:00:42", + netName: "bridge", + ctrWide: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.StartSpan(ctx, t) + + var copts []client.Opt + if tc.ctrWide { + copts = append(copts, client.WithVersion("1.43")) + } + c := d.NewClientT(t, copts...) + defer c.Close() + + if tc.netName != "bridge" { + const netName = "inspectcfgmac" + network.CreateNoError(ctx, t, c, netName, + network.WithDriver("bridge"), + network.WithOption(bridge.BridgeName, netName)) + defer network.RemoveNoError(ctx, t, c, netName) + } + + const ctrName = "ctr" + opts := []func(*container.TestContainerConfig){ + container.WithName(ctrName), + container.WithCmd("top"), + container.WithImage("busybox:latest"), + } + // Don't specify the network name for the bridge network, because that + // exercises a different code path (the network name isn't set until the + // container starts, until then it's "default"). + if tc.netName != "bridge" { + opts = append(opts, container.WithNetworkMode(tc.netName)) + } + if tc.desiredMAC != "" { + if tc.ctrWide { + opts = append(opts, container.WithContainerWideMacAddress(tc.desiredMAC)) + } else { + opts = append(opts, container.WithMacAddress(tc.netName, tc.desiredMAC)) + } + } + id := container.Create(ctx, t, c, opts...) + defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{ + Force: true, + }) + + inspect := container.Inspect(ctx, t, c, ctrName) + configMAC := inspect.Config.MacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + assert.Check(t, is.DeepEqual(configMAC, tc.desiredMAC)) + }) + } +}