diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 0a3fc2af5e..5df9640bb9 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -637,6 +637,9 @@ func cleanOperationalData(es *network.EndpointSettings) { 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 { @@ -708,6 +711,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. } var operIPAM bool + operMAC := true 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) { @@ -717,6 +721,11 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. // 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 } } @@ -746,6 +755,7 @@ func (daemon *Daemon) connectToNetwork(cfg *config.Config, container *container. container.NetworkSettings.Networks[nwName] = &network.EndpointSettings{ EndpointSettings: endpointConfig, IPAMOperational: operIPAM, + MACOperational: operMAC, } delete(container.NetworkSettings.Networks, n.ID()) diff --git a/daemon/network/settings.go b/daemon/network/settings.go index c31b139ae8..ec44ba0e31 100644 --- a/daemon/network/settings.go +++ b/daemon/network/settings.go @@ -33,6 +33,8 @@ type Settings struct { type EndpointSettings struct { *networktypes.EndpointSettings IPAMOperational bool + // MACOperational is false if EndpointSettings.MacAddress is a user-configured value. + MACOperational bool } // 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 new file mode 100644 index 0000000000..497b4f96dc --- /dev/null +++ b/integration/networking/mac_addr_test.go @@ -0,0 +1,79 @@ +package networking + +import ( + "testing" + + containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/integration/internal/container" + "github.com/docker/docker/integration/internal/network" + "github.com/docker/docker/testutil/daemon" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/skip" +) + +// TestMACAddrOnRestart is a regression test for https://github.com/moby/moby/issues/47146 +// - Start a container, let it use a generated MAC address. +// - Stop that container. +// - Start a second container, it'll also use a generated MAC address. +// (It's likely to recycle the first container's MAC address.) +// - Restart the first container. +// (The bug was that it kept its original MAC address, now already in-use.) +// - Check that the two containers have different MAC addresses. +func TestMACAddrOnRestart(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 = "testmacaddrs" + network.CreateNoError(ctx, t, c, netName, + network.WithDriver("bridge"), + network.WithOption("com.docker.network.bridge.name", netName)) + defer network.RemoveNoError(ctx, t, c, netName) + + const ctr1Name = "ctr1" + id1 := container.Run(ctx, t, c, + container.WithName(ctr1Name), + container.WithImage("busybox:latest"), + container.WithCmd("top"), + container.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, id1, containertypes.RemoveOptions{ + Force: true, + }) + err := c.ContainerStop(ctx, ctr1Name, containertypes.StopOptions{}) + assert.Assert(t, is.Nil(err)) + + // Start a second container, giving the daemon a chance to recycle the first container's + // IP and MAC addresses. + const ctr2Name = "ctr2" + id2 := container.Run(ctx, t, c, + container.WithName(ctr2Name), + container.WithImage("busybox:latest"), + container.WithCmd("top"), + container.WithNetworkMode(netName)) + defer c.ContainerRemove(ctx, id2, containertypes.RemoveOptions{ + Force: true, + }) + + // Restart the first container. + err = c.ContainerStart(ctx, ctr1Name, containertypes.StartOptions{}) + assert.Assert(t, is.Nil(err)) + + // Check that the containers ended up with different MAC addresses. + + ctr1Inspect := container.Inspect(ctx, t, c, ctr1Name) + ctr1MAC := ctr1Inspect.NetworkSettings.Networks[netName].MacAddress + + ctr2Inspect := container.Inspect(ctx, t, c, ctr2Name) + ctr2MAC := ctr2Inspect.NetworkSettings.Networks[netName].MacAddress + + assert.Check(t, ctr1MAC != ctr2MAC, + "expected containers to have different MAC addresses; got %q for both", ctr1MAC) +}