mirror of
https://github.com/docker/compose.git
synced 2026-05-13 13:58:02 +00:00
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 <jkrochmalski@gmail.com>
This commit is contained in:
jarek
Guillaume Lours
parent
3b1004c4d9
commit
a97738de7d
2 changed files with 178 additions and 0 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue