Merge pull request #30897 from yongtang/30242-network-duplicate-names

Update `FindNetwork` to address network name duplications
This commit is contained in:
Yong Tang 2018-01-10 05:30:58 -08:00 committed by GitHub
commit 484612f388
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 268 additions and 103 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 different from FindUniqueNetwork in 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 && !network.Info().Ingress() {
// 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

@ -170,7 +170,7 @@ func (daemon *Daemon) initializeNetworkingPaths(container *container.Container,
if nc.NetworkSettings != nil {
for n := range nc.NetworkSettings.Networks {
sn, err := daemon.FindNetwork(n)
sn, err := daemon.FindUniqueNetwork(n)
if err != nil {
continue
}

View File

@ -317,7 +317,7 @@ func TestValidateContainerIsolation(t *testing.T) {
func TestFindNetworkErrorType(t *testing.T) {
d := Daemon{}
_, err := d.FindNetwork("fakeNet")
_, err := d.FindUniqueNetwork("fakeNet")
_, ok := errors.Cause(err).(libnetwork.ErrNoSuchNetwork)
if !errdefs.IsNotFound(err) || !ok {
assert.Fail(t, "The FindNetwork method MUST always return an error that implements the NotFound interface and is ErrNoSuchNetwork")

View File

@ -226,20 +226,3 @@ func translateContainerdStartErr(cmd string, setExitCode func(int), err error) e
// TODO: it would be nice to get some better errors from containerd so we can return better errors here
return retErr
}
// TODO: cpuguy83 take care of it once the new library is ready
type errNotFound struct{ error }
func (errNotFound) NotFound() {}
func (e errNotFound) Cause() error {
return e.error
}
// notFound is a helper to create an error of the class with the same name from any error type
func notFound(err error) error {
if err == nil {
return nil
}
return errNotFound{err}
}

View File

@ -29,36 +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
}
func isNoSuchNetworkError(err error) bool {
_, ok := err.(libnetwork.ErrNoSuchNetwork)
return ok
return nil, libnetwork.ErrNoSuchNetwork(term)
}
// GetNetworkByID function returns a network whose ID matches the given ID.
@ -104,7 +104,11 @@ func (daemon *Daemon) GetNetworksByIDPrefix(partialID string) []libnetwork.Netwo
// getAllNetworks returns a list containing all networks
func (daemon *Daemon) getAllNetworks() []libnetwork.Network {
return daemon.netController.Networks()
c := daemon.netController
if c == nil {
return nil
}
return c.Networks()
}
type ingressJob struct {
@ -274,7 +278,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 +470,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

@ -159,7 +159,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) {
gwHNSID := ""
if c.NetworkSettings != nil {
for n := range c.NetworkSettings.Networks {
sn, err := daemon.FindNetwork(n)
sn, err := daemon.FindUniqueNetwork(n)
if err != nil {
continue
}

View File

@ -11,6 +11,7 @@ import (
"github.com/docker/docker/client"
"github.com/docker/docker/integration-cli/request"
"github.com/gotestyourself/gotestyourself/poll"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/net/context"
)
@ -80,6 +81,68 @@ func TestCreateServiceMultipleTimes(t *testing.T) {
poll.WaitOn(t, networkIsRemoved(client, overlayID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second))
}
func TestCreateWithDuplicateNetworkNames(t *testing.T) {
defer setupTest(t)()
d := newSwarm(t)
defer d.Stop(t)
client, err := request.NewClientForHost(d.Sock())
require.NoError(t, err)
name := "foo"
networkCreate := types.NetworkCreate{
CheckDuplicate: false,
Driver: "bridge",
}
n1, err := client.NetworkCreate(context.Background(), name, networkCreate)
require.NoError(t, err)
n2, err := client.NetworkCreate(context.Background(), name, networkCreate)
require.NoError(t, err)
// Dupliates with name but with different driver
networkCreate.Driver = "overlay"
n3, err := client.NetworkCreate(context.Background(), name, networkCreate)
require.NoError(t, err)
// Create Service with the same name
var instances uint64 = 1
serviceSpec := swarmServiceSpec("top", instances)
serviceSpec.TaskTemplate.Networks = append(serviceSpec.TaskTemplate.Networks, swarm.NetworkAttachmentConfig{Target: name})
service, err := client.ServiceCreate(context.Background(), serviceSpec, types.ServiceCreateOptions{})
require.NoError(t, err)
poll.WaitOn(t, serviceRunningTasksCount(client, service.ID, instances))
resp, _, err := client.ServiceInspectWithRaw(context.Background(), service.ID, types.ServiceInspectOptions{})
require.NoError(t, err)
assert.Equal(t, n3.ID, resp.Spec.TaskTemplate.Networks[0].Target)
// Remove Service
err = client.ServiceRemove(context.Background(), service.ID)
require.NoError(t, err)
// Make sure task has been destroyed.
poll.WaitOn(t, serviceIsRemoved(client, service.ID))
// Remove networks
err = client.NetworkRemove(context.Background(), n3.ID)
require.NoError(t, err)
err = client.NetworkRemove(context.Background(), n2.ID)
require.NoError(t, err)
err = client.NetworkRemove(context.Background(), n1.ID)
require.NoError(t, err)
// Make sure networks have been destroyed.
poll.WaitOn(t, networkIsRemoved(client, n3.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second))
poll.WaitOn(t, networkIsRemoved(client, n2.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second))
poll.WaitOn(t, networkIsRemoved(client, n1.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second))
}
func swarmServiceSpec(name string, replicas uint64) swarm.ServiceSpec {
return swarm.ServiceSpec{
Annotations: swarm.Annotations{