diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 6e3d3a773c..028ef374e7 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -42,16 +42,14 @@ type Configuration struct { } type bridgeEndpoint struct { - id types.UUID - addressIPv4 net.IP - addressIPv6 net.IP + id types.UUID + port *sandbox.Interface } type bridgeNetwork struct { - id types.UUID - // bridge interface points to the linux bridge and it's configuration - bridge *bridgeInterface - endpoint *bridgeEndpoint + id types.UUID + bridge *bridgeInterface // The bridge's L3 interface + endpoints map[string]*bridgeEndpoint // key: sandbox id sync.Mutex } @@ -66,11 +64,28 @@ func init() { portMapper = portmapper.New() } -// New provides a new instance of bridge driver instance +// New provides a new instance of bridge driver func New() (string, driverapi.Driver) { return networkType, &driver{} } +func (n *bridgeNetwork) getEndpoint(eid types.UUID) (string, *bridgeEndpoint, error) { + n.Lock() + defer n.Unlock() + + if eid == "" { + return "", nil, InvalidEndpointIDError(eid) + } + + for sk, ep := range n.endpoints { + if ep.id == eid { + return sk, ep, nil + } + } + + return "", nil, nil +} + func (d *driver) Config(option interface{}) error { var config *Configuration @@ -98,11 +113,9 @@ func (d *driver) Config(option interface{}) error { // Create a new network using simplebridge plugin func (d *driver) CreateNetwork(id types.UUID, option interface{}) error { + var err error - var ( - err error - ) - + // Driver must be configured d.Lock() if d.config == nil { d.Unlock() @@ -110,14 +123,18 @@ func (d *driver) CreateNetwork(id types.UUID, option interface{}) error { } config := d.config + // Sanity checks if d.network != nil { d.Unlock() return ErrNetworkExists } - d.network = &bridgeNetwork{id: id} + + // Create and set network handler in driver + d.network = &bridgeNetwork{id: id, endpoints: make(map[string]*bridgeEndpoint)} d.Unlock() + + // On failure make sure to reset driver network handler to nil defer func() { - // On failure make sure to reset d.network to nil if err != nil { d.Lock() d.network = nil @@ -125,7 +142,11 @@ func (d *driver) CreateNetwork(id types.UUID, option interface{}) error { } }() + // Create or retrieve the bridge L3 interface bridgeIface := newInterface(config) + d.network.bridge = bridgeIface + + // Prepare the bridge setup configuration bridgeSetup := newBridgeSetup(config, bridgeIface) // If the bridge interface doesn't exist, we need to start the setup steps @@ -176,21 +197,22 @@ func (d *driver) CreateNetwork(id types.UUID, option interface{}) error { return err } - d.network.bridge = bridgeIface return nil } func (d *driver) DeleteNetwork(nid types.UUID) error { var err error + + // Get network handler and remove it from driver d.Lock() n := d.network d.network = nil d.Unlock() + + // On failure set network handler back in driver, but + // only if is not already taken over by some other thread defer func() { if err != nil { - // On failure set d.network back to n - // but only if is not already take over - // by some other thread d.Lock() if d.network == nil { d.network = n @@ -199,27 +221,31 @@ func (d *driver) DeleteNetwork(nid types.UUID) error { } }() + // Sanity check if n == nil { err = driverapi.ErrNoNetwork return err } - if n.endpoint != nil { - err = &ActiveEndpointsError{nid: string(n.id), eid: string(n.endpoint.id)} + // Cannot remove network if endpoints are still present + if len(n.endpoints) != 0 { + err = ActiveEndpointsError(n.id) return err } + // Programming err = netlink.LinkDel(n.bridge.Link) + return err } func (d *driver) CreateEndpoint(nid, eid types.UUID, sboxKey string, epOption interface{}) (*sandbox.Info, error) { var ( ipv6Addr *net.IPNet - ip6 net.IP err error ) + // Get the network handler and make sure it exists d.Lock() n := d.network config := d.config @@ -228,37 +254,64 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, sboxKey string, epOption in return nil, driverapi.ErrNoNetwork } + // Sanity check n.Lock() if n.id != nid { n.Unlock() return nil, InvalidNetworkIDError(nid) } + n.Unlock() - if n.endpoint != nil { + // Check if endpoint id is good and retrieve correspondent endpoint + _, ep, err := n.getEndpoint(eid) + if err != nil { + return nil, err + } + + // Endpoint with that id exists either on desired or other sandbox + if ep != nil { + return nil, driverapi.ErrEndpointExists + } + + // Check if valid sandbox key + if sboxKey == "" { + return nil, InvalidSandboxIDError(sboxKey) + } + + // Check if endpoint already exists for this sandbox + n.Lock() + if _, ok := n.endpoints[sboxKey]; ok { n.Unlock() return nil, driverapi.ErrEndpointExists } - n.endpoint = &bridgeEndpoint{id: eid} + + // Create and add the endpoint + endpoint := &bridgeEndpoint{id: eid} + n.endpoints[sboxKey] = endpoint n.Unlock() + + // On failure make sure to remove the endpoint defer func() { - // On failye make sure to reset n.endpoint to nil if err != nil { n.Lock() - n.endpoint = nil + delete(n.endpoints, sboxKey) n.Unlock() } }() + // Generate a name for what will be the host side pipe interface name1, err := generateIfaceName() if err != nil { return nil, err } + // Generate a name for what will be the sandbox side pipe interface name2, err := generateIfaceName() if err != nil { return nil, err } + // Generate and add the interface pipe host <-> sandbox veth := &netlink.Veth{ LinkAttrs: netlink.LinkAttrs{Name: name1, TxQLen: 0}, PeerName: name2} @@ -266,6 +319,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, sboxKey string, epOption in return nil, err } + // Get the host side pipe interface handler host, err := netlink.LinkByName(name1) if err != nil { return nil, err @@ -276,27 +330,31 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, sboxKey string, epOption in } }() - container, err := netlink.LinkByName(name2) + // Get the sandbox side pipe interface handler + sbox, err := netlink.LinkByName(name2) if err != nil { return nil, err } defer func() { if err != nil { - netlink.LinkDel(container) + netlink.LinkDel(sbox) } }() + // Attach host side pipe interface into the bridge if err = netlink.LinkSetMaster(host, &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: config.BridgeName}}); err != nil { return nil, err } + // Reuqest a v4 address for the sandbox side pipe interface ip4, err := ipAllocator.RequestIP(n.bridge.bridgeIPv4, nil) if err != nil { return nil, err } ipv4Addr := &net.IPNet{IP: ip4, Mask: n.bridge.bridgeIPv4.Mask} + // Request a v6 address for the sandbox side pipe interface if config.EnableIPv6 { ip6, err := ipAllocator.RequestIP(n.bridge.bridgeIPv6, nil) if err != nil { @@ -305,29 +363,31 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, sboxKey string, epOption in ipv6Addr = &net.IPNet{IP: ip6, Mask: n.bridge.bridgeIPv6.Mask} } - var interfaces []*sandbox.Interface - sinfo := &sandbox.Info{} - + // Store the sandbox side pipe interface + // This is needed for cleanup on DeleteEndpoint() intf := &sandbox.Interface{} intf.SrcName = name2 intf.DstName = containerVeth intf.Address = ipv4Addr + + // Update endpoint with the sandbox interface info + endpoint.port = intf + + // Generate the sandbox info to return + sinfo := &sandbox.Info{Interfaces: []*sandbox.Interface{intf}} sinfo.Gateway = n.bridge.bridgeIPv4.IP if config.EnableIPv6 { intf.AddressIPv6 = ipv6Addr sinfo.GatewayIPv6 = n.bridge.bridgeIPv6.IP } - n.endpoint.addressIPv4 = ip4 - n.endpoint.addressIPv6 = ip6 - interfaces = append(interfaces, intf) - sinfo.Interfaces = interfaces return sinfo, nil } func (d *driver) DeleteEndpoint(nid, eid types.UUID) error { var err error + // Get the network handler and make sure it exists d.Lock() n := d.network config := d.config @@ -336,50 +396,61 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error { return driverapi.ErrNoNetwork } + // Sanity Check n.Lock() if n.id != nid { n.Unlock() return InvalidNetworkIDError(nid) } - - if n.endpoint == nil { - n.Unlock() - return driverapi.ErrNoEndpoint - } - - ep := n.endpoint - if ep.id != eid { - n.Unlock() - return InvalidEndpointIDError(eid) - } - - n.endpoint = nil n.Unlock() + + // Check endpoint id and if an endpoint is actually there + sboxKey, ep, err := n.getEndpoint(eid) + if err != nil { + return err + } + if ep == nil { + return EndpointNotFoundError(eid) + } + + // Remove it + n.Lock() + delete(n.endpoints, sboxKey) + n.Unlock() + + // On failure make sure to set back ep in n.endpoints, but only + // if it hasn't been taken over already by some other thread. defer func() { if err != nil { - // On failure make to set back n.endpoint with ep - // but only if it hasn't been taken over - // already by some other thread. n.Lock() - if n.endpoint == nil { - n.endpoint = ep + if _, ok := n.endpoints[sboxKey]; !ok { + n.endpoints[sboxKey] = ep } n.Unlock() } }() - err = ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, ep.addressIPv4) + // Release the v4 address allocated to this endpoint's sandbox interface + err = ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, ep.port.Address.IP) if err != nil { return err } + // Release the v6 address allocated to this endpoint's sandbox interface if config.EnableIPv6 { - err := ipAllocator.ReleaseIP(n.bridge.bridgeIPv6, ep.addressIPv6) + err := ipAllocator.ReleaseIP(n.bridge.bridgeIPv6, ep.port.AddressIPv6.IP) if err != nil { return err } } + // Try removal of link. Discard error: link pair might have + // already been deleted by sandbox delete. + link, err := netlink.LinkByName(ep.port.SrcName) + if err == nil { + netlink.LinkDel(link) + } + return nil } diff --git a/libnetwork/drivers/bridge/error.go b/libnetwork/drivers/bridge/error.go index ee28cd2aa5..dd8907d318 100644 --- a/libnetwork/drivers/bridge/error.go +++ b/libnetwork/drivers/bridge/error.go @@ -17,21 +17,18 @@ var ( ErrNetworkExists = errors.New("network already exists, simplebridge can only have one network") // ErrIfaceName error is returned when a new name could not be generated. - ErrIfaceName = errors.New("Failed to find name for new interface") + ErrIfaceName = errors.New("failed to find name for new interface") // ErrNoIPAddr error is returned when bridge has no IPv4 address configured. - ErrNoIPAddr = errors.New("Bridge has no IPv4 address configured") + ErrNoIPAddr = errors.New("bridge has no IPv4 address configured") ) // ActiveEndpointsError is returned when there are -// already active endpoints in the network being deleted. -type ActiveEndpointsError struct { - nid string - eid string -} +// still active endpoints in the network being deleted. +type ActiveEndpointsError string -func (aee *ActiveEndpointsError) Error() string { - return fmt.Sprintf("Network %s has active endpoint %s", aee.nid, aee.eid) +func (aee ActiveEndpointsError) Error() string { + return fmt.Sprintf("network %s has active endpoint", string(aee)) } // InvalidNetworkIDError is returned when the passed @@ -39,15 +36,31 @@ func (aee *ActiveEndpointsError) Error() string { type InvalidNetworkIDError string func (inie InvalidNetworkIDError) Error() string { - return fmt.Sprintf("invalid network id %s", inie) + return fmt.Sprintf("invalid network id %s", string(inie)) } // InvalidEndpointIDError is returned when the passed -// endpoint id for an existing endpoint is not a known id. +// endpoint id is not valid. type InvalidEndpointIDError string func (ieie InvalidEndpointIDError) Error() string { - return fmt.Sprintf("invalid endpoint id %s", ieie) + return fmt.Sprintf("invalid endpoint id: %s", string(ieie)) +} + +// InvalidSandboxIDError is returned when the passed +// sandbox id valid. +type InvalidSandboxIDError string + +func (isie InvalidSandboxIDError) Error() string { + return fmt.Sprintf("invalid sanbox id: %s", string(isie)) +} + +// EndpointNotFoundError is returned when the no endpoint +// with the passed endpoint id is found. +type EndpointNotFoundError string + +func (enfe EndpointNotFoundError) Error() string { + return fmt.Sprintf("endpoint not found: %s", string(enfe)) } // NonDefaultBridgeExistError is returned when a non-default @@ -55,7 +68,7 @@ func (ieie InvalidEndpointIDError) Error() string { type NonDefaultBridgeExistError string func (ndbee NonDefaultBridgeExistError) Error() string { - return fmt.Sprintf("bridge device with non default name %s must be created manually", ndbee) + return fmt.Sprintf("bridge device with non default name %s must be created manually", string(ndbee)) } // FixedCIDRv4Error is returned when fixed-cidrv4 configuration @@ -67,7 +80,7 @@ type FixedCIDRv4Error struct { } func (fcv4 *FixedCIDRv4Error) Error() string { - return fmt.Sprintf("Setup FixedCIDRv4 failed for subnet %s in %s: %v", fcv4.subnet, fcv4.net, fcv4.err) + return fmt.Sprintf("setup FixedCIDRv4 failed for subnet %s in %s: %v", fcv4.subnet, fcv4.net, fcv4.err) } // FixedCIDRv6Error is returned when fixed-cidrv6 configuration @@ -78,26 +91,26 @@ type FixedCIDRv6Error struct { } func (fcv6 *FixedCIDRv6Error) Error() string { - return fmt.Sprintf("Setup FixedCIDRv6 failed for subnet %s in %s: %v", fcv6.net, fcv6.net, fcv6.err) + return fmt.Sprintf("setup FixedCIDRv6 failed for subnet %s in %s: %v", fcv6.net, fcv6.net, fcv6.err) } type ipForwardCfgError bridgeInterface func (i *ipForwardCfgError) Error() string { - return fmt.Sprintf("Unexpected request to enable IP Forwarding for: %v", *i) + return fmt.Sprintf("unexpected request to enable IP Forwarding for: %v", *i) } type ipTableCfgError string func (name ipTableCfgError) Error() string { - return fmt.Sprintf("Unexpected request to set IP tables for interface: %s", name) + return fmt.Sprintf("unexpected request to set IP tables for interface: %s", string(name)) } // IPv4AddrRangeError is returned when a valid IP address range couldn't be found. type IPv4AddrRangeError string func (name IPv4AddrRangeError) Error() string { - return fmt.Sprintf("can't find an address range for interface %q", name) + return fmt.Sprintf("can't find an address range for interface %q", string(name)) } // IPv4AddrAddError is returned when IPv4 address could not be added to the bridge. @@ -107,7 +120,7 @@ type IPv4AddrAddError struct { } func (ipv4 *IPv4AddrAddError) Error() string { - return fmt.Sprintf("Failed to add IPv4 address %s to bridge: %v", ipv4.ip, ipv4.err) + return fmt.Sprintf("failed to add IPv4 address %s to bridge: %v", ipv4.ip, ipv4.err) } // IPv6AddrAddError is returned when IPv6 address could not be added to the bridge. @@ -117,7 +130,7 @@ type IPv6AddrAddError struct { } func (ipv6 *IPv6AddrAddError) Error() string { - return fmt.Sprintf("Failed to add IPv6 address %s to bridge: %v", ipv6.ip, ipv6.err) + return fmt.Sprintf("failed to add IPv6 address %s to bridge: %v", ipv6.ip, ipv6.err) } // IPv4AddrNoMatchError is returned when the bridge's IPv4 address does not match configured. @@ -127,12 +140,12 @@ type IPv4AddrNoMatchError struct { } func (ipv4 *IPv4AddrNoMatchError) Error() string { - return fmt.Sprintf("Bridge IPv4 (%s) does not match requested configuration %s", ipv4.ip, ipv4.cfgIP) + return fmt.Sprintf("bridge IPv4 (%s) does not match requested configuration %s", ipv4.ip, ipv4.cfgIP) } // IPv6AddrNoMatchError is returned when the bridge's IPv6 address does not match configured. type IPv6AddrNoMatchError net.IPNet func (ipv6 *IPv6AddrNoMatchError) Error() string { - return fmt.Sprintf("Bridge IPv6 addresses do not match the expected bridge configuration %s", ipv6) + return fmt.Sprintf("bridge IPv6 addresses do not match the expected bridge configuration %s", ipv6) } diff --git a/libnetwork/drivers/bridge/network_test.go b/libnetwork/drivers/bridge/network_test.go index 4a5e303306..109e8ca6a3 100644 --- a/libnetwork/drivers/bridge/network_test.go +++ b/libnetwork/drivers/bridge/network_test.go @@ -25,9 +25,37 @@ func TestLinkCreate(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } - sinfo, err := d.CreateEndpoint("dummy", "ep", "", "") + sinfo, err := d.CreateEndpoint("dummy", "", "sb1", "") if err != nil { - t.Fatalf("Failed to create a link: %v", err) + if _, ok := err.(InvalidEndpointIDError); !ok { + t.Fatalf("Failed with a wrong error :%s", err.Error()) + } + } else { + t.Fatalf("Failed to detect invalid config") + } + + sinfo, err = d.CreateEndpoint("dummy", "ep", "", "") + if err != nil { + if _, ok := err.(InvalidSandboxIDError); !ok { + t.Fatalf("Failed with a wrong error :%s", err.Error()) + } + } else { + t.Fatalf("Failed to detect invalid config") + } + + sinfo, err = d.CreateEndpoint("dummy", "ep", "cc", "") + if err != nil { + t.Fatalf("Failed to create a link: %s", err.Error()) + } + + _, err = d.CreateEndpoint("dummy", "ep", "cc2", "") + if err == nil { + t.Fatalf("Failed to detect duplicate endpoint id on same network") + } + + _, err = d.CreateEndpoint("dummy", "ep2", "cc", "") + if err == nil { + t.Fatalf("Failed to detect addition of more than one endpoint to same sandbox") } interfaces := sinfo.Interfaces @@ -82,18 +110,18 @@ func TestLinkCreateTwo(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } - _, err = d.CreateEndpoint("dummy", "ep", "", "") + _, err = d.CreateEndpoint("dummy", "ep", "s1", "") if err != nil { - t.Fatalf("Failed to create a link: %v", err) + t.Fatalf("Failed to create a link: %s", err.Error()) } - _, err = d.CreateEndpoint("dummy", "ep1", "", "") + _, err = d.CreateEndpoint("dummy", "ep", "s1", "") if err != nil { if err != driverapi.ErrEndpointExists { - t.Fatalf("Failed with a wrong error :%v", err) + t.Fatalf("Failed with a wrong error :%s", err.Error()) } } else { - t.Fatalf("Expected to fail while trying to add more than one endpoint") + t.Fatalf("Expected to fail while trying to add same endpoint twice") } } @@ -112,9 +140,9 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) { t.Fatalf("Failed to create bridge: %v", err) } - sinfo, err := d.CreateEndpoint("dummy", "ep", "", "") + sinfo, err := d.CreateEndpoint("dummy", "ep", "sb2", "") if err != nil { - t.Fatalf("Failed to create a link: %v", err) + t.Fatalf("Failed to create a link: %s", err.Error()) } interfaces := sinfo.Interfaces @@ -126,3 +154,44 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) { t.Fatalf("Expected GatewayIPv6 to be nil when IPv6 is not enabled. Got GatewayIPv6 = %s", sinfo.GatewayIPv6.String()) } } + +func TestLinkDelete(t *testing.T) { + defer netutils.SetupTestNetNS(t)() + _, d := New() + + config := &Configuration{ + BridgeName: DefaultBridgeName, + EnableIPv6: true} + if err := d.Config(config); err != nil { + t.Fatalf("Failed to setup driver config: %v", err) + } + + err := d.CreateNetwork("dummy", "") + if err != nil { + t.Fatalf("Failed to create bridge: %v", err) + } + + _, err = d.CreateEndpoint("dummy", "ep1", "s1", "") + if err != nil { + t.Fatalf("Failed to create a link: %s", err.Error()) + } + + err = d.DeleteEndpoint("dummy", "") + if err != nil { + if _, ok := err.(InvalidEndpointIDError); !ok { + t.Fatalf("Failed with a wrong error :%s", err.Error()) + } + } else { + t.Fatalf("Failed to detect invalid config") + } + + err = d.DeleteEndpoint("dummy", "ep1") + if err != nil { + t.Fatal(err) + } + + err = d.DeleteEndpoint("dummy", "ep1") + if err == nil { + t.Fatal(err) + } +} diff --git a/libnetwork/libnetwork_test.go b/libnetwork/libnetwork_test.go index ad72a408bf..62278fc5b0 100644 --- a/libnetwork/libnetwork_test.go +++ b/libnetwork/libnetwork_test.go @@ -67,7 +67,7 @@ func TestSimplebridge(t *testing.T) { t.Fatal(err) } - ep, err := network.CreateEndpoint("testep", "", "") + ep, err := network.CreateEndpoint("testep", "sb1", "") if err != nil { t.Fatal(err) } @@ -91,6 +91,7 @@ func TestSimplebridge(t *testing.T) { func TestUnknownDriver(t *testing.T) { defer netutils.SetupTestNetNS(t)() + _, err := createTestNetwork("unknowndriver", "testnetwork", options.Generic{}) if err == nil { t.Fatal("Expected to fail. But instead succeeded") @@ -204,7 +205,7 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) { t.Fatal(err) } - ep, err := network.CreateEndpoint("testep", "", "") + ep, err := network.CreateEndpoint("testep", "sb2", "") if err != nil { t.Fatal(err) } @@ -272,7 +273,7 @@ func TestUnknownEndpoint(t *testing.T) { t.Fatal(err) } - ep, err := network.CreateEndpoint("testep", "", "") + ep, err := network.CreateEndpoint("testep", "sb1", "") if err != nil { t.Fatal(err) }