From 4eed3dcdfeb147529339e06f2dceecf43caed45a Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Thu, 22 Feb 2024 11:18:36 +0100 Subject: [PATCH 1/2] api: normalize the default NetworkMode The NetworkMode "default" is now normalized into the value it aliases ("bridge" on Linux and "nat" on Windows) by the ContainerCreate endpoint, the legacy image builder, Swarm's cluster executor and by the container restore codepath. builder-next is left untouched as it already uses the normalized value (ie. bridge). Going forward, this will make maintenance easier as there's one less NetworkMode to care about. Signed-off-by: Albin Kerouanton --- .../router/container/container_routes.go | 18 ++++++++++--- builder/dockerfile/internals.go | 13 +++++++++- daemon/cluster/executor/container/adapter.go | 26 +++++++++++++++++-- daemon/daemon.go | 16 ++++++++++++ 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index bf132f19b1..731bbe4805 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -456,15 +456,27 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo if hostConfig == nil { hostConfig = &container.HostConfig{} } - if hostConfig.NetworkMode == "" { - hostConfig.NetworkMode = "default" - } if networkingConfig == nil { networkingConfig = &network.NetworkingConfig{} } if networkingConfig.EndpointsConfig == nil { networkingConfig.EndpointsConfig = make(map[string]*network.EndpointSettings) } + // The NetworkMode "default" is used as a way to express a container should + // be attached to the OS-dependant default network, in an OS-independent + // way. Doing this conversion as soon as possible ensures we have less + // NetworkMode to handle down the path (including in the + // backward-compatibility layer we have just below). + // + // Note that this is not the only place where this conversion has to be + // done (as there are various other places where containers get created). + if hostConfig.NetworkMode == "" || hostConfig.NetworkMode.IsDefault() { + hostConfig.NetworkMode = runconfig.DefaultDaemonNetworkMode() + if nw, ok := networkingConfig.EndpointsConfig[network.NetworkDefault]; ok { + networkingConfig.EndpointsConfig[hostConfig.NetworkMode.NetworkName()] = nw + delete(networkingConfig.EndpointsConfig, network.NetworkDefault) + } + } version := httputils.VersionFromContext(ctx) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index b6b0b8f42a..feca83e9f2 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -15,11 +15,13 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/builder" "github.com/docker/docker/image" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/stringid" + "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -377,12 +379,21 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf Ulimits: options.Ulimits, } + // We need to make sure no empty string or "default" NetworkMode is + // provided to the daemon as it doesn't support them. + // + // This is in line with what the ContainerCreate API endpoint does. + networkMode := options.NetworkMode + if networkMode == "" || networkMode == network.NetworkDefault { + networkMode = runconfig.DefaultDaemonNetworkMode().NetworkName() + } + hc := &container.HostConfig{ SecurityOpt: options.SecurityOpt, Isolation: options.Isolation, ShmSize: options.ShmSize, Resources: resources, - NetworkMode: container.NetworkMode(options.NetworkMode), + NetworkMode: container.NetworkMode(networkMode), // Set a log config to override any default value set on the daemon LogConfig: defaultLogConfig, ExtraHosts: options.ExtraHosts, diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 7360031ffa..262c614058 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -17,12 +17,14 @@ import ( "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/events" + "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/registry" containerpkg "github.com/docker/docker/container" "github.com/docker/docker/daemon" "github.com/docker/docker/daemon/cluster/convert" executorpkg "github.com/docker/docker/daemon/cluster/executor" "github.com/docker/docker/libnetwork" + "github.com/docker/docker/runconfig" volumeopts "github.com/docker/docker/volume/service/opts" gogotypes "github.com/gogo/protobuf/types" "github.com/moby/swarmkit/v2/agent/exec" @@ -290,14 +292,34 @@ func (c *containerAdapter) waitForDetach(ctx context.Context) error { } func (c *containerAdapter) create(ctx context.Context) error { + hostConfig := c.container.hostConfig(c.dependencies.Volumes()) + netConfig := c.container.createNetworkingConfig(c.backend) + + // We need to make sure no empty string or "default" NetworkMode is + // provided to the daemon as it doesn't support them. + // + // This is in line with what the ContainerCreate API endpoint does, but + // unlike that endpoint we can't do that in the ServiceCreate endpoint as + // the cluster leader and the current node might not be running on the same + // OS. Since the normalized value isn't the same on Windows and Linux, we + // need to make this normalization happen once we're sure we won't make a + // cross-OS API call. + if hostConfig.NetworkMode == "" || hostConfig.NetworkMode.IsDefault() { + hostConfig.NetworkMode = runconfig.DefaultDaemonNetworkMode() + if v, ok := netConfig.EndpointsConfig[network.NetworkDefault]; ok { + delete(netConfig.EndpointsConfig, network.NetworkDefault) + netConfig.EndpointsConfig[runconfig.DefaultDaemonNetworkMode().NetworkName()] = v + } + } + var cr containertypes.CreateResponse var err error if cr, err = c.backend.CreateManagedContainer(ctx, backend.ContainerCreateConfig{ Name: c.container.name(), Config: c.container.config(), - HostConfig: c.container.hostConfig(c.dependencies.Volumes()), + HostConfig: hostConfig, // Use the first network in container create - NetworkingConfig: c.container.createNetworkingConfig(c.backend), + NetworkingConfig: netConfig, }); err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index d8320629d7..e7ca77d8cb 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -32,6 +32,7 @@ import ( "github.com/docker/docker/api/types/backend" containertypes "github.com/docker/docker/api/types/container" imagetypes "github.com/docker/docker/api/types/image" + networktypes "github.com/docker/docker/api/types/network" registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/api/types/volume" @@ -373,6 +374,21 @@ func (daemon *Daemon) restore(cfg *configStore) error { Type: local.Name, } } + + // Normalize the "default" network mode into the network mode + // it aliases ("bridge on Linux and "nat" on Windows). This is + // also done by the container router, for new containers. But + // we need to do it here too to handle containers that were + // created prior to v26.0. + // + // TODO(aker): remove this migration code once the next LTM version of MCR is released. + if c.HostConfig.NetworkMode.IsDefault() { + c.HostConfig.NetworkMode = runconfig.DefaultDaemonNetworkMode() + if nw, ok := c.NetworkSettings.Networks[networktypes.NetworkDefault]; ok { + c.NetworkSettings.Networks[c.HostConfig.NetworkMode.NetworkName()] = nw + delete(c.NetworkSettings.Networks, networktypes.NetworkDefault) + } + } } if err := daemon.checkpointAndSave(c); err != nil { From c4689034fdad6a7ffbf45e62131d9a412ed07f77 Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Thu, 22 Feb 2024 17:35:11 +0100 Subject: [PATCH 2/2] daemon: don't call NetworkMode.IsDefault() Previous commit made this unnecessary. Signed-off-by: Albin Kerouanton --- api/server/router/container/container_routes.go | 4 ++-- daemon/container_operations.go | 12 ------------ daemon/inspect.go | 8 ++------ 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 731bbe4805..3872028652 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -658,7 +658,7 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf } return "", nil } - if !hostConfig.NetworkMode.IsDefault() && !hostConfig.NetworkMode.IsBridge() && !hostConfig.NetworkMode.IsUserDefined() { + if !hostConfig.NetworkMode.IsBridge() && !hostConfig.NetworkMode.IsUserDefined() { return "", runconfig.ErrConflictContainerNetworkAndMac } @@ -687,7 +687,7 @@ func handleMACAddressBC(config *container.Config, hostConfig *container.HostConf return "", nil } var warning string - if hostConfig.NetworkMode.IsDefault() || hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() { + if hostConfig.NetworkMode.IsBridge() || hostConfig.NetworkMode.IsUserDefined() { nwName := hostConfig.NetworkMode.NetworkName() // If there's no endpoint config, create a place to store the configured address. if len(networkingConfig.EndpointsConfig) == 0 { diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 78b1c98a19..48235ddd7d 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -420,9 +420,6 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai } networkName := mode.NetworkName() - if mode.IsDefault() { - networkName = daemon.netController.Config().DefaultNetwork - } if mode.IsUserDefined() { var err error @@ -461,15 +458,6 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai } } - // Convert any settings added by client in default name to - // engine's default network name key - if mode.IsDefault() { - if nConf, ok := container.NetworkSettings.Networks[mode.NetworkName()]; ok { - container.NetworkSettings.Networks[networkName] = nConf - delete(container.NetworkSettings.Networks, mode.NetworkName()) - } - } - if !mode.IsUserDefined() { return } diff --git a/daemon/inspect.go b/daemon/inspect.go index 62c7896ac3..c4324f1708 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -130,12 +130,8 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, container *contai // unconditionally, to keep backward compatibility with clients that use // unversioned API endpoints. if container.Config != nil && container.Config.MacAddress == "" { //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. - if nwm := hostConfig.NetworkMode; nwm.IsDefault() || nwm.IsBridge() || nwm.IsUserDefined() { - name := nwm.NetworkName() - if nwm.IsDefault() { - name = daemon.netController.Config().DefaultNetwork - } - if epConf, ok := container.NetworkSettings.Networks[name]; ok { + if nwm := hostConfig.NetworkMode; nwm.IsBridge() || nwm.IsUserDefined() { + if epConf, ok := container.NetworkSettings.Networks[nwm.NetworkName()]; ok { container.Config.MacAddress = epConf.DesiredMacAddress //nolint:staticcheck // ignore SA1019: field is deprecated, but still used on API < v1.44. } }