From a97738de7dd9d14fcf98b4ef3528c417dabe77d3 Mon Sep 17 00:00:00 2001 From: jarek Date: Tue, 31 Mar 2026 15:04:52 +0200 Subject: [PATCH] fix: add NetworkConnect fallback for API < 1.44 For Docker daemons older than API 1.44, the extra networks omitted from ContainerCreate must be connected individually after creation via NetworkConnect. If any NetworkConnect call fails, remove the freshly created container to prevent orphans. Add two tests: - TestCreateMobyContainerLegacyAPI: happy path verifying NetworkConnect is called for the secondary network on API 1.43 - TestCreateMobyContainerLegacyAPI_NetworkConnectFailure: verifies ContainerRemove is called with Force when NetworkConnect fails Signed-off-by: jarek --- pkg/compose/convergence.go | 32 +++++++ pkg/compose/convergence_test.go | 146 ++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index b480b6be9..973699e89 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 27d312c0c..2f7c31cf8 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()