diff --git a/api/server/router/network/backend.go b/api/server/router/network/backend.go index a32a0b9c00..8ae6a078b7 100644 --- a/api/server/router/network/backend.go +++ b/api/server/router/network/backend.go @@ -12,11 +12,11 @@ import ( // Backend is all the methods that need to be implemented // to provide network specific functionality. type Backend interface { - FindNetwork(idName string) (libnetwork.Network, error) + FindUniqueNetwork(idName string) (libnetwork.Network, error) GetNetworks() []libnetwork.Network CreateNetwork(nc types.NetworkCreateRequest) (*types.NetworkCreateResponse, error) ConnectContainerToNetwork(containerName, networkName string, endpointConfig *network.EndpointSettings) error DisconnectContainerFromNetwork(containerName string, networkName string, force bool) error - DeleteNetwork(name string) error + DeleteNetwork(networkID string) error NetworksPrune(ctx context.Context, pruneFilters filters.Args) (*types.NetworksPruneReport, error) } diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index a67bb9f0b2..497ce0a502 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -2,6 +2,7 @@ package network import ( "encoding/json" + "fmt" "net/http" "strconv" "strings" @@ -288,7 +289,12 @@ func (n *networkRouter) postNetworkConnect(ctx context.Context, w http.ResponseW return err } - return n.backend.ConnectContainerToNetwork(connect.Container, vars["id"], connect.EndpointConfig) + // Always make sure there is no ambiguity with respect to the network ID/name + nw, err := n.backend.FindUniqueNetwork(vars["id"]) + if err != nil { + return err + } + return n.backend.ConnectContainerToNetwork(connect.Container, nw.ID(), connect.EndpointConfig) } func (n *networkRouter) postNetworkDisconnect(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { @@ -312,15 +318,19 @@ func (n *networkRouter) deleteNetwork(ctx context.Context, w http.ResponseWriter if err := httputils.ParseForm(r); err != nil { return err } - if _, err := n.cluster.GetNetwork(vars["id"]); err == nil { - if err = n.cluster.RemoveNetwork(vars["id"]); err != nil { + + nw, err := n.findUniqueNetwork(vars["id"]) + if err != nil { + return err + } + if nw.Scope == "swarm" { + if err = n.cluster.RemoveNetwork(nw.ID); err != nil { + return err + } + } else { + if err := n.backend.DeleteNetwork(nw.ID); err != nil { return err } - w.WriteHeader(http.StatusNoContent) - return nil - } - if err := n.backend.DeleteNetwork(vars["id"]); err != nil { - return err } w.WriteHeader(http.StatusNoContent) return nil @@ -518,3 +528,79 @@ func (n *networkRouter) postNetworksPrune(ctx context.Context, w http.ResponseWr } return httputils.WriteJSON(w, http.StatusOK, pruneReport) } + +// findUniqueNetwork will search network across different scopes (both local and swarm). +// NOTE: This findUniqueNetwork is differnt from FindUniqueNetwork from the daemon. +// In case multiple networks have duplicate names, return error. +// 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 +// TODO (yongtang): should we wrap with version here for backward compatibility? +func (n *networkRouter) findUniqueNetwork(term string) (types.NetworkResource, error) { + listByFullName := map[string]types.NetworkResource{} + listByPartialID := map[string]types.NetworkResource{} + + nw := n.backend.GetNetworks() + for _, network := range nw { + if network.ID() == term { + return *n.buildDetailedNetworkResources(network, false), nil + + } + 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, false) + } + 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, false) + } + } + + nr, _ := n.cluster.GetNetworks() + for _, network := range nr { + if network.ID == term { + return network, nil + } + 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 v, nil + } + } + if len(listByFullName) > 1 { + return types.NetworkResource{}, 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 v, nil + } + } + if len(listByPartialID) > 1 { + return types.NetworkResource{}, fmt.Errorf("network %s is ambiguous (%d matches found based on ID prefix)", term, len(listByPartialID)) + } + + return types.NetworkResource{}, libnetwork.ErrNoSuchNetwork(term) +} diff --git a/daemon/cluster/executor/backend.go b/daemon/cluster/executor/backend.go index f6763d15cd..ed8f50467d 100644 --- a/daemon/cluster/executor/backend.go +++ b/daemon/cluster/executor/backend.go @@ -27,8 +27,8 @@ import ( // Backend defines the executor component for a swarm agent. type Backend interface { CreateManagedNetwork(clustertypes.NetworkCreateRequest) error - DeleteManagedNetwork(name string) error - FindNetwork(idName string) (libnetwork.Network, error) + DeleteManagedNetwork(networkID string) error + FindUniqueNetwork(idName string) (libnetwork.Network, error) SetupIngress(clustertypes.NetworkCreateRequest, string) (<-chan struct{}, error) ReleaseIngress() (<-chan struct{}, error) PullImage(ctx context.Context, image, tag, platform string, metaHeaders map[string][]string, authConfig *types.AuthConfig, outStream io.Writer) error diff --git a/daemon/cluster/executor/container/adapter.go b/daemon/cluster/executor/container/adapter.go index 81e740af13..7db47983cb 100644 --- a/daemon/cluster/executor/container/adapter.go +++ b/daemon/cluster/executor/container/adapter.go @@ -143,8 +143,8 @@ func (c *containerAdapter) pullImage(ctx context.Context) error { } func (c *containerAdapter) createNetworks(ctx context.Context) error { - for _, network := range c.container.networks() { - ncr, err := c.container.networkCreateRequest(network) + for name := range c.container.networksAttachments { + ncr, err := c.container.networkCreateRequest(name) if err != nil { return err } @@ -162,15 +162,15 @@ func (c *containerAdapter) createNetworks(ctx context.Context) error { } func (c *containerAdapter) removeNetworks(ctx context.Context) error { - for _, nid := range c.container.networks() { - if err := c.backend.DeleteManagedNetwork(nid); err != nil { + for name, v := range c.container.networksAttachments { + if err := c.backend.DeleteManagedNetwork(v.Network.ID); err != nil { switch err.(type) { case *libnetwork.ActiveEndpointsError: continue case libnetwork.ErrNoSuchNetwork: continue default: - log.G(ctx).Errorf("network %s remove failed: %v", nid, err) + log.G(ctx).Errorf("network %s remove failed: %v", name, err) return err } } diff --git a/daemon/cluster/executor/container/container.go b/daemon/cluster/executor/container/container.go index 4f41fb3e23..ad4ade0713 100644 --- a/daemon/cluster/executor/container/container.go +++ b/daemon/cluster/executor/container/container.go @@ -507,7 +507,7 @@ func getEndpointConfig(na *api.NetworkAttachment, b executorpkg.Backend) *networ DriverOpts: na.DriverAttachmentOpts, } if v, ok := na.Network.Spec.Annotations.Labels["com.docker.swarm.predefined"]; ok && v == "true" { - if ln, err := b.FindNetwork(na.Network.Spec.Annotations.Name); err == nil { + if ln, err := b.FindUniqueNetwork(na.Network.Spec.Annotations.Name); err == nil { n.NetworkID = ln.ID() } } @@ -575,19 +575,6 @@ func (c *containerConfig) serviceConfig() *clustertypes.ServiceConfig { return svcCfg } -// networks returns a list of network names attached to the container. The -// returned name can be used to lookup the corresponding network create -// options. -func (c *containerConfig) networks() []string { - var networks []string - - for name := range c.networksAttachments { - networks = append(networks, name) - } - - return networks -} - func (c *containerConfig) networkCreateRequest(name string) (clustertypes.NetworkCreateRequest, error) { na, ok := c.networksAttachments[name] if !ok { diff --git a/daemon/cluster/networks.go b/daemon/cluster/networks.go index 04582eb31e..3b01ea4efb 100644 --- a/daemon/cluster/networks.go +++ b/daemon/cluster/networks.go @@ -292,7 +292,7 @@ func (c *Cluster) populateNetworkID(ctx context.Context, client swarmapi.Control for i, n := range networks { apiNetwork, err := getNetwork(ctx, client, n.Target) if err != nil { - ln, _ := c.config.Backend.FindNetwork(n.Target) + ln, _ := c.config.Backend.FindUniqueNetwork(n.Target) if ln != nil && runconfig.IsPreDefinedNetwork(ln.Name()) { // Need to retrieve the corresponding predefined swarm network // and use its id for the request. diff --git a/daemon/container_operations.go b/daemon/container_operations.go index fc08f8263d..355d5496ea 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -251,8 +251,8 @@ func (daemon *Daemon) updateNetworkSettings(container *container.Container, n li return runconfig.ErrConflictHostNetwork } - for s := range container.NetworkSettings.Networks { - sn, err := daemon.FindNetwork(s) + for s, v := range container.NetworkSettings.Networks { + sn, err := daemon.FindUniqueNetwork(getNetworkID(s, v.EndpointSettings)) if err != nil { continue } @@ -308,8 +308,8 @@ func (daemon *Daemon) updateNetwork(container *container.Container) error { // Find if container is connected to the default bridge network var n libnetwork.Network - for name := range container.NetworkSettings.Networks { - sn, err := daemon.FindNetwork(name) + for name, v := range container.NetworkSettings.Networks { + sn, err := daemon.FindUniqueNetwork(getNetworkID(name, v.EndpointSettings)) if err != nil { continue } @@ -339,7 +339,7 @@ func (daemon *Daemon) updateNetwork(container *container.Container) error { } func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrName string, epConfig *networktypes.EndpointSettings) (libnetwork.Network, *networktypes.NetworkingConfig, error) { - n, err := daemon.FindNetwork(idOrName) + n, err := daemon.FindUniqueNetwork(getNetworkID(idOrName, epConfig)) if err != nil { // We should always be able to find the network for a // managed container. @@ -377,16 +377,16 @@ func (daemon *Daemon) findAndAttachNetwork(container *container.Container, idOrN // trigger attachment in the swarm cluster manager. if daemon.clusterProvider != nil { var err error - config, err = daemon.clusterProvider.AttachNetwork(idOrName, container.ID, addresses) + config, err = daemon.clusterProvider.AttachNetwork(getNetworkID(idOrName, epConfig), container.ID, addresses) if err != nil { return nil, nil, err } } - n, err = daemon.FindNetwork(idOrName) + n, err = daemon.FindUniqueNetwork(getNetworkID(idOrName, epConfig)) if err != nil { if daemon.clusterProvider != nil { - if err := daemon.clusterProvider.DetachNetwork(idOrName, container.ID); err != nil { + if err := daemon.clusterProvider.DetachNetwork(getNetworkID(idOrName, epConfig), container.ID); err != nil { logrus.Warnf("Could not rollback attachment for container %s to network %s: %v", container.ID, idOrName, err) } } @@ -437,7 +437,7 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai if mode.IsUserDefined() { var err error - n, err = daemon.FindNetwork(networkName) + n, err = daemon.FindUniqueNetwork(networkName) if err == nil { networkName = n.Name() } @@ -797,7 +797,7 @@ func (daemon *Daemon) connectToNetwork(container *container.Container, idOrName // ForceEndpointDelete deletes an endpoint from a network forcefully func (daemon *Daemon) ForceEndpointDelete(name string, networkName string) error { - n, err := daemon.FindNetwork(networkName) + n, err := daemon.FindUniqueNetwork(networkName) if err != nil { return err } @@ -949,7 +949,7 @@ func (daemon *Daemon) releaseNetwork(container *container.Container) { var networks []libnetwork.Network for n, epSettings := range settings { - if nw, err := daemon.FindNetwork(n); err == nil { + if nw, err := daemon.FindUniqueNetwork(getNetworkID(n, epSettings.EndpointSettings)); err == nil { networks = append(networks, nw) } @@ -993,7 +993,7 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName return errRemovalContainer(container.ID) } - n, err := daemon.FindNetwork(idOrName) + n, err := daemon.FindUniqueNetwork(idOrName) if err == nil && n != nil { if err := daemon.updateNetworkConfig(container, n, endpointConfig, true); err != nil { return err @@ -1016,7 +1016,7 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName // DisconnectFromNetwork disconnects container from network n. func (daemon *Daemon) DisconnectFromNetwork(container *container.Container, networkName string, force bool) error { - n, err := daemon.FindNetwork(networkName) + n, err := daemon.FindUniqueNetwork(networkName) container.Lock() defer container.Unlock() @@ -1087,3 +1087,12 @@ func (daemon *Daemon) DeactivateContainerServiceBinding(containerName string) er } return sb.DisableService() } + +func getNetworkID(name string, endpointSettings *networktypes.EndpointSettings) string { + // We only want to prefer NetworkID for user defined networks. + // For systems like bridge, none, etc. the name is preferred (otherwise restart may cause issues) + if containertypes.NetworkMode(name).IsUserDefined() && endpointSettings != nil && endpointSettings.NetworkID != "" { + return endpointSettings.NetworkID + } + return name +} diff --git a/daemon/network.go b/daemon/network.go index 573901e7a6..9b5a66f36a 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -29,31 +29,36 @@ func (daemon *Daemon) NetworkControllerEnabled() bool { return daemon.netController != nil } -// FindNetwork function finds a network for a given string that can represent network name or id -func (daemon *Daemon) FindNetwork(idName string) (libnetwork.Network, error) { - // 1. match by full ID. - n, err := daemon.GetNetworkByID(idName) - if err == nil || !isNoSuchNetworkError(err) { - return n, err +// FindUniqueNetwork returns a network based on: +// 1. Full ID +// 2. Full Name +// 3. Partial ID +// as long as there is no ambiguity +func (daemon *Daemon) FindUniqueNetwork(term string) (libnetwork.Network, error) { + listByFullName := []libnetwork.Network{} + listByPartialID := []libnetwork.Network{} + for _, nw := range daemon.GetNetworks() { + if nw.ID() == term { + return nw, nil + } + if nw.Name() == term { + listByFullName = append(listByFullName, nw) + } + if strings.HasPrefix(nw.ID(), term) { + listByPartialID = append(listByPartialID, nw) + } } - - // 2. match by full name - n, err = daemon.GetNetworkByName(idName) - if err == nil || !isNoSuchNetworkError(err) { - return n, err + switch { + case len(listByFullName) == 1: + return listByFullName[0], nil + case len(listByFullName) > 1: + return nil, fmt.Errorf("network %s is ambiguous (%d matches found based on name)", term, len(listByFullName)) + case len(listByPartialID) == 1: + return listByPartialID[0], nil + case len(listByPartialID) > 1: + return nil, fmt.Errorf("network %s is ambiguous (%d matches found based on ID prefix)", term, len(listByPartialID)) } - - // 3. match by ID prefix - list := daemon.GetNetworksByIDPrefix(idName) - if len(list) == 0 { - // Be very careful to change the error type here, the libnetwork.ErrNoSuchNetwork error is used by the controller - // to retry the creation of the network as managed through the swarm manager - return nil, errors.WithStack(notFound(libnetwork.ErrNoSuchNetwork(idName))) - } - if len(list) > 1 { - return nil, errors.WithStack(invalidIdentifier(idName)) - } - return list[0], nil + return nil, libnetwork.ErrNoSuchNetwork(term) } func isNoSuchNetworkError(err error) bool { @@ -274,7 +279,9 @@ func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string // check if user defined CheckDuplicate, if set true, return err // otherwise prepare a warning message if create.CheckDuplicate { - return nil, libnetwork.NetworkNameError(create.Name) + if !agent || nw.Info().Dynamic() { + return nil, libnetwork.NetworkNameError(create.Name) + } } warning = fmt.Sprintf("Network with name %s (id : %s) already exists", nw.Name(), nw.ID()) } @@ -464,25 +471,56 @@ func (daemon *Daemon) GetNetworkDriverList() []string { } // DeleteManagedNetwork deletes an agent network. +// The requirement of networkID is enforced. func (daemon *Daemon) DeleteManagedNetwork(networkID string) error { - return daemon.deleteNetwork(networkID, true) + n, err := daemon.GetNetworkByID(networkID) + if err != nil { + return err + } + return daemon.deleteNetwork(n, true) } // DeleteNetwork destroys a network unless it's one of docker's predefined networks. func (daemon *Daemon) DeleteNetwork(networkID string) error { - return daemon.deleteNetwork(networkID, false) -} - -func (daemon *Daemon) deleteNetwork(networkID string, dynamic bool) error { - nw, err := daemon.FindNetwork(networkID) + n, err := daemon.GetNetworkByID(networkID) if err != nil { return err } + return daemon.deleteNetwork(n, false) +} - if nw.Info().Ingress() { - return nil +func (daemon *Daemon) deleteLoadBalancerSandbox(n libnetwork.Network) { + controller := daemon.netController + + //The only endpoint left should be the LB endpoint (nw.Name() + "-endpoint") + endpoints := n.Endpoints() + if len(endpoints) == 1 { + sandboxName := n.Name() + "-sbox" + + info := endpoints[0].Info() + if info != nil { + sb := info.Sandbox() + if sb != nil { + if err := sb.DisableService(); err != nil { + logrus.Warnf("Failed to disable service on sandbox %s: %v", sandboxName, err) + //Ignore error and attempt to delete the load balancer endpoint + } + } + } + + if err := endpoints[0].Delete(true); err != nil { + logrus.Warnf("Failed to delete endpoint %s (%s) in %s: %v", endpoints[0].Name(), endpoints[0].ID(), sandboxName, err) + //Ignore error and attempt to delete the sandbox. + } + + if err := controller.SandboxDestroy(sandboxName); err != nil { + logrus.Warnf("Failed to delete %s sandbox: %v", sandboxName, err) + //Ignore error and attempt to delete the network. + } } +} +func (daemon *Daemon) deleteNetwork(nw libnetwork.Network, dynamic bool) error { if runconfig.IsPreDefinedNetwork(nw.Name()) && !dynamic { err := fmt.Errorf("%s is a pre-defined network and cannot be removed", nw.Name()) return notAllowedError{err} diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 283f576f57..0eff6aacb2 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -2137,3 +2137,76 @@ func (s *DockerSwarmSuite) TestSwarmClusterEventsConfig(c *check.C) { // filtered by config waitForEvent(c, d, t1, "-f type=config", "config remove "+id, defaultRetryCount) } + +func (s *DockerSwarmSuite) TestServiceCreateWithDuplicateNetworkNames(c *check.C) { + d := s.AddDaemon(c, true, true) + + name := "foo" + networkCreateRequest := types.NetworkCreateRequest{ + Name: name, + NetworkCreate: types.NetworkCreate{ + CheckDuplicate: false, + Driver: "bridge", + }, + } + + // Create networks with the same name, 2 in local scope and 1 in swarm scope + 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) + + 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) + + var n3 types.NetworkCreateResponse + // 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, &n3), checker.IsNil) + + // Create Service with the same name + d.CreateService(c, simpleTestService, func(s *swarm.Service) { + s.Spec.Name = "top" + s.Spec.TaskTemplate.Networks = []swarm.NetworkAttachmentConfig{ + {Target: name}, + } + }) + + // make sure task has been deployed. + waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) + + result := icmd.RunCmd(d.Command("ps", "-a", "-q")) + result.Assert(c, icmd.Success) + containers := strings.Split(strings.TrimSpace(result.Stdout()), "\n") + c.Assert(len(containers), checker.Equals, 1) + + result = icmd.RunCmd(d.Command("inspect", "--format", `{{.NetworkSettings.Networks.foo.NetworkID}}`, containers[0])) + result.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n3.ID) + + // Remove Service + result = icmd.RunCmd(d.Command("service", "rm", "top")) + result.Assert(c, icmd.Success) + + // make sure task has been destroyed. + waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 0) + + result = icmd.RunCmd(d.Command("network", "rm", n1.ID)) + result.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n1.ID) + + result = icmd.RunCmd(d.Command("network", "rm", n2.ID)) + result.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n2.ID) + + result = icmd.RunCmd(d.Command("network", "rm", n3.ID)) + result.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n3.ID) +}