Skip to content

Commit aaa7ef6

Browse files
laurazardndeloof
laurazard
authored and
ndeloof
committed
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]>
1 parent 6ef55a5 commit aaa7ef6

File tree

5 files changed

+243
-29
lines changed

5 files changed

+243
-29
lines changed

‎pkg/compose/compose.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,13 @@ func (s *composeService) isSWarmEnabled(ctx context.Context) (bool, error) {
312312
return swarmEnabled.val, swarmEnabled.err
313313
}
314314

315-
var runtimeVersion = struct {
315+
type runtimeVersionCache struct {
316316
once sync.Once
317317
val string
318318
err error
319-
}{}
319+
}
320+
321+
var runtimeVersion runtimeVersionCache
320322

321323
func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {
322324
runtimeVersion.once.Do(func() {

‎pkg/compose/convergence.go

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/docker/compose/v2/internal/tracing"
3535
moby "github.com/docker/docker/api/types"
3636
containerType "github.com/docker/docker/api/types/container"
37+
"github.com/docker/docker/api/types/versions"
3738
specs "github.com/opencontainers/image-spec/specs-go/v1"
3839
"github.com/sirupsen/logrus"
3940
"golang.org/x/sync/errgroup"
@@ -600,19 +601,27 @@ func (s *composeService) createMobyContainer(ctx context.Context,
600601
},
601602
}
602603

603-
// the highest-priority network is the primary and is included in the ContainerCreate API
604-
// call via container.NetworkMode & network.NetworkingConfig
605-
// any remaining networks are connected one-by-one here after creation (but before start)
606-
serviceNetworks := service.NetworksByPriority()
607-
for _, networkKey := range serviceNetworks {
608-
mobyNetworkName := project.Networks[networkKey].Name
609-
if string(cfgs.Host.NetworkMode) == mobyNetworkName {
610-
// primary network already configured as part of ContainerCreate
611-
continue
612-
}
613-
epSettings := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
614-
if err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, created.ID, epSettings); err != nil {
615-
return created, err
604+
apiVersion, err := s.RuntimeVersion(ctx)
605+
if err != nil {
606+
return created, err
607+
}
608+
// Starting API version 1.44, the ContainerCreate API call takes multiple networks
609+
// so we include all the configurations there and can skip the one-by-one calls here
610+
if versions.LessThan(apiVersion, "1.44") {
611+
// the highest-priority network is the primary and is included in the ContainerCreate API
612+
// call via container.NetworkMode & network.NetworkingConfig
613+
// any remaining networks are connected one-by-one here after creation (but before start)
614+
serviceNetworks := service.NetworksByPriority()
615+
for _, networkKey := range serviceNetworks {
616+
mobyNetworkName := project.Networks[networkKey].Name
617+
if string(cfgs.Host.NetworkMode) == mobyNetworkName {
618+
// primary network already configured as part of ContainerCreate
619+
continue
620+
}
621+
epSettings := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
622+
if err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, created.ID, epSettings); err != nil {
623+
return created, err
624+
}
616625
}
617626
}
618627

‎pkg/compose/convergence_test.go

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@ import (
2323
"testing"
2424

2525
"github.com/compose-spec/compose-go/v2/types"
26+
"github.com/docker/cli/cli/config/configfile"
2627
moby "github.com/docker/docker/api/types"
2728
containerType "github.com/docker/docker/api/types/container"
2829
"github.com/docker/docker/api/types/filters"
30+
"github.com/docker/docker/api/types/network"
31+
"github.com/docker/go-connections/nat"
2932
"go.uber.org/mock/gomock"
3033
"gotest.tools/v3/assert"
3134

3235
"github.com/docker/compose/v2/pkg/api"
3336
"github.com/docker/compose/v2/pkg/mocks"
37+
"github.com/docker/compose/v2/pkg/progress"
3438
)
3539

3640
func TestContainerName(t *testing.T) {
@@ -251,3 +255,182 @@ func TestWaitDependencies(t *testing.T) {
251255
assert.NilError(t, tested.waitDependencies(context.Background(), &project, "", dependencies, nil))
252256
})
253257
}
258+
259+
func TestCreateMobyContainer(t *testing.T) {
260+
t.Run("connects container networks one by one if API <1.44", func(t *testing.T) {
261+
mockCtrl := gomock.NewController(t)
262+
defer mockCtrl.Finish()
263+
apiClient := mocks.NewMockAPIClient(mockCtrl)
264+
cli := mocks.NewMockCli(mockCtrl)
265+
tested := composeService{
266+
dockerCli: cli,
267+
}
268+
cli.EXPECT().Client().Return(apiClient).AnyTimes()
269+
cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes()
270+
apiClient.EXPECT().DaemonHost().Return("").AnyTimes()
271+
apiClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(moby.ImageInspect{}, nil, nil).AnyTimes()
272+
// force `RuntimeVersion` to fetch again
273+
runtimeVersion = runtimeVersionCache{}
274+
apiClient.EXPECT().ServerVersion(gomock.Any()).Return(moby.Version{
275+
APIVersion: "1.43",
276+
}, nil).AnyTimes()
277+
278+
service := types.ServiceConfig{
279+
Name: "test",
280+
Networks: map[string]*types.ServiceNetworkConfig{
281+
"a": {
282+
Priority: 10,
283+
},
284+
"b": {
285+
Priority: 100,
286+
},
287+
},
288+
}
289+
project := types.Project{
290+
Name: "bork",
291+
Services: types.Services{
292+
"test": service,
293+
},
294+
Networks: types.Networks{
295+
"a": types.NetworkConfig{
296+
Name: "a-moby-name",
297+
},
298+
"b": types.NetworkConfig{
299+
Name: "b-moby-name",
300+
},
301+
},
302+
}
303+
304+
var falseBool bool
305+
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
306+
&containerType.HostConfig{
307+
PortBindings: nat.PortMap{},
308+
ExtraHosts: []string{},
309+
Tmpfs: map[string]string{},
310+
Resources: containerType.Resources{
311+
OomKillDisable: &falseBool,
312+
},
313+
NetworkMode: "b-moby-name",
314+
}), gomock.Eq(
315+
&network.NetworkingConfig{
316+
EndpointsConfig: map[string]*network.EndpointSettings{
317+
"b-moby-name": {
318+
IPAMConfig: &network.EndpointIPAMConfig{},
319+
Aliases: []string{"bork-test-0"},
320+
},
321+
},
322+
}), gomock.Any(), gomock.Any()).Times(1).Return(
323+
containerType.CreateResponse{
324+
ID: "an-id",
325+
}, nil)
326+
327+
apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id")).Times(1).Return(
328+
moby.ContainerJSON{
329+
ContainerJSONBase: &moby.ContainerJSONBase{
330+
ID: "an-id",
331+
Name: "a-name",
332+
},
333+
Config: &containerType.Config{},
334+
NetworkSettings: &moby.NetworkSettings{},
335+
}, nil)
336+
337+
apiClient.EXPECT().NetworkConnect(gomock.Any(), "a-moby-name", "an-id", gomock.Eq(
338+
&network.EndpointSettings{
339+
IPAMConfig: &network.EndpointIPAMConfig{},
340+
Aliases: []string{"bork-test-0"},
341+
}))
342+
343+
_, err := tested.createMobyContainer(context.Background(), &project, service, "test", 0, nil, createOptions{
344+
Labels: make(types.Labels),
345+
}, progress.ContextWriter(context.TODO()))
346+
assert.NilError(t, err)
347+
})
348+
349+
t.Run("includes all container networks in ContainerCreate call if API >=1.44", func(t *testing.T) {
350+
mockCtrl := gomock.NewController(t)
351+
defer mockCtrl.Finish()
352+
apiClient := mocks.NewMockAPIClient(mockCtrl)
353+
cli := mocks.NewMockCli(mockCtrl)
354+
tested := composeService{
355+
dockerCli: cli,
356+
}
357+
cli.EXPECT().Client().Return(apiClient).AnyTimes()
358+
cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes()
359+
apiClient.EXPECT().DaemonHost().Return("").AnyTimes()
360+
apiClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(moby.ImageInspect{}, nil, nil).AnyTimes()
361+
// force `RuntimeVersion` to fetch fresh version
362+
runtimeVersion = runtimeVersionCache{}
363+
apiClient.EXPECT().ServerVersion(gomock.Any()).Return(moby.Version{
364+
APIVersion: "1.44",
365+
}, nil).AnyTimes()
366+
367+
service := types.ServiceConfig{
368+
Name: "test",
369+
Networks: map[string]*types.ServiceNetworkConfig{
370+
"a": {
371+
Priority: 10,
372+
},
373+
"b": {
374+
Priority: 100,
375+
},
376+
},
377+
}
378+
project := types.Project{
379+
Name: "bork",
380+
Services: types.Services{
381+
"test": service,
382+
},
383+
Networks: types.Networks{
384+
"a": types.NetworkConfig{
385+
Name: "a-moby-name",
386+
},
387+
"b": types.NetworkConfig{
388+
Name: "b-moby-name",
389+
},
390+
},
391+
}
392+
393+
var falseBool bool
394+
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
395+
&containerType.HostConfig{
396+
PortBindings: nat.PortMap{},
397+
ExtraHosts: []string{},
398+
Tmpfs: map[string]string{},
399+
Resources: containerType.Resources{
400+
OomKillDisable: &falseBool,
401+
},
402+
NetworkMode: "b-moby-name",
403+
}), gomock.Eq(
404+
&network.NetworkingConfig{
405+
EndpointsConfig: map[string]*network.EndpointSettings{
406+
"a-moby-name": {
407+
IPAMConfig: &network.EndpointIPAMConfig{},
408+
Aliases: []string{"bork-test-0"},
409+
},
410+
"b-moby-name": {
411+
IPAMConfig: &network.EndpointIPAMConfig{},
412+
Aliases: []string{"bork-test-0"},
413+
},
414+
},
415+
}), gomock.Any(), gomock.Any()).Times(1).Return(
416+
containerType.CreateResponse{
417+
ID: "an-id",
418+
}, nil)
419+
420+
apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id")).Times(1).Return(
421+
moby.ContainerJSON{
422+
ContainerJSONBase: &moby.ContainerJSONBase{
423+
ID: "an-id",
424+
Name: "a-name",
425+
},
426+
Config: &containerType.Config{},
427+
NetworkSettings: &moby.NetworkSettings{},
428+
}, nil)
429+
430+
_, err := tested.createMobyContainer(context.Background(), &project, service, "test", 0, nil, createOptions{
431+
Labels: make(types.Labels),
432+
}, progress.ContextWriter(context.TODO()))
433+
assert.NilError(t, err)
434+
})
435+
436+
}

‎pkg/compose/create.go

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,11 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
235235
if err != nil {
236236
return createConfigs{}, err
237237
}
238-
networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases)
238+
apiVersion, err := s.RuntimeVersion(ctx)
239+
if err != nil {
240+
return createConfigs{}, err
241+
}
242+
networkMode, networkingConfig := defaultNetworkSettings(p, service, number, links, opts.UseNetworkAliases, apiVersion)
239243
portBindings := buildContainerPortBindingOptions(service)
240244

241245
// MISC
@@ -456,6 +460,7 @@ func defaultNetworkSettings(
456460
serviceIndex int,
457461
links []string,
458462
useNetworkAliases bool,
463+
version string,
459464
) (container.NetworkMode, *network.NetworkingConfig) {
460465
if service.NetworkMode != "" {
461466
return container.NetworkMode(service.NetworkMode), nil
@@ -465,23 +470,38 @@ func defaultNetworkSettings(
465470
return "none", nil
466471
}
467472

468-
var networkKey string
473+
var primaryNetworkKey string
469474
if len(service.Networks) > 0 {
470-
networkKey = service.NetworksByPriority()[0]
475+
primaryNetworkKey = service.NetworksByPriority()[0]
471476
} else {
472-
networkKey = "default"
477+
primaryNetworkKey = "default"
478+
}
479+
primaryNetworkMobyNetworkName := project.Networks[primaryNetworkKey].Name
480+
endpointsConfig := map[string]*network.EndpointSettings{
481+
primaryNetworkMobyNetworkName: createEndpointSettings(project, service, serviceIndex, primaryNetworkKey, links, useNetworkAliases),
482+
}
483+
484+
// Starting from API version 1.44, the Engine will take several EndpointsConfigs
485+
// so we can pass all the extra networks we want the container to be connected to
486+
// in the network configuration instead of connecting the container to each extra
487+
// network individually after creation.
488+
if versions.GreaterThanOrEqualTo(version, "1.44") && len(service.Networks) > 1 {
489+
serviceNetworks := service.NetworksByPriority()
490+
for _, networkKey := range serviceNetworks[1:] {
491+
mobyNetworkName := project.Networks[networkKey].Name
492+
epSettings := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases)
493+
endpointsConfig[mobyNetworkName] = epSettings
494+
}
473495
}
474-
mobyNetworkName := project.Networks[networkKey].Name
475-
epSettings := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases)
496+
476497
networkConfig := &network.NetworkingConfig{
477-
EndpointsConfig: map[string]*network.EndpointSettings{
478-
mobyNetworkName: epSettings,
479-
},
498+
EndpointsConfig: endpointsConfig,
480499
}
500+
481501
// From the Engine API docs:
482502
// > Supported standard values are: bridge, host, none, and container:<name|id>.
483503
// > Any other value is taken as a custom network's name to which this container should connect to.
484-
return container.NetworkMode(mobyNetworkName), networkConfig
504+
return container.NetworkMode(primaryNetworkMobyNetworkName), networkConfig
485505
}
486506

487507
func getRestartPolicy(service types.ServiceConfig) container.RestartPolicy {

‎pkg/compose/create_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
193193
}),
194194
}
195195

196-
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
196+
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
197197
assert.Equal(t, string(networkMode), "myProject_myNetwork2")
198198
assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
199199
assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_myNetwork2"))
@@ -221,7 +221,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
221221
}),
222222
}
223223

224-
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
224+
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
225225
assert.Equal(t, string(networkMode), "myProject_default")
226226
assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1))
227227
assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_default"))
@@ -238,7 +238,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
238238
},
239239
}
240240

241-
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
241+
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
242242
assert.Equal(t, string(networkMode), "none")
243243
assert.Check(t, cmp.Nil(networkConfig))
244244
})
@@ -258,7 +258,7 @@ func TestDefaultNetworkSettings(t *testing.T) {
258258
}),
259259
}
260260

261-
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true)
261+
networkMode, networkConfig := defaultNetworkSettings(&project, service, 1, nil, true, "1.43")
262262
assert.Equal(t, string(networkMode), "host")
263263
assert.Check(t, cmp.Nil(networkConfig))
264264
})

0 commit comments

Comments
 (0)