Repository: moby/moby (language: go)

Issue:
MAC address conflict since v25
### Description

I understand that, unless --mac-address is specified when calling `docker run` that a MAC address will be generated on container start with the prefix `02:42:ac` and then the last 3 components being the container's ipv4 address in hex notation.

Since v25.0.0 (using the official docker-ce Ubuntu package for 22.04) it seems that "docker start" retains the previous MAC address even when the IP is changed and that this can cause a MAC address conflict.

This behaviour does not occur on 24.x.

### Reproduce

docker network create test-network
docker run -d --name server1 --network test-network httpd
docker stop server1
docker run -d --name server2 --network test-network httpd
docker start server1

Now if you “docker inspect” for server1 and server2 they will have the same MAC address but a different IP.  Connectivity will be intermittent for both containers.


### Expected behavior

Starting a container should assign a unique MAC address if no MAC address was specified when the container was created.

### docker version

```bash
Client: Docker Engine - Community
 Version:           25.0.0
 API version:       1.44
 Go version:        go1.21.6
 Git commit:        e758fe5
 Built:             Thu Jan 18 17:09:49 2024
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          25.0.0
  API version:      1.44 (minimum version 1.24)
  Go version:       go1.21.6
  Git commit:       615dfdf
  Built:            Thu Jan 18 17:09:49 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.27
  GitCommit:        a1496014c916f9e62104b33d1bb5bd03b0858e59
 runc:
  Version:          1.1.11
  GitCommit:        v1.1.11-0-g4bccb38
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0
```


### docker info

```bash
Client: Docker Engine - Community
 Version:           25.0.0
 API version:       1.44
 Go version:        go1.21.6
 Git commit:        e758fe5
 Built:             Thu Jan 18 17:09:49 2024
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          25.0.0
  API version:      1.44 (minimum version 1.24)
  Go version:       go1.21.6
  Git commit:       615dfdf
  Built:            Thu Jan 18 17:09:49 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.27
  GitCommit:        a1496014c916f9e62104b33d1bb5bd03b0858e59
 runc:
  Version:          1.1.11
  GitCommit:        v1.1.11-0-g4bccb38
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0
root@containering:~# docker info
Client: Docker Engine - Community
 Version:    25.0.0
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.12.1
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.24.1
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 2
  Running: 0
  Paused: 0
  Stopped: 2
 Images: 1
 Server Version: 25.0.0
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: systemd
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: a1496014c916f9e62104b33d1bb5bd03b0858e59
 runc version: v1.1.11-0-g4bccb38
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 5.15.0-91-generic
 Operating System: Ubuntu 22.04.3 LTS
 OSType: linux
 Architecture: x86_64
 CPUs: 1
 Total Memory: 1.918GiB
 Name: containering
 ID: 172a9933-7614-4ae1-a496-3f40c37b45c7
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false
```


### Additional Info

_No response_


Discussion / hints:

Fix patch:
diff --git a/daemon/container_operations.go b/daemon/container_operations.go
index ac366c08d2f42..db4f291770031 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 c31b139ae8dcf..ec44ba0e31370 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.


Test patch:
diff --git a/integration/networking/mac_addr_test.go b/integration/networking/mac_addr_test.go
new file mode 100644
index 0000000000000..497b4f96dc7f3
--- /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)
+}