Merge pull request #45906 from akerouanton/create-with-several-networks

api: Allow ContainerCreate to take several EndpointsConfig for >= API 1.44

api/server/router/container/container_routes.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"runtime"
 	"strconv"
+	"strings"
 
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/platforms"
@@ -607,6 +608,17 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
 		}
 	}
 
+	if versions.LessThan(version, "1.44") {
+		// Creating a container connected to several networks is not supported until v1.44.
+		if networkingConfig != nil && len(networkingConfig.EndpointsConfig) > 1 {
+			l := make([]string, 0, len(networkingConfig.EndpointsConfig))
+			for k := range networkingConfig.EndpointsConfig {
+				l = append(l, k)
+			}
+			return errdefs.InvalidParameter(errors.Errorf("Container cannot be created with multiple network endpoints: %s", strings.Join(l, ", ")))
+		}
+	}
+
 	ccr, err := s.backend.ContainerCreate(ctx, types.ContainerCreateConfig{
 		Name:             name,
 		Config:           config,

api/swagger.yaml

@@ -1363,16 +1363,16 @@ definitions:
       EndpointsConfig:
         description: |
           A mapping of network name to endpoint configuration for that network.
+          The endpoint configuration can be left empty to connect to that
+          network with no particular endpoint configuration.
         type: "object"
         additionalProperties:
           $ref: "#/definitions/EndpointSettings"
     example:
       # putting an example here, instead of using the example values from
-      # /definitions/EndpointSettings, because containers/create currently
-      # does not support attaching to multiple networks, so the example request
-      # would be confusing if it showed that multiple networks can be contained
-      # in the EndpointsConfig.
-      # TODO remove once we support multiple networks on container create (see https://github.com/moby/moby/blob/07e6b843594e061f82baa5fa23c2ff7d536c2a05/daemon/create.go#L323)
+      # /definitions/EndpointSettings, because EndpointSettings contains
+      # operational data returned when inspecting a container that we don't
+      # accept here.
       EndpointsConfig:
         isolated_nw:
           IPAMConfig:
@@ -1387,6 +1387,7 @@ definitions:
           Aliases:
             - "server_x"
             - "server_y"
+        database_nw: {}
 
   NetworkSettings:
     description: "NetworkSettings exposes the network settings in the API"
@@ -6439,6 +6440,7 @@ paths:
                     Aliases:
                       - "server_x"
                       - "server_y"
+                  database_nw: {}
 
           required: true
       responses:

daemon/cluster/executor/container/adapter.go

@@ -300,18 +300,6 @@ func (c *containerAdapter) create(ctx context.Context) error {
 		return err
 	}
 
-	// Docker daemon currently doesn't support multiple networks in container create
-	// Connect to all other networks
-	nc := c.container.connectNetworkingConfig(c.backend)
-
-	if nc != nil {
-		for n, ep := range nc.EndpointsConfig {
-			if err := c.backend.ConnectContainerToNetwork(cr.ID, n, ep); err != nil {
-				return err
-			}
-		}
-	}
-
 	container := c.container.task.Spec.GetContainer()
 	if container == nil {
 		return errors.New("unable to get container from task spec")

daemon/cluster/executor/container/container.go

@@ -503,7 +503,6 @@ func (c *containerConfig) resources() enginecontainer.Resources {
 	return resources
 }
 
-// Docker daemon supports just 1 network during container create.
 func (c *containerConfig) createNetworkingConfig(b executorpkg.Backend) *network.NetworkingConfig {
 	var networks []*api.NetworkAttachment
 	if c.task.Spec.GetContainer() != nil || c.task.Spec.GetAttachment() != nil {
@@ -511,28 +510,10 @@ func (c *containerConfig) createNetworkingConfig(b executorpkg.Backend) *network
 	}
 
 	epConfig := make(map[string]*network.EndpointSettings)
-	if len(networks) > 0 {
-		epConfig[networks[0].Network.Spec.Annotations.Name] = getEndpointConfig(networks[0], b)
-	}
-
-	return &network.NetworkingConfig{EndpointsConfig: epConfig}
-}
-
-// TODO: Merge this function with createNetworkingConfig after daemon supports multiple networks in container create
-func (c *containerConfig) connectNetworkingConfig(b executorpkg.Backend) *network.NetworkingConfig {
-	var networks []*api.NetworkAttachment
-	if c.task.Spec.GetContainer() != nil {
-		networks = c.task.Networks
-	}
-	// First network is used during container create. Other networks are used in "docker network connect"
-	if len(networks) < 2 {
-		return nil
-	}
-
-	epConfig := make(map[string]*network.EndpointSettings)
-	for _, na := range networks[1:] {
+	for _, na := range networks {
 		epConfig[na.Network.Spec.Annotations.Name] = getEndpointConfig(na, b)
 	}
+
 	return &network.NetworkingConfig{EndpointsConfig: epConfig}
 }
 

daemon/create.go

@@ -321,19 +321,11 @@ func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *i
 	return nil
 }
 
-// Checks if the client set configurations for more than one network while creating a container
-// Also checks if the IPAMConfig is valid
+// verifyNetworkingConfig validates if the nwConfig is valid.
 func verifyNetworkingConfig(nwConfig *networktypes.NetworkingConfig) error {
-	if nwConfig == nil || len(nwConfig.EndpointsConfig) == 0 {
+	if nwConfig == nil {
 		return nil
 	}
-	if len(nwConfig.EndpointsConfig) > 1 {
-		l := make([]string, 0, len(nwConfig.EndpointsConfig))
-		for k := range nwConfig.EndpointsConfig {
-			l = append(l, k)
-		}
-		return fmt.Errorf("container cannot be connected to network endpoints: %s", strings.Join(l, ", "))
-	}
 
 	for k, v := range nwConfig.EndpointsConfig {
 		if v == nil {

docs/api/version-history.md

@@ -45,6 +45,8 @@ keywords: "API, Docker, rcli, REST, documentation"
   such, the `CheckDuplicate` field is now deprecated. Note that this change is
   _unversioned_ and applied to all API versions on daemon that support version
   1.44.
+* `POST /containers/create` now accepts multiple `EndpointSettings` in
+  `NetworkingConfig.EndpointSettings`.
 
 ## v1.43 API changes
 

integration-cli/docker_api_containers_test.go

@@ -556,33 +556,6 @@ func (s *DockerAPISuite) TestContainerAPICreateEmptyConfig(c *testing.T) {
 	assert.ErrorContains(c, err, "no command specified")
 }
 
-func (s *DockerAPISuite) TestContainerAPICreateMultipleNetworksConfig(c *testing.T) {
-	// Container creation must fail if client specified configurations for more than one network
-	config := container.Config{
-		Image: "busybox",
-	}
-
-	networkingConfig := network.NetworkingConfig{
-		EndpointsConfig: map[string]*network.EndpointSettings{
-			"net1": {},
-			"net2": {},
-			"net3": {},
-		},
-	}
-
-	apiClient, err := client.NewClientWithOpts(client.FromEnv)
-	assert.NilError(c, err)
-	defer apiClient.Close()
-
-	_, err = apiClient.ContainerCreate(testutil.GetContext(c), &config, &container.HostConfig{}, &networkingConfig, nil, "")
-	msg := err.Error()
-	// network name order in error message is not deterministic
-	assert.Assert(c, strings.Contains(msg, "container cannot be connected to network endpoints"))
-	assert.Assert(c, strings.Contains(msg, "net1"))
-	assert.Assert(c, strings.Contains(msg, "net2"))
-	assert.Assert(c, strings.Contains(msg, "net3"))
-}
-
 func (s *DockerAPISuite) TestContainerAPICreateBridgeNetworkMode(c *testing.T) {
 	// Windows does not support bridge
 	testRequires(c, DaemonIsLinux)

integration/container/create_test.go

@@ -13,6 +13,7 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/api/types/versions"
+	"github.com/docker/docker/client"
 	"github.com/docker/docker/errdefs"
 	ctr "github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/oci"
@@ -585,3 +586,39 @@ func TestCreateInvalidHostConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestCreateWithMultipleEndpointSettings(t *testing.T) {
+	ctx := setupTest(t)
+
+	testcases := []struct {
+		apiVersion  string
+		expectedErr string
+	}{
+		{apiVersion: "1.44"},
+		{apiVersion: "1.43", expectedErr: "Container cannot be created with multiple network endpoints"},
+	}
+
+	for _, tc := range testcases {
+		t.Run("with API v"+tc.apiVersion, func(t *testing.T) {
+			apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion(tc.apiVersion))
+			assert.NilError(t, err)
+
+			config := container.Config{
+				Image: "busybox",
+			}
+			networkingConfig := network.NetworkingConfig{
+				EndpointsConfig: map[string]*network.EndpointSettings{
+					"net1": {},
+					"net2": {},
+					"net3": {},
+				},
+			}
+			_, err = apiClient.ContainerCreate(ctx, &config, &container.HostConfig{}, &networkingConfig, nil, "")
+			if tc.expectedErr == "" {
+				assert.NilError(t, err)
+			} else {
+				assert.ErrorContains(t, err, tc.expectedErr)
+			}
+		})
+	}
+}

integration/internal/container/ops.go

@@ -146,6 +146,17 @@ func WithIPv6(networkName, ip string) func(*TestContainerConfig) {
 	}
 }
 
+func WithEndpointSettings(nw string, config *network.EndpointSettings) func(*TestContainerConfig) {
+	return func(c *TestContainerConfig) {
+		if c.NetworkingConfig.EndpointsConfig == nil {
+			c.NetworkingConfig.EndpointsConfig = map[string]*network.EndpointSettings{}
+		}
+		if _, ok := c.NetworkingConfig.EndpointsConfig[nw]; !ok {
+			c.NetworkingConfig.EndpointsConfig[nw] = config
+		}
+	}
+}
+
 // WithLogDriver sets the log driver to use for the container
 func WithLogDriver(driver string) func(*TestContainerConfig) {
 	return func(c *TestContainerConfig) {

integration/internal/network/network.go

@@ -33,3 +33,10 @@ func CreateNoError(ctx context.Context, t *testing.T, client client.APIClient, n
 	assert.NilError(t, err)
 	return name
 }
+
+func RemoveNoError(ctx context.Context, t *testing.T, apiClient client.APIClient, name string) {
+	t.Helper()
+
+	err := apiClient.NetworkRemove(ctx, name)
+	assert.NilError(t, err)
+}

integration/network/bridge_test.go

@@ -0,0 +1,45 @@
+package network
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	networktypes "github.com/docker/docker/api/types/network"
+	"github.com/docker/docker/api/types/versions"
+	ctr "github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/integration/internal/network"
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/skip"
+)
+
+func TestCreateWithMultiNetworks(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.44"), "requires API v1.44")
+
+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	network.CreateNoError(ctx, t, apiClient, "testnet1")
+	defer network.RemoveNoError(ctx, t, apiClient, "testnet1")
+
+	network.CreateNoError(ctx, t, apiClient, "testnet2")
+	defer network.RemoveNoError(ctx, t, apiClient, "testnet2")
+
+	attachCtx, cancel := context.WithTimeout(ctx, 1*time.Second)
+	defer cancel()
+	res := ctr.RunAttach(attachCtx, t, apiClient,
+		ctr.WithCmd("ip", "-o", "-4", "addr", "show"),
+		ctr.WithNetworkMode("testnet1"),
+		ctr.WithEndpointSettings("testnet1", &networktypes.EndpointSettings{}),
+		ctr.WithEndpointSettings("testnet2", &networktypes.EndpointSettings{}))
+
+	assert.Equal(t, res.ExitCode, 0)
+	assert.Equal(t, res.Stderr.String(), "")
+
+	// Only interfaces with an IPv4 address are printed by iproute2 when flag -4 is specified. Here, we should have two
+	// interfaces for testnet1 and testnet2, plus lo.
+	ifacesWithAddress := strings.Count(res.Stdout.String(), "\n")
+	assert.Equal(t, ifacesWithAddress, 3)
+}