浏览代码

Include all networks in ContainerCreate call if API >= 1.44

Previously, we included the container's primary network in the
ContainerCreate API call, and then connected the created container
to each extra network one-by-one, by calling NetworkConnect.

However, starting API version 1.44, the ContainerCreate endpoint now
takes multiple EndpointsConfigs, allowing us to just include all the
network configurations there and skip connecting the container to each
extra network separately.

Signed-off-by: Laura Brehm <[email protected]>
Laura Brehm 1 年之前
父节点
当前提交
aaa7ef6de5
共有 5 个文件被更改,包括 243 次插入29 次删除
  1. 4 2
      pkg/compose/compose.go
  2. 22 13
      pkg/compose/convergence.go
  3. 183 0
      pkg/compose/convergence_test.go
  4. 30 10
      pkg/compose/create.go
  5. 4 4
      pkg/compose/create_test.go

+ 4 - 2
pkg/compose/compose.go

@@ -312,11 +312,13 @@ func (s *composeService) isSWarmEnabled(ctx context.Context) (bool, error) {
 	return swarmEnabled.val, swarmEnabled.err
 	return swarmEnabled.val, swarmEnabled.err
 }
 }
 
 
-var runtimeVersion = struct {
+type runtimeVersionCache struct {
 	once sync.Once
 	once sync.Once
 	val  string
 	val  string
 	err  error
 	err  error
-}{}
+}
+
+var runtimeVersion runtimeVersionCache
 
 
 func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {
 func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {
 	runtimeVersion.once.Do(func() {
 	runtimeVersion.once.Do(func() {

+ 22 - 13
pkg/compose/convergence.go

@@ -34,6 +34,7 @@ import (
 	"github.com/docker/compose/v2/internal/tracing"
 	"github.com/docker/compose/v2/internal/tracing"
 	moby "github.com/docker/docker/api/types"
 	moby "github.com/docker/docker/api/types"
 	containerType "github.com/docker/docker/api/types/container"
 	containerType "github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/versions"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/sync/errgroup"
@@ -600,19 +601,27 @@ func (s *composeService) createMobyContainer(ctx context.Context,
 		},
 		},
 	}
 	}
 
 
-	// the highest-priority network is the primary and is included in the ContainerCreate API
-	// call via container.NetworkMode & network.NetworkingConfig
-	// any remaining networks are connected one-by-one here after creation (but before start)
-	serviceNetworks := service.NetworksByPriority()
-	for _, networkKey := range serviceNetworks {
-		mobyNetworkName := project.Networks[networkKey].Name
-		if string(cfgs.Host.NetworkMode) == mobyNetworkName {
-			// primary network already configured as part of ContainerCreate
-			continue
-		}
-		epSettings := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
-		if err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, created.ID, epSettings); err != nil {
-			return created, err
+	apiVersion, err := s.RuntimeVersion(ctx)
+	if err != nil {
+		return created, err
+	}
+	// Starting API version 1.44, the ContainerCreate API call takes multiple networks
+	// so we include all the configurations there and can skip the one-by-one calls here
+	if versions.LessThan(apiVersion, "1.44") {
+		// the highest-priority network is the primary and is included in the ContainerCreate API
+		// call via container.NetworkMode & network.NetworkingConfig
+		// any remaining networks are connected one-by-one here after creation (but before start)
+		serviceNetworks := service.NetworksByPriority()
+		for _, networkKey := range serviceNetworks {
+			mobyNetworkName := project.Networks[networkKey].Name
+			if string(cfgs.Host.NetworkMode) == mobyNetworkName {
+				// primary network already configured as part of ContainerCreate
+				continue
+			}
+			epSettings := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
+			if err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, created.ID, epSettings); err != nil {
+				return created, err
+			}
 		}
 		}
 	}
 	}
 
 

+ 183 - 0
pkg/compose/convergence_test.go

@@ -23,14 +23,18 @@ import (
 	"testing"
 	"testing"
 
 
 	"github.com/compose-spec/compose-go/v2/types"
 	"github.com/compose-spec/compose-go/v2/types"
+	"github.com/docker/cli/cli/config/configfile"
 	moby "github.com/docker/docker/api/types"
 	moby "github.com/docker/docker/api/types"
 	containerType "github.com/docker/docker/api/types/container"
 	containerType "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/api/types/network"
+	"github.com/docker/go-connections/nat"
 	"go.uber.org/mock/gomock"
 	"go.uber.org/mock/gomock"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/assert"
 
 
 	"github.com/docker/compose/v2/pkg/api"
 	"github.com/docker/compose/v2/pkg/api"
 	"github.com/docker/compose/v2/pkg/mocks"
 	"github.com/docker/compose/v2/pkg/mocks"
+	"github.com/docker/compose/v2/pkg/progress"
 )
 )
 
 
 func TestContainerName(t *testing.T) {
 func TestContainerName(t *testing.T) {
@@ -251,3 +255,182 @@ func TestWaitDependencies(t *testing.T) {
 		assert.NilError(t, tested.waitDependencies(context.Background(), &project, "", dependencies, nil))
 		assert.NilError(t, tested.waitDependencies(context.Background(), &project, "", dependencies, nil))
 	})
 	})
 }
 }
+
+func TestCreateMobyContainer(t *testing.T) {
+	t.Run("connects container networks one by one if API <1.44", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		defer mockCtrl.Finish()
+		apiClient := mocks.NewMockAPIClient(mockCtrl)
+		cli := mocks.NewMockCli(mockCtrl)
+		tested := composeService{
+			dockerCli: cli,
+		}
+		cli.EXPECT().Client().Return(apiClient).AnyTimes()
+		cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes()
+		apiClient.EXPECT().DaemonHost().Return("").AnyTimes()
+		apiClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(moby.ImageInspect{}, nil, nil).AnyTimes()
+		// force `RuntimeVersion` to fetch again
+		runtimeVersion = runtimeVersionCache{}
+		apiClient.EXPECT().ServerVersion(gomock.Any()).Return(moby.Version{
+			APIVersion: "1.43",
+		}, nil).AnyTimes()
+
+		service := types.ServiceConfig{
+			Name: "test",
+			Networks: map[string]*types.ServiceNetworkConfig{
+				"a": {
+					Priority: 10,
+				},
+				"b": {
+					Priority: 100,
+				},
+			},
+		}
+		project := types.Project{
+			Name: "bork",
+			Services: types.Services{
+				"test": service,
+			},
+			Networks: types.Networks{
+				"a": types.NetworkConfig{
+					Name: "a-moby-name",
+				},
+				"b": types.NetworkConfig{
+					Name: "b-moby-name",
+				},
+			},
+		}
+
+		var falseBool bool
+		apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
+			&containerType.HostConfig{
+				PortBindings: nat.PortMap{},
+				ExtraHosts:   []string{},
+				Tmpfs:        map[string]string{},
+				Resources: containerType.Resources{
+					OomKillDisable: &falseBool,
+				},
+				NetworkMode: "b-moby-name",
+			}), gomock.Eq(
+			&network.NetworkingConfig{
+				EndpointsConfig: map[string]*network.EndpointSettings{
+					"b-moby-name": {
+						IPAMConfig: &network.EndpointIPAMConfig{},
+						Aliases:    []string{"bork-test-0"},
+					},
+				},
+			}), gomock.Any(), gomock.Any()).Times(1).Return(
+			containerType.CreateResponse{
+				ID: "an-id",
+			}, nil)
+
+		apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id")).Times(1).Return(
+			moby.ContainerJSON{
+				ContainerJSONBase: &moby.ContainerJSONBase{
+					ID:   "an-id",
+					Name: "a-name",
+				},
+				Config:          &containerType.Config{},
+				NetworkSettings: &moby.NetworkSettings{},
+			}, nil)
+
+		apiClient.EXPECT().NetworkConnect(gomock.Any(), "a-moby-name", "an-id", gomock.Eq(
+			&network.EndpointSettings{
+				IPAMConfig: &network.EndpointIPAMConfig{},
+				Aliases:    []string{"bork-test-0"},
+			}))
+
+		_, err := tested.createMobyContainer(context.Background(), &project, service, "test", 0, nil, createOptions{
+			Labels: make(types.Labels),
+		}, progress.ContextWriter(context.TODO()))
+		assert.NilError(t, err)
+	})
+
+	t.Run("includes all container networks in ContainerCreate call if API >=1.44", func(t *testing.T) {
+		mockCtrl := gomock.NewController(t)
+		defer mockCtrl.Finish()
+		apiClient := mocks.NewMockAPIClient(mockCtrl)
+		cli := mocks.NewMockCli(mockCtrl)
+		tested := composeService{
+			dockerCli: cli,
+		}
+		cli.EXPECT().Client().Return(apiClient).AnyTimes()
+		cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes()
+		apiClient.EXPECT().DaemonHost().Return("").AnyTimes()
+		apiClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(moby.ImageInspect{}, nil, nil).AnyTimes()
+		// force `RuntimeVersion` to fetch fresh version
+		runtimeVersion = runtimeVersionCache{}
+		apiClient.EXPECT().ServerVersion(gomock.Any()).Return(moby.Version{
+			APIVersion: "1.44",
+		}, nil).AnyTimes()
+
+		service := types.ServiceConfig{
+			Name: "test",
+			Networks: map[string]*types.ServiceNetworkConfig{
+				"a": {
+					Priority: 10,
+				},
+				"b": {
+					Priority: 100,
+				},
+			},
+		}
+		project := types.Project{
+			Name: "bork",
+			Services: types.Services{
+				"test": service,
+			},
+			Networks: types.Networks{
+				"a": types.NetworkConfig{
+					Name: "a-moby-name",
+				},
+				"b": types.NetworkConfig{
+					Name: "b-moby-name",
+				},
+			},
+		}
+
+		var falseBool bool
+		apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
+			&containerType.HostConfig{
+				PortBindings: nat.PortMap{},
+				ExtraHosts:   []string{},
+				Tmpfs:        map[string]string{},
+				Resources: containerType.Resources{
+					OomKillDisable: &falseBool,
+				},
+				NetworkMode: "b-moby-name",
+			}), gomock.Eq(
+			&network.NetworkingConfig{
+				EndpointsConfig: map[string]*network.EndpointSettings{
+					"a-moby-name": {
+						IPAMConfig: &network.EndpointIPAMConfig{},
+						Aliases:    []string{"bork-test-0"},
+					},
+					"b-moby-name": {
+						IPAMConfig: &network.EndpointIPAMConfig{},
+						Aliases:    []string{"bork-test-0"},
+					},
+				},
+			}), gomock.Any(), gomock.Any()).Times(1).Return(
+			containerType.CreateResponse{
+				ID: "an-id",
+			}, nil)
+
+		apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id")).Times(1).Return(
+			moby.ContainerJSON{
+				ContainerJSONBase: &moby.ContainerJSONBase{
+					ID:   "an-id",
+					Name: "a-name",
+				},
+				Config:          &containerType.Config{},
+				NetworkSettings: &moby.NetworkSettings{},
+			}, nil)
+
+		_, err := tested.createMobyContainer(context.Background(), &project, service, "test", 0, nil, createOptions{
+			Labels: make(types.Labels),
+		}, progress.ContextWriter(context.TODO()))
+		assert.NilError(t, err)
+	})
+
+}

+ 30 - 10
pkg/compose/create.go

@@ -235,7 +235,11 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
 	if err != nil {
 	if err != nil {
 		return createConfigs{}, err
 		return createConfigs{}, err
 	}
 	}
-	networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases)
+	apiVersion, err := s.RuntimeVersion(ctx)
+	if err != nil {
+		return createConfigs{}, err
+	}
+	networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases, apiVersion)
 	portBindings := buildContainerPortBindingOptions(service)
 	portBindings := buildContainerPortBindingOptions(service)
 
 
 	// MISC
 	// MISC
@@ -456,6 +460,7 @@ func defaultNetworkSettings(
 	serviceIndex int,
 	serviceIndex int,
 	links []string,
 	links []string,
 	useNetworkAliases bool,
 	useNetworkAliases bool,
+	version string,
 ) (container.NetworkMode, *network.NetworkingConfig) {
 ) (container.NetworkMode, *network.NetworkingConfig) {
 	if service.NetworkMode != "" {
 	if service.NetworkMode != "" {
 		return container.NetworkMode(service.NetworkMode), nil
 		return container.NetworkMode(service.NetworkMode), nil
@@ -465,23 +470,38 @@ func defaultNetworkSettings(
 		return "none", nil
 		return "none", nil
 	}
 	}
 
 
-	var networkKey string
+	var primaryNetworkKey string
 	if len(service.Networks) > 0 {
 	if len(service.Networks) > 0 {
-		networkKey = service.NetworksByPriority()[0]
+		primaryNetworkKey = service.NetworksByPriority()[0]
 	} else {
 	} else {
-		networkKey = "default"
+		primaryNetworkKey = "default"
+	}
+	primaryNetworkMobyNetworkName := project.Networks[primaryNetworkKey].Name
+	endpointsConfig := map[string]*network.EndpointSettings{
+		primaryNetworkMobyNetworkName: createEndpointSettings(project, service, serviceIndex, primaryNetworkKey, links, useNetworkAliases),
+	}
+
+	// Starting from API version 1.44, the Engine will take several EndpointsConfigs
+	// so we can pass all the extra networks we want the container to be connected to
+	// in the network configuration instead of connecting the container to each extra
+	// network individually after creation.
+	if versions.GreaterThanOrEqualTo(version, "1.44") && len(service.Networks) > 1 {
+		serviceNetworks := service.NetworksByPriority()
+		for _, networkKey := range serviceNetworks[1:] {
+			mobyNetworkName := project.Networks[networkKey].Name
+			epSettings := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases)
+			endpointsConfig[mobyNetworkName] = epSettings
+		}
 	}
 	}
-	mobyNetworkName := project.Networks[networkKey].Name
-	epSettings := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases)
+
 	networkConfig := &network.NetworkingConfig{
 	networkConfig := &network.NetworkingConfig{
-		EndpointsConfig: map[string]*network.EndpointSettings{
-			mobyNetworkName: epSettings,
-		},
+		EndpointsConfig: endpointsConfig,
 	}
 	}
+
 	// From the Engine API docs:
 	// From the Engine API docs:
 	// > Supported standard values are: bridge, host, none, and container:<name|id>.
 	// > Supported standard values are: bridge, host, none, and container:<name|id>.
 	// > Any other value is taken as a custom network's name to which this container should connect to.
 	// > Any other value is taken as a custom network's name to which this container should connect to.
-	return container.NetworkMode(mobyNetworkName), networkConfig
+	return container.NetworkMode(primaryNetworkMobyNetworkName), networkConfig
 }
 }
 
 
 func getRestartPolicy(service types.ServiceConfig) container.RestartPolicy {
 func getRestartPolicy(service types.ServiceConfig) container.RestartPolicy {

+ 4 - 4
pkg/compose/create_test.go

@@ -193,7 +193,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
 			}),
 			}),
 		}
 		}
 
 
-		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
+		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
 		assert.Equal(t, string(networkMode), "myProject_myNetwork2")
 		assert.Equal(t, string(networkMode), "myProject_myNetwork2")
 		assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
 		assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
 		assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_myNetwork2"))
 		assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_myNetwork2"))
@@ -221,7 +221,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
 			}),
 			}),
 		}
 		}
 
 
-		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
+		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
 		assert.Equal(t, string(networkMode), "myProject_default")
 		assert.Equal(t, string(networkMode), "myProject_default")
 		assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
 		assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
 		assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_default"))
 		assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_default"))
@@ -238,7 +238,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
 			},
 			},
 		}
 		}
 
 
-		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
+		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
 		assert.Equal(t, string(networkMode), "none")
 		assert.Equal(t, string(networkMode), "none")
 		assert.Check(t, cmp.Nil(networkConfig))
 		assert.Check(t, cmp.Nil(networkConfig))
 	})
 	})
@@ -258,7 +258,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
 			}),
 			}),
 		}
 		}
 
 
-		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
+		networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
 		assert.Equal(t, string(networkMode), "host")
 		assert.Equal(t, string(networkMode), "host")
 		assert.Check(t, cmp.Nil(networkConfig))
 		assert.Check(t, cmp.Nil(networkConfig))
 	})
 	})