Browse code

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

The API endpoint `/containers/create` accepts several EndpointsConfig
since v1.22 but the daemon would error out in such case. This check is
moved from the daemon to the api and is now applied only for API < 1.44,
effectively allowing the daemon to create containers connected to
several networks.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>

Albin Kerouanton authored on 2023/07/04 19:32:14
Showing 9 changed files
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"net/http"
9 9
 	"runtime"
10 10
 	"strconv"
11
+	"strings"
11 12
 
12 13
 	"github.com/containerd/containerd/log"
13 14
 	"github.com/containerd/containerd/platforms"
... ...
@@ -607,6 +608,17 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
607 607
 		}
608 608
 	}
609 609
 
610
+	if versions.LessThan(version, "1.44") {
611
+		// Creating a container connected to several networks is not supported until v1.44.
612
+		if networkingConfig != nil && len(networkingConfig.EndpointsConfig) > 1 {
613
+			l := make([]string, 0, len(networkingConfig.EndpointsConfig))
614
+			for k := range networkingConfig.EndpointsConfig {
615
+				l = append(l, k)
616
+			}
617
+			return errdefs.InvalidParameter(errors.Errorf("Container cannot be connected to network endpoints: %s", strings.Join(l, ", ")))
618
+		}
619
+	}
620
+
610 621
 	ccr, err := s.backend.ContainerCreate(ctx, types.ContainerCreateConfig{
611 622
 		Name:             name,
612 623
 		Config:           config,
... ...
@@ -1363,16 +1363,16 @@ definitions:
1363 1363
       EndpointsConfig:
1364 1364
         description: |
1365 1365
           A mapping of network name to endpoint configuration for that network.
1366
+          The endpoint configuration can be left empty to connect to that
1367
+          network with no particular endpoint configuration.
1366 1368
         type: "object"
1367 1369
         additionalProperties:
1368 1370
           $ref: "#/definitions/EndpointSettings"
1369 1371
     example:
1370 1372
       # putting an example here, instead of using the example values from
1371
-      # /definitions/EndpointSettings, because containers/create currently
1372
-      # does not support attaching to multiple networks, so the example request
1373
-      # would be confusing if it showed that multiple networks can be contained
1374
-      # in the EndpointsConfig.
1375
-      # TODO remove once we support multiple networks on container create (see https://github.com/moby/moby/blob/07e6b843594e061f82baa5fa23c2ff7d536c2a05/daemon/create.go#L323)
1373
+      # /definitions/EndpointSettings, because EndpointSettings contains
1374
+      # operational data returned when inspecting a container that we don't
1375
+      # accept here.
1376 1376
       EndpointsConfig:
1377 1377
         isolated_nw:
1378 1378
           IPAMConfig:
... ...
@@ -1387,6 +1387,7 @@ definitions:
1387 1387
           Aliases:
1388 1388
             - "server_x"
1389 1389
             - "server_y"
1390
+        database_nw: {}
1390 1391
 
1391 1392
   NetworkSettings:
1392 1393
     description: "NetworkSettings exposes the network settings in the API"
... ...
@@ -6439,6 +6440,7 @@ paths:
6439 6439
                     Aliases:
6440 6440
                       - "server_x"
6441 6441
                       - "server_y"
6442
+                  database_nw: {}
6442 6443
 
6443 6444
           required: true
6444 6445
       responses:
... ...
@@ -321,19 +321,11 @@ func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *i
321 321
 	return nil
322 322
 }
323 323
 
324
-// Checks if the client set configurations for more than one network while creating a container
325
-// Also checks if the IPAMConfig is valid
324
+// verifyNetworkingConfig validates if the nwConfig is valid.
326 325
 func verifyNetworkingConfig(nwConfig *networktypes.NetworkingConfig) error {
327
-	if nwConfig == nil || len(nwConfig.EndpointsConfig) == 0 {
326
+	if nwConfig == nil {
328 327
 		return nil
329 328
 	}
330
-	if len(nwConfig.EndpointsConfig) > 1 {
331
-		l := make([]string, 0, len(nwConfig.EndpointsConfig))
332
-		for k := range nwConfig.EndpointsConfig {
333
-			l = append(l, k)
334
-		}
335
-		return fmt.Errorf("container cannot be connected to network endpoints: %s", strings.Join(l, ", "))
336
-	}
337 329
 
338 330
 	for k, v := range nwConfig.EndpointsConfig {
339 331
 		if v == nil {
... ...
@@ -45,6 +45,8 @@ keywords: "API, Docker, rcli, REST, documentation"
45 45
   such, the `CheckDuplicate` field is now deprecated. Note that this change is
46 46
   _unversioned_ and applied to all API versions on daemon that support version
47 47
   1.44.
48
+* `POST /containers/create` now accepts multiple `EndpointSettings` in
49
+  `NetworkingConfig.EndpointSettings`.
48 50
 
49 51
 ## v1.43 API changes
50 52
 
... ...
@@ -556,33 +556,6 @@ func (s *DockerAPISuite) TestContainerAPICreateEmptyConfig(c *testing.T) {
556 556
 	assert.ErrorContains(c, err, "no command specified")
557 557
 }
558 558
 
559
-func (s *DockerAPISuite) TestContainerAPICreateMultipleNetworksConfig(c *testing.T) {
560
-	// Container creation must fail if client specified configurations for more than one network
561
-	config := container.Config{
562
-		Image: "busybox",
563
-	}
564
-
565
-	networkingConfig := network.NetworkingConfig{
566
-		EndpointsConfig: map[string]*network.EndpointSettings{
567
-			"net1": {},
568
-			"net2": {},
569
-			"net3": {},
570
-		},
571
-	}
572
-
573
-	apiClient, err := client.NewClientWithOpts(client.FromEnv)
574
-	assert.NilError(c, err)
575
-	defer apiClient.Close()
576
-
577
-	_, err = apiClient.ContainerCreate(testutil.GetContext(c), &config, &container.HostConfig{}, &networkingConfig, nil, "")
578
-	msg := err.Error()
579
-	// network name order in error message is not deterministic
580
-	assert.Assert(c, strings.Contains(msg, "container cannot be connected to network endpoints"))
581
-	assert.Assert(c, strings.Contains(msg, "net1"))
582
-	assert.Assert(c, strings.Contains(msg, "net2"))
583
-	assert.Assert(c, strings.Contains(msg, "net3"))
584
-}
585
-
586 559
 func (s *DockerAPISuite) TestContainerAPICreateBridgeNetworkMode(c *testing.T) {
587 560
 	// Windows does not support bridge
588 561
 	testRequires(c, DaemonIsLinux)
... ...
@@ -13,6 +13,7 @@ import (
13 13
 	containertypes "github.com/docker/docker/api/types/container"
14 14
 	"github.com/docker/docker/api/types/network"
15 15
 	"github.com/docker/docker/api/types/versions"
16
+	"github.com/docker/docker/client"
16 17
 	"github.com/docker/docker/errdefs"
17 18
 	ctr "github.com/docker/docker/integration/internal/container"
18 19
 	"github.com/docker/docker/oci"
... ...
@@ -585,3 +586,39 @@ func TestCreateInvalidHostConfig(t *testing.T) {
585 585
 		})
586 586
 	}
587 587
 }
588
+
589
+func TestCreateWithMultipleEndpointSettings(t *testing.T) {
590
+	defer setupTest(t)()
591
+	ctx := context.Background()
592
+
593
+	testcases := []struct {
594
+		apiVersion  string
595
+		expectedErr string
596
+	}{
597
+		{apiVersion: "1.44"},
598
+		{apiVersion: "1.43", expectedErr: "Container cannot be connected to network endpoints"},
599
+	}
600
+	for _, tc := range testcases {
601
+		t.Run("with API v"+tc.apiVersion, func(t *testing.T) {
602
+			apiClient, err := client.NewClientWithOpts(client.FromEnv, client.WithVersion(tc.apiVersion))
603
+			assert.NilError(t, err)
604
+
605
+			config := container.Config{
606
+				Image: "busybox",
607
+			}
608
+			networkingConfig := network.NetworkingConfig{
609
+				EndpointsConfig: map[string]*network.EndpointSettings{
610
+					"net1": {},
611
+					"net2": {},
612
+					"net3": {},
613
+				},
614
+			}
615
+			_, err = apiClient.ContainerCreate(ctx, &config, &container.HostConfig{}, &networkingConfig, nil, "")
616
+			if tc.expectedErr == "" {
617
+				assert.NilError(t, err)
618
+			} else {
619
+				assert.ErrorContains(t, err, tc.expectedErr)
620
+			}
621
+		})
622
+	}
623
+}
... ...
@@ -146,6 +146,17 @@ func WithIPv6(networkName, ip string) func(*TestContainerConfig) {
146 146
 	}
147 147
 }
148 148
 
149
+func WithEndpointSettings(nw string, config *network.EndpointSettings) func(*TestContainerConfig) {
150
+	return func(c *TestContainerConfig) {
151
+		if c.NetworkingConfig.EndpointsConfig == nil {
152
+			c.NetworkingConfig.EndpointsConfig = map[string]*network.EndpointSettings{}
153
+		}
154
+		if _, ok := c.NetworkingConfig.EndpointsConfig[nw]; !ok {
155
+			c.NetworkingConfig.EndpointsConfig[nw] = config
156
+		}
157
+	}
158
+}
159
+
149 160
 // WithLogDriver sets the log driver to use for the container
150 161
 func WithLogDriver(driver string) func(*TestContainerConfig) {
151 162
 	return func(c *TestContainerConfig) {
... ...
@@ -33,3 +33,10 @@ func CreateNoError(ctx context.Context, t *testing.T, client client.APIClient, n
33 33
 	assert.NilError(t, err)
34 34
 	return name
35 35
 }
36
+
37
+func RemoveNoError(ctx context.Context, t *testing.T, apiClient client.APIClient, name string) {
38
+	t.Helper()
39
+
40
+	err := apiClient.NetworkRemove(ctx, name)
41
+	assert.NilError(t, err)
42
+}
36 43
new file mode 100644
... ...
@@ -0,0 +1,45 @@
0
+package network
1
+
2
+import (
3
+	"context"
4
+	"strings"
5
+	"testing"
6
+	"time"
7
+
8
+	networktypes "github.com/docker/docker/api/types/network"
9
+	"github.com/docker/docker/api/types/versions"
10
+	ctr "github.com/docker/docker/integration/internal/container"
11
+	"github.com/docker/docker/integration/internal/network"
12
+	"gotest.tools/v3/assert"
13
+	"gotest.tools/v3/skip"
14
+)
15
+
16
+func TestCreateWithMultiNetworks(t *testing.T) {
17
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
18
+	skip.If(t, versions.LessThan(testEnv.DaemonAPIVersion(), "1.44"), "requires API v1.44")
19
+
20
+	ctx := setupTest(t)
21
+	apiClient := testEnv.APIClient()
22
+
23
+	network.CreateNoError(ctx, t, apiClient, "testnet1")
24
+	defer network.RemoveNoError(ctx, t, apiClient, "testnet1")
25
+
26
+	network.CreateNoError(ctx, t, apiClient, "testnet2")
27
+	defer network.RemoveNoError(ctx, t, apiClient, "testnet2")
28
+
29
+	attachCtx, cancel := context.WithTimeout(ctx, 1*time.Second)
30
+	defer cancel()
31
+	res := ctr.RunAttach(attachCtx, t, apiClient,
32
+		ctr.WithCmd("ip", "-o", "-4", "addr", "show"),
33
+		ctr.WithNetworkMode("testnet1"),
34
+		ctr.WithEndpointSettings("testnet1", &networktypes.EndpointSettings{}),
35
+		ctr.WithEndpointSettings("testnet2", &networktypes.EndpointSettings{}))
36
+
37
+	assert.Equal(t, res.ExitCode, 0)
38
+	assert.Equal(t, res.Stderr.String(), "")
39
+
40
+	// Only interfaces with an IPv4 address are printed by iproute2 when flag -4 is specified. Here, we should have two
41
+	// interfaces for testnet1 and testnet2, plus lo.
42
+	ifacesWithAddress := strings.Count(res.Stdout.String(), "\n")
43
+	assert.Equal(t, ifacesWithAddress, 3)
44
+}