Skip to content

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

Merged
thaJeztah merged 2 commits intomoby:masterfrom
thaJeztah:fix_backfil
Oct 16, 2025
Merged

daemon/server/router/container: fix back-filling of top-level network fields#51201
thaJeztah merged 2 commits intomoby:masterfrom
thaJeztah:fix_backfil

Conversation

@thaJeztah
Copy link
Member

This was introduced in fc1ff44, which
back-filled the top-level network-properties for the bridge network,
but the fields ended up at the top-level of the response, not the top-level
of the NetworkSettings.

- Human readable description for the release notes

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

@thaJeztah thaJeztah added this to the 29.0.0 milestone Oct 16, 2025
@thaJeztah thaJeztah added area/api API status/2-code-review area/networking Networking kind/bugfix PR's that fix bugs release-blocker PRs we want to block a release on labels Oct 16, 2025
@github-actions github-actions bot added the area/daemon Core Engine label Oct 16, 2025
… fields

This was introduced in fc1ff44, which
back-filled the top-level network-properties for the bridge network,
but the fields ended up at the top-level of the response, not the top-level
of the NetworkSettings.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The TestMacAddressIsAppliedToMainNetworkWithShortID was starting its
own daemon, but the apiClient was created with the defaults, so was
connecting to the global test-daemon.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah requested a review from akerouanton October 16, 2025 13:35
@thaJeztah thaJeztah marked this pull request as ready for review October 16, 2025 13:35
@thaJeztah thaJeztah requested a review from robmry October 16, 2025 13:35
"IPAddress": bridgeNw.IPAddress,
"IPPrefixLen": bridgeNw.IPPrefixLen,
"IPv6Gateway": bridgeNw.IPv6Gateway,
"MacAddress": bridgeNw.MacAddress,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we don't add an extra MacAddress field to the Config struct because we forgot to add this migration in #47233?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this version / PR still adds it, but through the daemon.GetInspectdata (or whatever it's called). The other PR removes that and puts it all in the router.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra fields in NetworkSettings are quite separate from the Config.MacAddress - I think the rules are ...

The NetworkSettings fields are filled in with default bridge info, if the container's connected to the default bridge (and now, only if the API version is < 1.52).

The Config field is filled in with the MAC address from network HostConfig.NetworkMode, but only if the MAC is explicitly configured.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go ahead and bring this one in; IIUC; if something is needed (for v28.x??), we can fix it separately

@thaJeztah thaJeztah merged commit 248333d into moby:master Oct 16, 2025
181 checks passed
@thaJeztah thaJeztah deleted the fix_backfil branch October 16, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api API area/daemon Core Engine area/networking Networking kind/bugfix PR's that fix bugs release-blocker PRs we want to block a release on status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants