diff --git a/pkg/compose/api_versions.go b/pkg/compose/api_versions.go index bccb4ad277..d39aef8aac 100644 --- a/pkg/compose/api_versions.go +++ b/pkg/compose/api_versions.go @@ -19,6 +19,16 @@ package compose // Docker Engine API version constants. // These versions correspond to specific Docker Engine releases and their features. const ( + // apiVersion144 represents Docker Engine API version 1.44 (Engine v25.0). + // + // New features in this version: + // - ContainerCreate API accepts multiple EndpointsConfig entries + // + // Before this version: + // - Only a single EndpointsConfig entry was accepted in ContainerCreate + // - Extra networks must be connected individually after container creation via NetworkConnect + apiVersion144 = "1.44" + // apiVersion148 represents Docker Engine API version 1.48 (Engine v28.0). // // New features in this version: diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index b480b6be9d..973699e894 100644 --- a/pkg/compose/convergence.go +++ b/pkg/compose/convergence.go @@ -33,6 +33,7 @@ import ( "github.com/moby/moby/api/types/container" mmount "github.com/moby/moby/api/types/mount" "github.com/moby/moby/client" + "github.com/moby/moby/client/pkg/versions" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/attribute" @@ -732,6 +733,37 @@ func (s *composeService) createMobyContainer(ctx context.Context, project *types Text: warning, }) } + // Starting API version 1.44, the ContainerCreate API call takes multiple networks + // so we include all configurations there and can skip the one-by-one calls here. + // For older API versions (e.g. Docker 20.10/API 1.41, Synology DSM 7.1/7.2), + // extra networks must be connected individually after creation via NetworkConnect. + apiVersion, err := s.RuntimeAPIVersion(ctx) + if err != nil { + return created, err + } + if versions.LessThan(apiVersion, apiVersion144) { + 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, err := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases) + if err != nil { + _, _ = s.apiClient().ContainerRemove(ctx, response.ID, client.ContainerRemoveOptions{Force: true}) + return created, err + } + if _, err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, client.NetworkConnectOptions{ + Container: response.ID, + EndpointConfig: epSettings, + }); err != nil { + _, _ = s.apiClient().ContainerRemove(ctx, response.ID, client.ContainerRemoveOptions{Force: true}) + return created, err + } + } + } + res, err := s.apiClient().ContainerInspect(ctx, response.ID, client.ContainerInspectOptions{}) if err != nil { return created, err diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index 27d312c0cb..2f7c31cf81 100644 --- a/pkg/compose/convergence_test.go +++ b/pkg/compose/convergence_test.go @@ -498,6 +498,152 @@ func TestCreateMobyContainer(t *testing.T) { assert.NilError(t, err) } +func TestCreateMobyContainerLegacyAPI(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes() + apiClient.EXPECT().DaemonHost().Return("").AnyTimes() + apiClient.EXPECT().ImageInspect(anyCancellableContext(), gomock.Any()). + Return(client.ImageInspectResult{}, nil).AnyTimes() + + apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). + Return(client.PingResult{APIVersion: "1.43"}, nil).AnyTimes() + apiClient.EXPECT().ClientVersion().Return("1.43").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 gotCreate client.ContainerCreateOptions + apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + gotCreate = opts + return client.ContainerCreateResult{ID: "an-id"}, nil + }) + + // For API < 1.44, the secondary network "a" should be connected via NetworkConnect. + var gotConnect client.NetworkConnectOptions + connectCall := apiClient.EXPECT(). + NetworkConnect(gomock.Any(), gomock.Eq("a-moby-name"), gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, opts client.NetworkConnectOptions) (client.NetworkConnectResult, error) { + gotConnect = opts + return client.NetworkConnectResult{}, nil + }) + + apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id"), gomock.Any()). + Times(1).After(connectCall).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: "an-id", + Name: "a-name", + Config: &container.Config{}, + NetworkSettings: &container.NetworkSettings{ + Networks: map[string]*network.EndpointSettings{ + "b-moby-name": { + IPAMConfig: &network.EndpointIPAMConfig{}, + Aliases: []string{"bork-test-0"}, + }, + "a-moby-name": { + IPAMConfig: &network.EndpointIPAMConfig{}, + Aliases: []string{"bork-test-0"}, + }, + }, + }, + }, + }, nil) + + _, err = tested.(*composeService).createMobyContainer(t.Context(), &project, service, "test", 0, nil, createOptions{ + Labels: make(types.Labels), + UseNetworkAliases: true, + }) + assert.NilError(t, err) + + // ContainerCreate should only have the primary network (b, highest priority) + assert.Check(t, gotCreate.NetworkingConfig != nil) + assert.Equal(t, len(gotCreate.NetworkingConfig.EndpointsConfig), 1) + _, hasPrimary := gotCreate.NetworkingConfig.EndpointsConfig["b-moby-name"] + assert.Check(t, hasPrimary, "primary network b-moby-name should be in ContainerCreate EndpointsConfig") + + // NetworkConnect should have been called for the secondary network "a" + assert.Equal(t, gotConnect.Container, "an-id") + assert.Check(t, gotConnect.EndpointConfig != nil) +} + +func TestCreateMobyContainerLegacyAPI_NetworkConnectFailure(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes() + apiClient.EXPECT().DaemonHost().Return("").AnyTimes() + apiClient.EXPECT().ImageInspect(anyCancellableContext(), gomock.Any()). + Return(client.ImageInspectResult{}, nil).AnyTimes() + + apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). + Return(client.PingResult{APIVersion: "1.43"}, nil).AnyTimes() + apiClient.EXPECT().ClientVersion().Return("1.43").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"}, + }, + } + + apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()). + Return(client.ContainerCreateResult{ID: "an-id"}, nil) + + // NetworkConnect fails + connectErr := fmt.Errorf("network connect failed") + apiClient.EXPECT().NetworkConnect(gomock.Any(), gomock.Eq("a-moby-name"), gomock.Any()). + Return(client.NetworkConnectResult{}, connectErr) + + // ContainerRemove should be called to clean up the orphan container + apiClient.EXPECT().ContainerRemove(gomock.Any(), gomock.Eq("an-id"), gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, opts client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + assert.Check(t, opts.Force, "ContainerRemove should use Force") + return client.ContainerRemoveResult{}, nil + }) + + _, err = tested.(*composeService).createMobyContainer(t.Context(), &project, service, "test", 0, nil, createOptions{ + Labels: make(types.Labels), + UseNetworkAliases: true, + }) + assert.ErrorContains(t, err, "network connect failed") +} + func TestRuntimeAPIVersionCachesNegotiation(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 78db3d0fd8..cc0b4afbce 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -563,13 +563,17 @@ func defaultNetworkSettings(project *types.Project, // 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. - for _, networkKey := range serviceNetworks { - epSettings, err := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases) - if err != nil { - return "", nil, err + // For older API versions, extra networks are connected via NetworkConnect after + // container creation (see createMobyContainer in convergence.go). + if !versions.LessThan(version, apiVersion144) { + for _, networkKey := range serviceNetworks { + epSettings, err := createEndpointSettings(project, service, serviceIndex, networkKey, links, useNetworkAliases) + if err != nil { + return "", nil, err + } + mobyNetworkName := project.Networks[networkKey].Name + endpointsConfig[mobyNetworkName] = epSettings } - mobyNetworkName := project.Networks[networkKey].Name - endpointsConfig[mobyNetworkName] = epSettings } networkConfig := &network.NetworkingConfig{ diff --git a/pkg/compose/create_test.go b/pkg/compose/create_test.go index 2ac5c392be..e08e4227da 100644 --- a/pkg/compose/create_test.go +++ b/pkg/compose/create_test.go @@ -273,6 +273,30 @@ func TestDefaultNetworkSettings(t *testing.T) { assert.Check(t, cmp.Nil(networkConfig)) }) + t.Run("returns only primary network in EndpointsConfig for API < 1.44", func(t *testing.T) { + service := composetypes.ServiceConfig{ + Name: "myService", + Networks: map[string]*composetypes.ServiceNetworkConfig{ + "myNetwork1": {Priority: 10}, + "myNetwork2": {Priority: 1000}, + }, + } + project := composetypes.Project{ + Name: "myProject", + Services: composetypes.Services{"myService": service}, + Networks: composetypes.Networks(map[string]composetypes.NetworkConfig{ + "myNetwork1": {Name: "myProject_myNetwork1"}, + "myNetwork2": {Name: "myProject_myNetwork2"}, + }), + } + + networkMode, networkConfig, err := defaultNetworkSettings(&project, service, 1, nil, true, "1.43") + assert.NilError(t, err) + assert.Equal(t, string(networkMode), "myProject_myNetwork2") + assert.Check(t, cmp.Len(networkConfig.EndpointsConfig, 1)) + assert.Check(t, cmp.Contains(networkConfig.EndpointsConfig, "myProject_myNetwork2")) + }) + t.Run("returns defined network mode if explicitly set", func(t *testing.T) { service := composetypes.ServiceConfig{ Name: "myService",