From 4eed3dcdfeb147529339e06f2dceecf43caed45a Mon Sep 17 00:00:00 2001 From: Albin Kerouanton Date: Thu, 22 Feb 2024 11:18:36 +0100 Subject: [PATCH] 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 {