Skip to content

Commit 1e13ed6

Browse files
committed
fix: clean up container on NetworkConnect failure to prevent orphans
When NetworkConnect fails for a secondary network on API < 1.44, remove the just-created container before returning the error. This prevents orphan containers that cause name conflicts on retry. Signed-off-by: jarek <jkrochmalski@gmail.com>
1 parent 31495e4 commit 1e13ed6

2 files changed

Lines changed: 73 additions & 4 deletions

File tree

pkg/compose/convergence.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,12 +751,16 @@ func (s *composeService) createMobyContainer(ctx context.Context, project *types
751751
}
752752
epSettings, err := createEndpointSettings(project, service, number, networkKey, cfgs.Links, opts.UseNetworkAliases)
753753
if err != nil {
754+
// Clean up the container we just created to avoid orphans
755+
_, _ = s.apiClient().ContainerRemove(ctx, response.ID, client.ContainerRemoveOptions{Force: true})
754756
return created, err
755757
}
756758
if _, err := s.apiClient().NetworkConnect(ctx, mobyNetworkName, client.NetworkConnectOptions{
757759
Container: response.ID,
758760
EndpointConfig: epSettings,
759761
}); err != nil {
762+
// Clean up the container we just created to avoid orphans
763+
_, _ = s.apiClient().ContainerRemove(ctx, response.ID, client.ContainerRemoveOptions{Force: true})
760764
return created, err
761765
}
762766
}

pkg/compose/convergence_test.go

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -552,10 +552,11 @@ func TestCreateMobyContainerLegacyAPI(t *testing.T) {
552552
// For API < 1.44, the secondary network "a" should be connected via NetworkConnect.
553553
// Using gomock.InOrder to verify NetworkConnect is called before ContainerInspect.
554554
var gotConnect client.NetworkConnectOptions
555-
connectCall := apiClient.EXPECT().NetworkConnect(gomock.Any(), gomock.Eq("a-moby-name"), gomock.Any()).DoAndReturn(func(_ context.Context, _ string, opts client.NetworkConnectOptions) (client.NetworkConnectResult, error) {
556-
gotConnect = opts
557-
return client.NetworkConnectResult{}, nil
558-
})
555+
connectCall := apiClient.EXPECT().NetworkConnect(gomock.Any(), gomock.Eq("a-moby-name"), gomock.Any()).
556+
DoAndReturn(func(_ context.Context, _ string, opts client.NetworkConnectOptions) (client.NetworkConnectResult, error) {
557+
gotConnect = opts
558+
return client.NetworkConnectResult{}, nil
559+
})
559560

560561
apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Eq("an-id"), gomock.Any()).Times(1).After(connectCall).Return(client.ContainerInspectResult{
561562
Container: container.InspectResponse{
@@ -593,3 +594,67 @@ func TestCreateMobyContainerLegacyAPI(t *testing.T) {
593594
assert.Equal(t, gotConnect.Container, "an-id")
594595
assert.Check(t, gotConnect.EndpointConfig != nil)
595596
}
597+
598+
func TestCreateMobyContainerLegacyAPI_NetworkConnectFailure(t *testing.T) {
599+
mockCtrl := gomock.NewController(t)
600+
defer mockCtrl.Finish()
601+
apiClient := mocks.NewMockAPIClient(mockCtrl)
602+
cli := mocks.NewMockCli(mockCtrl)
603+
tested, err := NewComposeService(cli)
604+
assert.NilError(t, err)
605+
cli.EXPECT().Client().Return(apiClient).AnyTimes()
606+
cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes()
607+
apiClient.EXPECT().DaemonHost().Return("").AnyTimes()
608+
apiClient.EXPECT().ImageInspect(anyCancellableContext(), gomock.Any()).Return(client.ImageInspectResult{}, nil).AnyTimes()
609+
610+
// force `RuntimeVersion` to return a pre-1.44 API version
611+
runtimeVersion = runtimeVersionCache{}
612+
apiClient.EXPECT().ServerVersion(gomock.Any(), gomock.Any()).Return(client.ServerVersionResult{
613+
APIVersion: "1.43",
614+
}, nil).AnyTimes()
615+
616+
service := types.ServiceConfig{
617+
Name: "test",
618+
Networks: map[string]*types.ServiceNetworkConfig{
619+
"a": {
620+
Priority: 10,
621+
},
622+
"b": {
623+
Priority: 100,
624+
},
625+
},
626+
}
627+
project := types.Project{
628+
Name: "bork",
629+
Services: types.Services{
630+
"test": service,
631+
},
632+
Networks: types.Networks{
633+
"a": types.NetworkConfig{
634+
Name: "a-moby-name",
635+
},
636+
"b": types.NetworkConfig{
637+
Name: "b-moby-name",
638+
},
639+
},
640+
}
641+
642+
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).Return(client.ContainerCreateResult{ID: "an-id"}, nil)
643+
644+
// NetworkConnect fails
645+
connectErr := fmt.Errorf("network connect failed")
646+
apiClient.EXPECT().NetworkConnect(gomock.Any(), gomock.Eq("a-moby-name"), gomock.Any()).Return(client.NetworkConnectResult{}, connectErr)
647+
648+
// ContainerRemove should be called to clean up the orphan container
649+
apiClient.EXPECT().ContainerRemove(gomock.Any(), gomock.Eq("an-id"), gomock.Any()).
650+
DoAndReturn(func(_ context.Context, _ string, opts client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) {
651+
assert.Check(t, opts.Force, "ContainerRemove should use Force")
652+
return client.ContainerRemoveResult{}, nil
653+
})
654+
655+
_, err = tested.(*composeService).createMobyContainer(t.Context(), &project, service, "test", 0, nil, createOptions{
656+
Labels: make(types.Labels),
657+
UseNetworkAliases: true,
658+
})
659+
assert.ErrorContains(t, err, "network connect failed")
660+
}

0 commit comments

Comments
 (0)