Skip to content

Commit

Permalink
Include all networks in ContainerCreate call if API >= 1.44
Browse files Browse the repository at this point in the history
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 <laurabrehm@hey.com>
  • Loading branch information
laurazard committed Jan 30, 2024
1 parent 8fdd45c commit 39a5531
Show file tree
Hide file tree
Showing 4 changed files with 236 additions and 27 deletions.
35 changes: 22 additions & 13 deletions pkg/compose/convergence.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Check warning on line 607 in pkg/compose/convergence.go

View check run for this annotation

Codecov / codecov/patch

pkg/compose/convergence.go#L606-L607

Added lines #L606 - L607 were not covered by tests
// 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
}

Check warning on line 624 in pkg/compose/convergence.go

View check run for this annotation

Codecov / codecov/patch

pkg/compose/convergence.go#L623-L624

Added lines #L623 - L624 were not covered by tests
}
}

Expand Down
180 changes: 180 additions & 0 deletions pkg/compose/convergence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,19 @@ import (
"testing"

"github.com/compose-spec/compose-go/v2/types"
"github.com/docker/cli/cli/config/configfile"
moby "github.com/docker/docker/api/types"
"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/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) {
Expand Down Expand Up @@ -251,3 +256,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(
&container.HostConfig{
PortBindings: nat.PortMap{},
ExtraHosts: []string{},
Tmpfs: map[string]string{},
Resources: container.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(
container.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(
&container.HostConfig{
PortBindings: nat.PortMap{},
ExtraHosts: []string{},
Tmpfs: map[string]string{},
Resources: container.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(
container.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)
})

}
40 changes: 30 additions & 10 deletions pkg/compose/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Check warning on line 241 in pkg/compose/create.go

View check run for this annotation

Codecov / codecov/patch

pkg/compose/create.go#L240-L241

Added lines #L240 - L241 were not covered by tests
networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases, versionInfo)
portBindings := buildContainerPortBindingOptions(service)

// MISC
Expand Down Expand Up @@ -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
Expand All @@ -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:<name|id>.
// > 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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/compose/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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"))
Expand All @@ -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))
})
Expand All @@ -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))
})
Expand Down

0 comments on commit 39a5531

Please sign in to comment.