Update `FindUniqueNetwork` to address network name duplications

This fix is part of the effort to address 30242 where
issue arise because of the fact that multiple networks
may share the same name (within or across local/swarm scopes).

The focus of this fix is to allow creation of service
when a network in local scope has the same name as the
service network.

An integration test has been added.

This fix fixes 30242.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
This commit is contained in:
Yong Tang 2017-10-31 19:46:53 +00:00
parent 39faf2a3a8
commit cafed80cd0
9 changed files with 270 additions and 77 deletions

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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

View File

@ -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
}
}

View File

@ -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 {

View File

@ -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.

View File

@ -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
}

View File

@ -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}

View File

@ -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)
}