From 79912d4c7f40c9432a83157e6834098d60729351 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 15 Oct 2025 17:30:11 +0200 Subject: [PATCH] daemon: Daemon.getInspectData: move migration code to router 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 --- daemon/cluster/executor/backend.go | 2 +- daemon/cluster/executor/container/adapter.go | 2 +- daemon/inspect.go | 31 ++++++++++---------- daemon/inspect_test.go | 4 +-- daemon/server/router/container/backend.go | 2 +- daemon/server/router/container/inspect.go | 17 +++++++++-- 6 files changed, 36 insertions(+), 22 deletions(-) diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index b78e0457f9..428eb7f568 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -43,7 +43,7 @@ type Backend interface { ActivateContainerServiceBinding(containerName string) error DeactivateContainerServiceBinding(containerName string) error UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error - ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (*container.InspectResponse, error) + ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ error) ContainerWait(ctx context.Context, name string, condition container.WaitCondition) (<-chan containerpkg.StateStatus, error) ContainerRm(name string, config *backend.ContainerRmConfig) error ContainerKill(name string, sig string) error diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 39f7d292d4..3964284446 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -375,7 +375,7 @@ func (c *containerAdapter) start(ctx context.Context) error { } func (c *containerAdapter) inspect(ctx context.Context) (containertypes.InspectResponse, error) { - cs, err := c.backend.ContainerInspect(ctx, c.container.name(), backend.ContainerInspectOptions{}) + cs, _, err := c.backend.ContainerInspect(ctx, c.container.name(), backend.ContainerInspectOptions{}) if ctx.Err() != nil { return containertypes.InspectResponse{}, ctx.Err() } diff --git a/daemon/inspect.go b/daemon/inspect.go index ee4dd7fdea..835f22f28f 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -19,18 +19,18 @@ import ( // ContainerInspect returns low-level information about a // container. Returns an error if the container cannot be found, or if // there is an error getting the data. -func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (*containertypes.InspectResponse, error) { +func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) { ctr, err := daemon.GetContainer(name) if err != nil { - return nil, err + return nil, "", err } ctr.Lock() - base, err := daemon.getInspectData(&daemon.config().Config, ctr) + base, desiredMACAddress, err := daemon.getInspectData(&daemon.config().Config, ctr) if err != nil { ctr.Unlock() - return nil, err + return nil, "", err } // TODO(thaJeztah): do we need a deep copy here? Otherwise we could use maps.Clone (see https://github.com/moby/moby/commit/7917a36cc787ada58987320e67cc6d96858f3b55) @@ -61,7 +61,7 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options if options.Size { sizeRw, sizeRootFs, err := daemon.imageService.GetContainerLayerSize(ctx, base.ID) if err != nil { - return nil, err + return nil, "", err } base.SizeRw = &sizeRw base.SizeRootFs = &sizeRootFs @@ -80,10 +80,10 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options base.NetworkSettings = networkSettings base.ImageManifestDescriptor = imageManifest - return base, nil + return base, desiredMACAddress, nil } -func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (*containertypes.InspectResponse, error) { +func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Container) (_ *containertypes.InspectResponse, desiredMACAddress string, _ error) { // make a copy to play with hostConfig := *ctr.HostConfig @@ -101,10 +101,11 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co // Config.MacAddress field for older API versions (< 1.44). We set it here // unconditionally, to keep backward compatibility with clients that use // unversioned API endpoints. - if ctr.Config != nil && ctr.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + var macAddress string + if ctr.Config != nil { if nwm := hostConfig.NetworkMode; nwm.IsBridge() || nwm.IsUserDefined() { if epConf, ok := ctr.NetworkSettings.Networks[nwm.NetworkName()]; ok { - ctr.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. + macAddress = epConf.DesiredMacAddress } } } @@ -163,7 +164,7 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co } // Additional information only applies to graphDrivers, so we're done. - return inspectResponse, nil + return inspectResponse, macAddress, nil } inspectResponse.GraphDriver = &storage.DriverData{ @@ -171,9 +172,9 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co } if ctr.RWLayer == nil { if ctr.State.Dead { - return inspectResponse, nil + return inspectResponse, macAddress, nil } - return nil, errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil")) + return nil, "", errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil")) } graphDriverData, err := ctr.RWLayer.Metadata() @@ -181,13 +182,13 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co if ctr.State.Dead { // container is marked as Dead, and its graphDriver metadata may // have been removed; we can ignore errors. - return inspectResponse, nil + return inspectResponse, macAddress, nil } - return nil, errdefs.System(err) + return nil, "", errdefs.System(err) } inspectResponse.GraphDriver.Data = graphDriverData - return inspectResponse, nil + return inspectResponse, macAddress, nil } // ContainerExecInspect returns low-level information about the exec diff --git a/daemon/inspect_test.go b/daemon/inspect_test.go index 924c05179b..9ed81ec0ee 100644 --- a/daemon/inspect_test.go +++ b/daemon/inspect_test.go @@ -26,10 +26,10 @@ func TestGetInspectData(t *testing.T) { cfg := &configStore{} d.configStore.Store(cfg) - _, err := d.getInspectData(&cfg.Config, c) + _, _, err := d.getInspectData(&cfg.Config, c) assert.Check(t, is.ErrorContains(err, "RWLayer of container inspect-me is unexpectedly nil")) c.State.Dead = true - _, err = d.getInspectData(&cfg.Config, c) + _, _, err = d.getInspectData(&cfg.Config, c) assert.Check(t, err) } diff --git a/daemon/server/router/container/backend.go b/daemon/server/router/container/backend.go index 1bf19d4dec..ffba42989c 100644 --- a/daemon/server/router/container/backend.go +++ b/daemon/server/router/container/backend.go @@ -48,7 +48,7 @@ type stateBackend interface { // monitorBackend includes functions to implement to provide containers monitoring functionality. type monitorBackend interface { ContainerChanges(ctx context.Context, name string) ([]archive.Change, error) - ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (*container.InspectResponse, error) + ContainerInspect(ctx context.Context, name string, options backend.ContainerInspectOptions) (_ *container.InspectResponse, desiredMACAddress string, _ error) ContainerLogs(ctx context.Context, name string, config *backend.ContainerLogsOptions) (msgs <-chan *backend.LogMessage, tty bool, err error) ContainerStats(ctx context.Context, name string, config *backend.ContainerStatsConfig) error ContainerTop(name string, psArgs string) (*container.TopResponse, error) diff --git a/daemon/server/router/container/inspect.go b/daemon/server/router/container/inspect.go index 52412e8480..d9334f055a 100644 --- a/daemon/server/router/container/inspect.go +++ b/daemon/server/router/container/inspect.go @@ -15,7 +15,7 @@ import ( // getContainersByName inspects container's configuration and serializes it as json. func (c *containerRouter) getContainersByName(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - ctr, err := c.backend.ContainerInspect(ctx, vars["name"], backend.ContainerInspectOptions{ + ctr, desiredMACAddress, err := c.backend.ContainerInspect(ctx, vars["name"], backend.ContainerInspectOptions{ Size: httputils.BoolValue(r, "size"), }) if err != nil { @@ -39,7 +39,7 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon if versions.LessThan(version, "1.52") { if bridgeNw := ctr.NetworkSettings.Networks["bridge"]; bridgeNw != nil { // Old API versions showed the bridge's configuration as top-level - // fields in "NetworkConfig". + // fields in "NetworkSettings". // // This was deprecated in API v1.44, but kept in place until // API v1.52, which removes this entirely. @@ -56,6 +56,19 @@ func (c *containerRouter) getContainersByName(ctx context.Context, w http.Respon }, })) } + + // Migrate the container's main / default network's MacAddress to + // the Config.MacAddress field for older API versions (< 1.44). + // + // This was deprecated in API v1.44, but kept in place until + // API v1.52, which removed this entirely. + if desiredMACAddress != "" { + wrapOpts = append(wrapOpts, compat.WithExtraFields(map[string]any{ + "Config": map[string]any{ + "MacAddress": desiredMACAddress, + }, + })) + } } return httputils.WriteJSON(w, http.StatusOK, compat.Wrap(ctr, wrapOpts...))