Return error in case docker network inspect is ambiguous

This fix is partially based on comment
https://github.com/docker/docker/issues/30242#issuecomment-273517205

Currently, `docker network inspect` relies on `FindNetwork()` which
does not take into consideration that multiple networks with the same
name might exist.

This fix propose to return `docker network inspect` in a similiar
fashion like other commands:
1. Lookup full ID
2. Lookup full name
3. Lookup partial ID
If multiple networks exist, an error will be returned.

NOTE: this fix is not a complete fix for the issue raised in
https://github.com/docker/docker/issues/30242#issuecomment-273517205
where SwarmKit is unable to update when multiple networks with the same
name exit.
To fix that issue requires multiple places when `FindNetwork()` is called.
Because of the impact of changing `FindNetwork()`, this fix focus on
the issue in `docker network inspect`.

A separate PR will be created to address
https://github.com/docker/docker/issues/30242#issuecomment-273517205

An integration test has been added.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

api/server/router/network/backend.go

@@ -11,8 +11,6 @@ import (
 // to provide network specific functionality.
 type Backend interface {
 	FindNetwork(idName string) (libnetwork.Network, error)
-	GetNetworkByName(idName string) (libnetwork.Network, error)
-	GetNetworksByID(partialID string) []libnetwork.Network
 	GetNetworks() []libnetwork.Network
 	CreateNetwork(nc types.NetworkCreateRequest) (*types.NetworkCreateResponse, error)
 	ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error

api/server/router/network/network_routes.go

@@ -2,7 +2,9 @@ package network
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
+	"strings"
 
 	"golang.org/x/net/context"
 
@@ -82,14 +84,80 @@ func (n *networkRouter) getNetwork(ctx context.Context, w http.ResponseWriter, r
 		return err
 	}
 
-	nw, err := n.backend.FindNetwork(vars["id"])
+	term := vars["id"]
-	if err != nil {
+
-		if nr, err := n.cluster.GetNetwork(vars["id"]); err == nil {
+	// In case multiple networks have duplicate names, return error.
-			return httputils.WriteJSON(w, http.StatusOK, nr)
+	// TODO (yongtang): should we wrap with version here for backward compatibility?
+
+	// First find based on full ID, return immediately once one is found.
+	// If a network appears both in swarm and local, assume it is in local first
+
+	// For full name and partial ID, save the result first, and process later
+	// in case multiple records was found based on the same term
+	listByFullName := map[string]types.NetworkResource{}
+	listByPartialID := map[string]types.NetworkResource{}
+
+	nw := n.backend.GetNetworks()
+	for _, network := range nw {
+		if network.ID() == term {
+			return httputils.WriteJSON(w, http.StatusOK, *n.buildDetailedNetworkResources(network))
+		}
+		if network.Name() == term {
+			// No need to check the ID collision here as we are still in
+			// local scope and the network ID is unique in this scope.
+			listByFullName[network.ID()] = *n.buildDetailedNetworkResources(network)
+		}
+		if strings.HasPrefix(network.ID(), term) {
+			// No need to check the ID collision here as we are still in
+			// local scope and the network ID is unique in this scope.
+			listByPartialID[network.ID()] = *n.buildDetailedNetworkResources(network)
 		}
-		return err
 	}
-	return httputils.WriteJSON(w, http.StatusOK, n.buildDetailedNetworkResources(nw))
+
+	nr, _ := n.cluster.GetNetworks()
+	for _, network := range nr {
+		if network.ID == term {
+			return httputils.WriteJSON(w, http.StatusOK, network)
+		}
+		if network.Name == term {
+			// Check the ID collision as we are in swarm scope here, and
+			// the map (of the listByFullName) may have already had a
+			// network with the same ID (from local scope previously)
+			if _, ok := listByFullName[network.ID]; !ok {
+				listByFullName[network.ID] = network
+			}
+		}
+		if strings.HasPrefix(network.ID, term) {
+			// Check the ID collision as we are in swarm scope here, and
+			// the map (of the listByPartialID) may have already had a
+			// network with the same ID (from local scope previously)
+			if _, ok := listByPartialID[network.ID]; !ok {
+				listByPartialID[network.ID] = network
+			}
+		}
+	}
+
+	// Find based on full name, returns true only if no duplicates
+	if len(listByFullName) == 1 {
+		for _, v := range listByFullName {
+			return httputils.WriteJSON(w, http.StatusOK, v)
+		}
+	}
+	if len(listByFullName) > 1 {
+		return fmt.Errorf("network %s is ambiguous (%d matches found based on name)", term, len(listByFullName))
+	}
+
+	// Find based on partial ID, returns true only if no duplicates
+	if len(listByPartialID) == 1 {
+		for _, v := range listByPartialID {
+			return httputils.WriteJSON(w, http.StatusOK, v)
+		}
+	}
+	if len(listByPartialID) > 1 {
+		return fmt.Errorf("network %s is ambiguous (%d matches found based on ID prefix)", term, len(listByPartialID))
+	}
+
+	return libnetwork.ErrNoSuchNetwork(term)
 }
 
 func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

integration-cli/docker_cli_swarm_test.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/daemon"
@@ -1665,3 +1666,77 @@ func (s *DockerSwarmSuite) TestSwarmReadonlyRootfs(c *check.C) {
 	c.Assert(err, checker.IsNil, check.Commentf(out))
 	c.Assert(strings.TrimSpace(out), checker.Equals, "true")
 }
+
+func (s *DockerSwarmSuite) TestNetworkInspectWithDuplicateNames(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+
+	name := "foo"
+	networkCreateRequest := types.NetworkCreateRequest{
+		Name: name,
+		NetworkCreate: types.NetworkCreate{
+			CheckDuplicate: false,
+			Driver:         "bridge",
+		},
+	}
+
+	var n1 types.NetworkCreateResponse
+	status, body, err := d.SockRequest("POST", "/networks/create", networkCreateRequest)
+	c.Assert(err, checker.IsNil, check.Commentf(string(body)))
+	c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body)))
+	c.Assert(json.Unmarshal(body, &n1), checker.IsNil)
+
+	// Full ID always works
+	out, err := d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID)
+
+	// Name works if it is unique
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID)
+
+	var n2 types.NetworkCreateResponse
+	status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest)
+	c.Assert(err, checker.IsNil, check.Commentf(string(body)))
+	c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body)))
+	c.Assert(json.Unmarshal(body, &n2), checker.IsNil)
+
+	// Full ID always works
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID)
+
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n2.ID)
+
+	// Name with duplicates
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name)
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)")
+
+	out, err = d.Cmd("network", "rm", n2.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+
+	// Dupliates with name but with different driver
+	networkCreateRequest.NetworkCreate.Driver = "overlay"
+
+	status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest)
+	c.Assert(err, checker.IsNil, check.Commentf(string(body)))
+	c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body)))
+	c.Assert(json.Unmarshal(body, &n2), checker.IsNil)
+
+	// Full ID always works
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n1.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n1.ID)
+
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", n2.ID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.Equals, n2.ID)
+
+	// Name with duplicates
+	out, err = d.Cmd("network", "inspect", "--format", "{{.ID}}", name)
+	c.Assert(err, checker.NotNil, check.Commentf(out))
+	c.Assert(out, checker.Contains, "network foo is ambiguous (2 matches found based on name)")
+}