Merge pull request #47168 from robmry/47146-duplicate_mac_addrs

Remove generated MAC addresses on restart.

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())

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.

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)
+}