api/types/container: remove deprecated Config.MacAddress by thaJeztah · Pull Request #51194 · moby/moby

thaJeztah · 2025-10-15T14:36:58Z

relates to api/t/ctr: drop NetworkSettingsBase, DefaultNetworkSettings #50846
depends on / stacked on daemon/server/router/container: fix back-filling of top-level network fields #51201

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2025-10-15T15:12:01Z

Hm... weird; I tried this locally and it worked 🤔 let me double-check what I missed;

=== RUN   TestInspectCfgdMAC/configured_address_default_bridge
    mac_addr_test.go:249: assertion failed: 
        --- configMAC
        +++ tc.desiredMAC
          string(
        - 	"",
        + 	"02:42:ac:11:00:42",
          )

thaJeztah · 2025-10-15T17:28:34Z

integration/networking/mac_addr_test.go

 			if tc.ctrWide {
 				copts = append(copts, client.WithVersion("1.43"))
+			} else {
+				copts = append(copts, client.WithVersion("1.51"))


This one caused me some head-scratching; this test depended on the daemon.getInspectData unconditionally doing the conversion, which broke when I moved that code and versioned it 😅

thaJeztah · 2025-10-15T17:30:31Z

daemon/server/router/container/inspect.go

-		}))
+		var macAddress string
+		if bridgeNw := ctr.NetworkSettings.Networks["bridge"]; bridgeNw != nil {
+			macAddress = bridgeNw.MacAddress


This was the other bit;

If there's a default "bridge" network attached, we copy various fields to the top-level, including the MacAddress

BUT if there's a custom network set as default (not the bridge) then the bridge network's fields are still copied, BUT the MacAddress must be taken from the default network (well, apparently)

I've spent far too long trying to untangle this, and haven't really got there - the breakage doesn't all come from this PR, but it is broken ...

Here's an inspect response from 28.x, API 1.50 ...

{ ... "Config": { "Hostname": "b4640dbaf62b", "Domainname": "", "User": "", "AttachStdin": false, "AttachStdout": false, "AttachStderr": false, "Tty": false, "OpenStdin": false, "StdinOnce": false, "Env": [ "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ], "Cmd": [ "sleep", "infinity" ], "Image": "alpine", "Volumes": null, "WorkingDir": "/", "Entrypoint": null, "OnBuild": null, "Labels": {} }, "HostConfig": { "NetworkMode": "n1", ... }, "NetworkSettings": { "Bridge": "", "SandboxID": "2c222056a6712aba736ba3f6ef49f93cfcd67b18c3c6840d9557983635839ac8", "SandboxKey": "/var/run/docker/netns/2c222056a671", "Ports": {}, "HairpinMode": false, "LinkLocalIPv6Address": "", "LinkLocalIPv6PrefixLen": 0, "SecondaryIPAddresses": null, "SecondaryIPv6Addresses": null, "EndpointID": "df0670107fae052c969fde3c2b9d3258e16c8bbd2607dd19ae2ee0d63e6e16a6", "Gateway": "172.18.0.1", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "IPAddress": "172.18.0.2", ******* DEFAULT BRIDGE "IPPrefixLen": 16, "IPv6Gateway": "", "MacAddress": "7e:1c:d4:dc:e9:13", ******** DEFAULT BRIDGE "Networks": { "bridge": { "IPAMConfig": {}, "Links": null, "Aliases": [], "MacAddress": "7e:1c:d4:dc:e9:13", "DriverOpts": {}, "GwPriority": 0, "NetworkID": "f7fe034b75caddf766239ff5f3a6a3f586101062d5e452c502d9e67661f19acc", "EndpointID": "df0670107fae052c969fde3c2b9d3258e16c8bbd2607dd19ae2ee0d63e6e16a6", "Gateway": "172.18.0.1", "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "DNSNames": null }, "n1": { "IPAMConfig": null, "Links": null, "Aliases": null, "MacAddress": "02:94:8f:8c:04:08", "DriverOpts": null, "GwPriority": 0, "NetworkID": "691b073f12fb793c495c3cfa99ae126b7262679158afd320e07e9eead34d7424", "EndpointID": "38a62db5028cab05d2ec9e0139f091250dff2fabd5e9a123d079d786986b50ac", "Gateway": "172.19.0.1", "IPAddress": "172.19.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "DNSNames": [ "c1", "b4640dbaf62b" ] } } } }

And here's the response from this PR ...

{ ... "Config": { "AttachStderr": false, "AttachStdin": false, "AttachStdout": false, "Cmd": [ "sleep", "infinity" ], "Domainname": "", "Entrypoint": null, "Env": [ "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ], "Hostname": "00baebcc73c4", "Image": "alpine", "Labels": {}, "MacAddress": "2a:6f:37:eb:f2:cb", ********** Wrong network's MAC, in the wrong place "OpenStdin": false, "StdinOnce": false, "Tty": false, "User": "", "Volumes": null, "WorkingDir": "/" }, "HostConfig": { "NetworkMode": "n1", ... }, "NetworkSettings": { *************** no top level config here "Networks": { "bridge": { "Aliases": [], "DNSNames": null, "DriverOpts": {}, "EndpointID": "98deb035fe1426db437fb5b2d74a419792cd9017c7870df596beab17a8cd0807", "Gateway": "172.18.0.1", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "GwPriority": 0, "IPAMConfig": { "IPv4Address": "", "IPv6Address": "" }, "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "Links": null, "MacAddress": "ba:87:1a:84:98:ac", "NetworkID": "1dcb20acf3eaf27c116edd982ef662a92f2bee5d69afd9ba48b2401725db57fe" }, "n1": { "Aliases": null, "DNSNames": [ "c1", "00baebcc73c4" ], "DriverOpts": null, "EndpointID": "e6c962f3e6477c04f1ed89ccc602f965e1b53ccdb930bff6f6b9f1bf7258c325", "Gateway": "172.19.0.1", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "GwPriority": 0, "IPAMConfig": null, "IPAddress": "172.19.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "Links": null, "MacAddress": "2a:6f:37:eb:f2:cb", "NetworkID": "f4ecbf30358a0ffa96e238f59c40d2848f6c0cb6c183131e7cdc08f815966cc8" } }, "Ports": {}, "SandboxID": "238841bd84f331e1cffab6a0767cd9c3ce143f04c8cd006e93e01f7faf6cb83b", "SandboxKey": "/var/run/docker/netns/238841bd84f3" }, ... }

Hm... nice one so the MacAddress merging into Config is .. I think intentional; at least (but we can check) I think that’s where an oid client would put it (Container.Config.MacAddress), and we’re back-filling it here

For the other top-level fields; so IIUC, we’re currently returning them at the top-top-top level, but it should’ve been inside NetworkSettings ? 🤔

I just tried this on a 20.10 daemon;

DOCKER_API_VERSION=1.24 docker run -d --mac-address=7e:1c:d4:dc:e9:13 nginx:alpine

Then inspect (removed some redundant output);

{ "Id": "af5d57adab4563ff23da305ebce146936a3a2f562868a8e27cbd046ffaa08f5c", "Created": "2025-10-16T10:07:27.498593429Z", "Path": "/docker-entrypoint.sh", "Args": ["nginx", "-g", "daemon off;"], "Config": { "Hostname": "af5d57adab45", "Domainname": "", "User": "", "AttachStdin": false, "AttachStdout": false, "AttachStderr": false, "ExposedPorts": { "80/tcp": {} }, "Tty": false, "OpenStdin": false, "StdinOnce": false, "Env": ["..."], "Cmd": ["nginx", "-g", "daemon off;"], "Image": "nginx:alpine", "Volumes": null, "WorkingDir": "/", "Entrypoint": ["/docker-entrypoint.sh"], "MacAddress": "7e:1c:d4:dc:e9:13", "OnBuild": null, "Labels": {}, "StopSignal": "SIGQUIT" }, "NetworkSettings": { "Bridge": "", "SandboxID": "d2dd8cb7b05b615839b6be0a180bd981a23d30766f0c71919fe99f459040a55e", "HairpinMode": false, "LinkLocalIPv6Address": "", "LinkLocalIPv6PrefixLen": 0, "Ports": { "80/tcp": null }, "SandboxKey": "/var/run/docker/netns/d2dd8cb7b05b", "SecondaryIPAddresses": null, "SecondaryIPv6Addresses": null, "EndpointID": "65ef5c720f24c2a4ed03356dca08170b95303f1c94eedff621e2fe7d0da298ba", "Gateway": "172.18.0.1", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "MacAddress": "7e:1c:d4:dc:e9:13", "Networks": { "bridge": { "IPAMConfig": null, "Links": null, "Aliases": null, "NetworkID": "01b896f853f82c94c0a52ad17ca8d3a2cd342290291d0f368a1d6455912682b4", "EndpointID": "65ef5c720f24c2a4ed03356dca08170b95303f1c94eedff621e2fe7d0da298ba", "Gateway": "172.18.0.1", "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "7e:1c:d4:dc:e9:13", "DriverOpts": null } } } }

And with a custom network;

docker network create mynetwork DOCKER_API_VERSION=1.24 docker run -d --network mynetwork --mac-address=7e:1c:d4:dc:e9:13 nginx:alpine

For these;

"IPAddress": "172.18.0.2", ******* DEFAULT BRIDGE "IPPrefixLen": 16, "IPv6Gateway": "", "MacAddress": "7e:1c:d4:dc:e9:13", ******** DEFAULT BRIDGE

And .. indeed, it looks like ONLY the default bridge's properties are moved to the top-level NetworkSettings.MacAddress

{ "Id": "44bf0d822f80ff31bc263c62322604e619ead13342ff9711ecd2e8b6c38caaed", "Created": "2025-10-16T10:17:38.996314462Z", "Path": "/docker-entrypoint.sh", "Args": ["nginx", "-g", "daemon off;"], "Config": { "Hostname": "44bf0d822f80", "Domainname": "", "User": "", "AttachStdin": false, "AttachStdout": false, "AttachStderr": false, "ExposedPorts": { "80/tcp": {} }, "Tty": false, "OpenStdin": false, "StdinOnce": false, "Env": ["..."], "Cmd": ["nginx", "-g", "daemon off;"], "Image": "nginx:alpine", "Volumes": null, "WorkingDir": "/", "Entrypoint": ["/docker-entrypoint.sh"], "MacAddress": "7e:1c:d4:dc:e9:13", "OnBuild": null, "Labels": {}, "StopSignal": "SIGQUIT" }, "NetworkSettings": { "Bridge": "", "SandboxID": "cd77c875551b787d0c2a85620aa703fd7b3c2eabbdadd18bf08e2195a763c267", "HairpinMode": false, "LinkLocalIPv6Address": "", "LinkLocalIPv6PrefixLen": 0, "Ports": { "80/tcp": null }, "SandboxKey": "/var/run/docker/netns/cd77c875551b", "SecondaryIPAddresses": null, "SecondaryIPv6Addresses": null, "EndpointID": "", "Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "IPAddress": "", "IPPrefixLen": 0, "IPv6Gateway": "", "MacAddress": "", "Networks": { "mynetwork": { "IPAMConfig": null, "Links": null, "Aliases": [ "44bf0d822f80" ], "NetworkID": "8aa88ce187300910e941a18ce0708c7ad91313d3050ef94a0464d5592713e2be", "EndpointID": "4ba4a6f1f043cb46008183fbc73dc4abebae7722e7655c58a1eaca0bc5a2c1e2", "Gateway": "172.19.0.1", "IPAddress": "172.19.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "7e:1c:d4:dc:e9:13", "DriverOpts": null } } } }

On docker 26.0;

docker run -d --mac-address=7e:1c:d4:dc:e9:13 nginx:alpine

Container.Config has the MacAddress of the bridge network if none is specified

DOCKER_API_VERSION=1.24 docker inspect cbb597cee5cf29512dd30107b158c8f148f9f015aff4e7791d673fa37 | jq .[].Config.MacAddress "7e:1c:d4:dc:e9:13"

and networksettings top-level fields same;

DOCKER_API_VERSION=1.24 docker inspect cbb597cee5cf29512dd30107b158c8f148f9f015aff4e7791d673fa37 | jq .[].NetworkSettings

{ "Bridge": "", "SandboxID": "40ebfb7d6395e9de268274a45f1bfd43339e9625b479285a579d57ff071c39a9", "SandboxKey": "/var/run/docker/netns/40ebfb7d6395", "Ports": { "80/tcp": null }, "HairpinMode": false, "LinkLocalIPv6Address": "", "LinkLocalIPv6PrefixLen": 0, "SecondaryIPAddresses": null, "SecondaryIPv6Addresses": null, "EndpointID": "64f3e1a4dd9c8647739df1156be9e5d3cefacd35b0ba02366e0a40467908a46f", "Gateway": "172.18.0.1", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "MacAddress": "7e:1c:d4:dc:e9:13", "Networks": { "bridge": { "IPAMConfig": null, "Links": null, "Aliases": null, "MacAddress": "7e:1c:d4:dc:e9:13", "NetworkID": "51094f5d3e83752f3d438532b9f6716d1154f270a3348eed4fb04637971c2226", "EndpointID": "64f3e1a4dd9c8647739df1156be9e5d3cefacd35b0ba02366e0a40467908a46f", "Gateway": "172.18.0.1", "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "DriverOpts": null, "DNSNames": null } } }

With a custom network;

docker network create mynetwork docker run -d --network=mynetwork --mac-address=7e:1c:d4:dc:e9:13 nginx:alpine

The Config has the MAC address of the custom network;

DOCKER_API_VERSION=1.24 docker inspect dbbb5ff499502f55bf078d81054955b2583defed949fb7c84e12fc62dee1bc1a | jq .[].Config.MacAddress "7e:1c:d4:dc:e9:13"

But NetworkSettings top-level reflects the default bridge (which isn't used);

DOCKER_API_VERSION=1.24 docker inspect dbbb5ff499502f55bf078d81054955b2583defed949fb7c84e12fc62dee1bc1a | jq .[].NetworkSettings

{ "Bridge": "", "SandboxID": "dbde9074751f57cdbaa6c098e9fe2bfcde087b05d17d3e7343a2b3d9c92a45a7", "SandboxKey": "/var/run/docker/netns/dbde9074751f", "Ports": { "80/tcp": null }, "HairpinMode": false, "LinkLocalIPv6Address": "", "LinkLocalIPv6PrefixLen": 0, "SecondaryIPAddresses": null, "SecondaryIPv6Addresses": null, "EndpointID": "", "Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "IPAddress": "", "IPPrefixLen": 0, "IPv6Gateway": "", "MacAddress": "", "Networks": { "mynetwork": { "IPAMConfig": null, "Links": null, "Aliases": [ "dbbb5ff49950" ], "MacAddress": "7e:1c:d4:dc:e9:13", "NetworkID": "b2a0183d5d4c0d93be62a539af0cb347bc6185ff3671a601785fe90107915fac", "EndpointID": "55ed1946ff40b72e2de65645c8b8cb17eb171df031921c083dbc9b4d5833259d", "Gateway": "172.19.0.1", "IPAddress": "172.19.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "DriverOpts": null, "DNSNames": [ "infallible_goldwasser", "dbbb5ff49950" ] } } }

thaJeztah · 2025-10-15T18:54:15Z

This needs a rebase but should be ready for review

austinvazquez

Should remove it from the swagger as well?

austinvazquez · 2025-10-15T19:43:20Z

daemon/server/router/container/container_routes.go

+	// Use a tee-reader to allow reading the body for legacy fields.
+	var requestBody bytes.Buffer
+	rdr := io.TeeReader(r.Body, &requestBody)


Damn.. that's clever and simpler than marshaling into a backend type. :)

Yes; this one was more complicated than it .. had to be (probably) because the container-decoder living elsewhere. Adding some fields to a backend type wouldn't have been the worst, but for this one it's more tricky also because the type may be used to persist things on disk, and .. ultimately we only need it in the API endpoint to convert back <--> forth.

austinvazquez · 2025-10-15T20:00:05Z

Should remove it from the swagger as well?

Ignore me. I missed this got merged already. #51189

thaJeztah · 2025-10-15T20:15:48Z

integration/container/run_linux_test.go

+type legacyCreateRequest struct {
+	containertypes.CreateRequest
+	// Mac Address of the container.
+	//
+	// MacAddress field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead.
+	MacAddress string `json:",omitempty"`
+}
+
+func createLegacyContainer(ctx context.Context, t *testing.T, apiClient client.APIClient, desiredMAC string, ops ...func(*container.TestContainerConfig)) string {


This probably could've been done with the compat package, but it's internal to the daemon; thought it was OK to have a slightly more ugly approach here for now (but we could clean things up a bit in future if we expect it to stay around for longer).

100, and tbf it isn't all that ugly. I don't think I opened up a PR on it because it was very ugly, but over the past weekend I worked a solution that went down the rabbit hole of making the compat.Wrapper type use generics for the base. Once I hit the need to use reflection to have pointers I know it was not the way. 😅

over the past weekend I worked a solution that went down the rabbit hole of making the compat.Wrapper type use generics for the base. Once I hit the need to use reflection to have pointers I know it was not the way. 😅

Yes, I think I originally went down a similar rabit hole, then decided to simplify the implementation.

That said; we can improve the compat package, and add features where needed;

it's an internal package

which means we have the flexibility to make any changes as we gain experience with using it

In general, I still think we should develop most internal packages with the assumption they either may be "public" at some point

Or if they don't become external, still try to make them fit for "public" quality; have the right GoDoc in place to describe usage, etc.

Because internal packages may start spreading across the code-base, so still try to keep their API well-designed if possible.

But if we don't have a good API / signature yet; using a non-exported implementation (such as here) can be a good stepping-stone; it more clearly make it "this is ad-hoc", but also keeps the code-changes local (easier to remove these bits later).

... of course there's exceptions; some internal packages may have a very specific use-case, and "cutting corners" (for performance reasons or otherwise), and that's OK (e.g. an internal package having requirements for callers to handle locking of their own; but maybe that's the same for "public" packages; they MAY have similar requirements, so it's important to have behavior properly documented).

but we could clean things up a bit in future if we expect it to stay around for longer

I think we should have the necessary utils to test that sort of cases as we may need to deprecate / remove API field in the future.

I'm wondering if we could use a custom http.RoundTripper to modify the request on the fly. This way we'd be able to keep using the client and testutil methods instead of writing dedicated types and client functions.

Yeah, we should improve those utils; also want to cleanup the actual "request" code in the client https://github.com/moby/moby/blob/1402c025e1f13ce041e53495870ace5330e3b1ea/client/request.go

and where possible look if we can combine some things there; having two completely distinct implementations is good for some cases, but not great for others.

thaJeztah · 2025-10-15T20:20:04Z

integration/container/run_linux_test.go

+	config := container.NewTestConfig(ops...)
+	ep := "/v" + apiClient.ClientVersion() + "/containers/create"
+	if config.Name != "" {
+		ep += "?name=" + config.Name
+	}
+	res, _, err := request.Post(ctx, ep, request.Host(apiClient.DaemonHost()), request.JSONBody(&legacyCreateRequest{
+		CreateRequest: containertypes.CreateRequest{
+			Config:           config.Config,
+			HostConfig:       config.HostConfig,
+			NetworkingConfig: config.NetworkingConfig,
+		},
+		MacAddress: desiredMAC,
+	}))


Same for this code; I think the request test-utility is fairly useful; it makes sure we can create API requests without accounting for the client and/or API-types.

That said, there's some amount of ugliness in dealing with these, and perhaps it would be nice to have utilities / functional arguments to also being able to unmarshal the response (without having to do so manually)

To some extent, it's been something I've been considering for the client's "request" utilities (which have some amount of logic w.r.t. API-version and the host to connect to); it'd be nice to export those / make them accessible to make it easier to write tooling for extending the API with custom endpoints (but still being able to share the common setup logic).

austinvazquez · 2025-10-15T20:20:15Z

daemon/server/router/container/container_routes.go

-		return err
-	} else if warn != "" {
-		warnings = append(warnings, warn)
+	if versions.LessThan(version, "1.52") {


Within handleMACAddressBC you can remove this piece now.
40715ad#diff-9f55aa9860d398370c167bbd161a965c6e6727b8a9798996cdaafe0f74772c20R764-R766

Yes, was considering that (actually had it removed while rebasing), because it's redundant.

Then opted to leave it be, and be explicit if it would ever be moved and we'd have to trace back "what API version was it again?".

I'll probably forget about that at some point, and will be the one removing it at some point 😂

austinvazquez

LGTM!

vvoland

SGTM

thaJeztah · 2025-10-16T10:40:05Z

Last push is just a rebase before I start making changes, so ignore

akerouanton · 2025-10-16T13:23:25Z

daemon/server/router/container/container_routes_test.go

 		epConfig            map[string]*network.EndpointSettings
 		expEpWithCtrWideMAC string
 		expEpWithNoMAC      string
 		expCtrWideMAC       string


You can remove that field now.

akerouanton · 2025-10-16T13:41:06Z

integration/container/run_linux_test.go

+type legacyCreateRequest struct {
+	containertypes.CreateRequest
+	// Mac Address of the container.
+	//
+	// MacAddress field is deprecated since API v1.44. Use EndpointSettings.MacAddress instead.
+	MacAddress string `json:",omitempty"`
+}
+
+func createLegacyContainer(ctx context.Context, t *testing.T, apiClient client.APIClient, desiredMAC string, ops ...func(*container.TestContainerConfig)) string {


but we could clean things up a bit in future if we expect it to stay around for longer

I think we should have the necessary utils to test that sort of cases as we may need to deprecate / remove API field in the future.

I'm wondering if we could use a custom http.RoundTripper to modify the request on the fly. This way we'd be able to keep using the client and testutil methods instead of writing dedicated types and client functions.

robmry · 2025-10-16T14:48:01Z

daemon/server/router/container/inspect.go

+			// API v1.52, which removes this entirely.
+			wrapOpts = append(wrapOpts,
+				compat.WithExtraFields(map[string]any{
+					"NetworkConfig": map[string]any{


This should be NetworkSettings.

DERP! Thought I fixed it but only in the other PR 🙈

robmry · 2025-10-16T15:07:26Z

daemon/server/router/container/inspect.go

+				mainNetwork = ctr.HostConfig.NetworkMode.NetworkName()
+			}
+			if v := ctr.NetworkSettings.Networks[mainNetwork]; v != nil && v.MacAddress != "" {
+				macAddress = v.MacAddress


Since commit 8c64b85, this has only been filled in if the MAC address is explicitly configured - by copying DesiredMacAddress instead of MacAddress ... but I don't think this code has that struct.

So it's a change in the older API responses, but maybe not an important one.

🙈 completely missed the other field name. Thanks! Fixing

thaJeztah · 2025-10-20T11:15:17Z

daemon/container_operations.go

+			// 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.
+			epConfig.DesiredMacAddress = epConfig.MacAddress
 			ctr.NetworkSettings.Networks[name] = &network.EndpointSettings{


If we do actually return this in the API, and accept this in the API request, then the client could send this value (and for older API versions, we could reset .MacAddress to make it only used for generated, or "actual" MAC Address)

thaJeztah · 2025-10-20T12:25:52Z

Hmm.. ok looks like there's at least one legit failure;

=== RUN   TestCfgdMACAddrOnRestart
    mac_addr_test.go:143: assertion failed: 02:42:ac:11:00:42 (wantMAC string) != 1e:92:87:a5:c1:00 (gotMAC string)
--- FAIL: TestCfgdMACAddrOnRestart (2.41s)

thompson-shaun · 2025-10-20T15:08:04Z

Going with 2730600 option for now -- @robmry 👀

thaJeztah · 2025-10-20T16:44:59Z

Moved this one out of draft again; I kept the "option 2", and have a branch that I can push as draft after this (but for future).

@robmry @austinvazquez PTAL 🤗

robmry · 2025-10-20T17:45:40Z

daemon/inspect.go

 	if ctr.RWLayer == nil {
 		if ctr.State.Dead {
-			return inspectResponse, nil
+			return inspectResponse, "", nil


I think we should still return the configured MAC address in this case - and ditto for line 185?

It'd previously have been assigned to ctr.Config.MacAddress, which ends up in the response. (And it's config, not running state.)

Ah, yeah, perhaps. Then again, a dead container probably won't even be shown, as it's only used for containers that failed to be removed if I'm not mistaken.

Oh ok, no problem then - thank you.

I updated the code to also return it for these; while it shouldn't be needed, it also doesn't hurt as we have the information already.

This part of the code will also go away once graph-drivers are removed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Rewrite the router to use a local struct to unmarshal the deprecated field for requests that send it, and adjust the adoption code. There also appeared to be duplication between daemon.getInspectData, and the containerRouter.postContainersCreate methods, as both were back-filling the field. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

There also appeared to be duplication between daemon.getInspectData, and the containerRouter.postContainersCreate methods, as both were back-filling the field. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

github-actions bot added area/networking Networking area/daemon Core Engine area/dependencies module/api module/client labels Oct 15, 2025

thaJeztah force-pushed the handle_macaddr branch 2 times, most recently from 18571d7 to 5ae61d1 Compare October 15, 2025 16:44

thaJeztah changed the title ~~[wip] testing alternative approach for deprecated field~~ api/types/container: remove deprecated Config.MacAddress Oct 15, 2025

thaJeztah commented Oct 15, 2025

View reviewed changes

thaJeztah marked this pull request as ready for review October 15, 2025 18:54

austinvazquez reviewed Oct 15, 2025

View reviewed changes

austinvazquez mentioned this pull request Oct 15, 2025

api/types/container: remove support for config mac address #51152

Closed

thaJeztah force-pushed the handle_macaddr branch from 5ae61d1 to 939f1e8 Compare October 15, 2025 20:12

thaJeztah commented Oct 15, 2025

View reviewed changes

austinvazquez reviewed Oct 15, 2025

View reviewed changes

thaJeztah added this to the 29.0.0 milestone Oct 15, 2025

thaJeztah added the release-blocker PRs we want to block a release on label Oct 15, 2025

austinvazquez approved these changes Oct 15, 2025

View reviewed changes

thaJeztah requested review from robmry and vvoland October 16, 2025 07:35

vvoland approved these changes Oct 16, 2025

View reviewed changes

thaJeztah force-pushed the handle_macaddr branch from 939f1e8 to 7b10745 Compare October 16, 2025 10:39

thaJeztah force-pushed the handle_macaddr branch from 7b10745 to 93b6ee1 Compare October 16, 2025 11:26

thaJeztah mentioned this pull request Oct 16, 2025

daemon/server/router/container: fix back-filling of top-level network fields #51201

Merged

akerouanton reviewed Oct 16, 2025

View reviewed changes

robmry reviewed Oct 16, 2025

View reviewed changes

thaJeztah marked this pull request as draft October 16, 2025 20:26

thaJeztah force-pushed the handle_macaddr branch 3 times, most recently from efb3c08 to bf36a73 Compare October 20, 2025 11:13

thaJeztah commented Oct 20, 2025

View reviewed changes

thaJeztah force-pushed the handle_macaddr branch 2 times, most recently from 8641eb5 to fe1585a Compare October 20, 2025 12:47

thaJeztah force-pushed the handle_macaddr branch from fe1585a to ecb7d22 Compare October 20, 2025 15:20

github-actions bot added the area/swarm label Oct 20, 2025

thaJeztah marked this pull request as ready for review October 20, 2025 16:44

robmry reviewed Oct 20, 2025

View reviewed changes

robmry approved these changes Oct 20, 2025

View reviewed changes

thaJeztah added 7 commits October 21, 2025 08:33

integration/container: rewrite with request for MacAddress

b4b0b6f

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration/networking: TestInspectCfgdMAC: rewrite with request util

8088de7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

integration/networking: TestWatchtowerCreate: rewrite with request util

e5864a5

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

client: ContainerCreate: remove handling of legacy MacAddress

0030ba2

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

daemon: Daemon.getInspectData: move migration code to router

79912d4

There also appeared to be duplication between daemon.getInspectData, and the containerRouter.postContainersCreate methods, as both were back-filling the field. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/types/container: remove deprecated Config.MacAddress

a8950e0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the handle_macaddr branch from ecb7d22 to a8950e0 Compare October 21, 2025 07:15

thaJeztah added the status/4-merge label Oct 21, 2025

thaJeztah merged commit ebfcc6c into moby:master Oct 21, 2025
289 of 291 checks passed

thaJeztah deleted the handle_macaddr branch October 21, 2025 09:16

Conversation

thaJeztah commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 15, 2025

Uh oh!

austinvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

austinvazquez commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

austinvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

vvoland left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robmry Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 20, 2025

Uh oh!

thompson-shaun commented Oct 20, 2025

Uh oh!

thaJeztah commented Oct 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

thaJeztah commented Oct 15, 2025 •

edited

Loading

robmry Oct 16, 2025 •

edited

Loading