Merge pull request #47431 from akerouanton/api-normalize-default-NetworkMode

api: normalize the default NetworkMode

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)
 
@@ -646,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
 		}
 
@@ -675,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 {

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,

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
 	}

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
 	}

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 {

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.
 			}
 		}