From 02cd50ce3aa4c680246813c555137b94effc8940 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Tue, 30 Jan 2024 17:41:19 +0000 Subject: [PATCH] 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 --- pkg/compose/convergence.go | 35 ++++--- pkg/compose/convergence_test.go | 179 ++++++++++++++++++++++++++++++++ pkg/compose/create.go | 40 +++++-- pkg/compose/create_test.go | 8 +- 4 files changed, 235 insertions(+), 27 deletions(-) diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index f65247b086b..eb97a1d8fb0 100644 --- a/pkg/compose/convergence.go +++ b/pkg/compose/convergence.go @@ -34,6 +34,7 @@ import ( "github.com/docker/compose/v2/internal/tracing" moby "github.com/docker/docker/api/types" containerType "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/versions" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "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 + versionInfo, err := s.apiClient().ServerVersion(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(versionInfo.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 + } } } diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index 32917cb6610..c4dd239a7cd 100644 --- a/pkg/compose/convergence_test.go +++ b/pkg/compose/convergence_test.go @@ -23,14 +23,18 @@ import ( "testing" "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/config/configfile" moby "github.com/docker/docker/api/types" containerType "github.com/docker/docker/api/types/container" "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" "gotest.tools/v3/assert" "github.com/docker/compose/v2/pkg/api" "github.com/docker/compose/v2/pkg/mocks" + "github.com/docker/compose/v2/pkg/progress" ) func TestContainerName(t *testing.T) { @@ -251,3 +255,178 @@ func TestWaitDependencies(t *testing.T) { 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() + 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() + 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) + }) + +} diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 069db7cfc70..cbb4351287b 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -235,7 +235,11 @@ func (s *composeService) getCreateConfigs(ctx context.Context, if err != nil { return createConfigs{}, err } - networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases) + versionInfo, err := s.apiClient().ServerVersion(ctx) + if err != nil { + return createConfigs{}, err + } + networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases, versionInfo) portBindings := buildContainerPortBindingOptions(service) // MISC @@ -456,6 +460,7 @@ func defaultNetworkSettings( serviceIndex int, links []string, useNetworkAliases bool, + version moby.Version, ) (container.NetworkMode, *network.NetworkingConfig) { if service.NetworkMode != "" { return container.NetworkMode(service.NetworkMode), nil @@ -465,23 +470,38 @@ func defaultNetworkSettings( return "none", nil } - var networkKey string + var primaryNetworkKey string if len(service.Networks) > 0 { - networkKey = service.NetworksByPriority()[0] + primaryNetworkKey = service.NetworksByPriority()[0] } 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.APIVersion, "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{ - EndpointsConfig: map[string]*network.EndpointSettings{ - mobyNetworkName: epSettings, - }, + EndpointsConfig: endpointsConfig, } + // From the Engine API docs: // > Supported standard values are: bridge, host, none, and container:. // > 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 { diff --git a/pkg/compose/create_test.go b/pkg/compose/create_test.go index 1a47348960a..3a00768b1ed 100644 --- a/pkg/compose/create_test.go +++ b/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, moby.Version{APIVersion: "1.43"}) assert.Equal(t, string(networkMode), "myProject_myNetwork2") assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1)) 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, moby.Version{APIVersion: "1.43"}) assert.Equal(t, string(networkMode), "myProject_default") assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1)) 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, moby.Version{APIVersion: "1.43"}) assert.Equal(t, string(networkMode), "none") 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, moby.Version{APIVersion: "1.43"}) assert.Equal(t, string(networkMode), "host") assert.Check(t, cmp.Nil(networkConfig)) })