networks: prevent issues due to duplicate names

Within the Docker API/engine, networks have unique IDs, and the
name is a friendly label/alias, which notably does NOT have any
guarantees around uniqueness (see moby/moby#18864 [^1]).

During day-to-day interactive/CLI Compose usage, this is rarely
an issue, as Compose itself isn't creating networks concurrently
across goroutines. However, if multiple Compose instances are
executed simultaneously (e.g. as part of a test suite that runs
in parallel), this can easily occur.

When it does happen, it's very confusing for users and resolving
it via the `docker` CLI is not straightforward either [^2].

There's two primary changes here:
 * Pass `CheckDuplicates: true` to the Docker API when creating
   networks to reduce the likelihood of Compose creating duplicates
   in the first place
 * On `down`, list networks using a name filter and then remove
   them all by ID, as the Docker API will return an error if the
   name alias is used and maps to more than one network

Hopefully, this provides a better UX, since the issue should be
less likely to occur, and if it does, it can now be resolved via
standard Compose workflow commands.

[^1]: moby/moby#18864
[^2]: https://stackoverflow.com/questions/63776518/error-2-matches-found-based-on-name-network-nameofservice-default-is-ambiguo

Signed-off-by: Milas Bowman <[email protected]>

Author: milas

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 {

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,19 +132,59 @@ 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)
 		})
 	}
 	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 {

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)