diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 0c6bf8067..9f33dbc3a 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -1070,6 +1070,7 @@ func (s *composeService) ensureNetwork(ctx context.Context, n types.NetworkConfi } } createOpts := moby.NetworkCreate{ + CheckDuplicate: true, // TODO NameSpace Labels Labels: n.Labels, Driver: n.Driver, @@ -1110,19 +1111,6 @@ func (s *composeService) ensureNetwork(ctx context.Context, n types.NetworkConfi return nil } -func (s *composeService) removeNetwork(ctx context.Context, network string, w progress.Writer) error { - eventName := fmt.Sprintf("Network %s", network) - w.Event(progress.RemovingEvent(eventName)) - - if err := s.apiClient().NetworkRemove(ctx, network); err != nil { - w.Event(progress.ErrorEvent(eventName)) - return errors.Wrapf(err, fmt.Sprintf("failed to remove network %s", network)) - } - - w.Event(progress.RemovedEvent(eventName)) - return nil -} - func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeConfig, project string) error { inspected, err := s.apiClient().VolumeInspect(ctx, volume.Name) if err != nil { diff --git a/pkg/compose/down.go b/pkg/compose/down.go index 0130f5a58..b04ac1087 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -23,9 +23,10 @@ import ( "time" "github.com/compose-spec/compose-go/types" - "github.com/docker/cli/cli/registry/client" moby "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" "github.com/docker/docker/errdefs" + "github.com/pkg/errors" "golang.org/x/sync/errgroup" "github.com/docker/compose/v2/pkg/api" @@ -131,12 +132,8 @@ func (s *composeService) ensureNetworksDown(ctx context.Context, project *types. if n.External.External { continue } + // loop capture variable for op closure networkName := n.Name - _, err := s.apiClient().NetworkInspect(ctx, networkName, moby.NetworkInspectOptions{}) - if client.IsNotFound(err) { - return nil - } - ops = append(ops, func() error { return s.removeNetwork(ctx, networkName, w) }) @@ -144,6 +141,50 @@ func (s *composeService) ensureNetworksDown(ctx context.Context, project *types. return ops } +func (s *composeService) removeNetwork(ctx context.Context, name string, w progress.Writer) error { + // networks are guaranteed to have unique IDs but NOT names, so it's + // possible to get into a situation where a compose down will fail with + // an error along the lines of: + // failed to remove network test: Error response from daemon: network test is ambiguous (2 matches found based on name) + // as a workaround here, the delete is done by ID after doing a list using + // the name as a filter (99.9% of the time this will return a single result) + networks, err := s.apiClient().NetworkList(ctx, moby.NetworkListOptions{ + Filters: filters.NewArgs(filters.Arg("name", name)), + }) + if err != nil { + return errors.Wrapf(err, fmt.Sprintf("failed to inspect network %s", name)) + } + if len(networks) == 0 { + return nil + } + + eventName := fmt.Sprintf("Network %s", name) + w.Event(progress.RemovingEvent(eventName)) + + var removed int + for _, net := range networks { + if err := s.apiClient().NetworkRemove(ctx, net.ID); err != nil { + if errdefs.IsNotFound(err) { + continue + } + w.Event(progress.ErrorEvent(eventName)) + return errors.Wrapf(err, fmt.Sprintf("failed to remove network %s", name)) + } + removed++ + } + + if removed == 0 { + // in practice, it's extremely unlikely for this to ever occur, as it'd + // mean the network was present when we queried at the start of this + // method but was then deleted by something else in the interim + w.Event(progress.NewEvent(eventName, progress.Done, "Warning: No resource found to remove")) + return nil + } + + w.Event(progress.RemovedEvent(eventName)) + return nil +} + func (s *composeService) getServiceImages(options api.DownOptions, project *types.Project) map[string]struct{} { images := map[string]struct{}{} for _, service := range project.Services { diff --git a/pkg/compose/down_test.go b/pkg/compose/down_test.go index c799975cc..0568c72ed 100644 --- a/pkg/compose/down_test.go +++ b/pkg/compose/down_test.go @@ -21,13 +21,14 @@ import ( "strings" "testing" - compose "github.com/docker/compose/v2/pkg/api" - "github.com/docker/compose/v2/pkg/mocks" moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/volume" "github.com/golang/mock/gomock" "gotest.tools/v3/assert" + + compose "github.com/docker/compose/v2/pkg/api" + "github.com/docker/compose/v2/pkg/mocks" ) func TestDown(t *testing.T) { @@ -59,8 +60,16 @@ func TestDown(t *testing.T) { api.EXPECT().ContainerRemove(gomock.Any(), "456", moby.ContainerRemoveOptions{Force: true}).Return(nil) api.EXPECT().ContainerRemove(gomock.Any(), "789", moby.ContainerRemoveOptions{Force: true}).Return(nil) - api.EXPECT().NetworkInspect(gomock.Any(), "myProject_default", moby.NetworkInspectOptions{}).Return(moby.NetworkResource{Name: "myProject_default"}, nil) - api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil) + // network names are not guaranteed to be unique, ensure Compose handles + // cleanup properly if duplicates are inadvertently created + api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{ + Filters: filters.NewArgs(filters.Arg("name", "myProject_default")), + }).Return([]moby.NetworkResource{ + {ID: "abc123", Name: "myProject_default"}, + {ID: "def456", Name: "myProject_default"}, + }, nil) + api.EXPECT().NetworkRemove(gomock.Any(), "abc123").Return(nil) + api.EXPECT().NetworkRemove(gomock.Any(), "def456").Return(nil) err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{}) assert.NilError(t, err) @@ -94,8 +103,10 @@ func TestDownRemoveOrphans(t *testing.T) { api.EXPECT().ContainerRemove(gomock.Any(), "789", moby.ContainerRemoveOptions{Force: true}).Return(nil) api.EXPECT().ContainerRemove(gomock.Any(), "321", moby.ContainerRemoveOptions{Force: true}).Return(nil) - api.EXPECT().NetworkInspect(gomock.Any(), "myProject_default", moby.NetworkInspectOptions{}).Return(moby.NetworkResource{Name: "myProject_default"}, nil) - api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil) + api.EXPECT().NetworkList(gomock.Any(), moby.NetworkListOptions{ + Filters: filters.NewArgs(filters.Arg("name", "myProject_default")), + }).Return([]moby.NetworkResource{{ID: "abc123", Name: "myProject_default"}}, nil) + api.EXPECT().NetworkRemove(gomock.Any(), "abc123").Return(nil) err := tested.Down(context.Background(), strings.ToLower(testProject), compose.DownOptions{RemoveOrphans: true}) assert.NilError(t, err)