Issue #68: In bridge.go driver remove veth on endpoint delete

- Store *Interface on endpoint create
- Remove from bridgeEndpoint ip params now available in Interface
- On endpoint delete attempt a removal of veth plugged into bridge
- (tested disabling defer netutils.SetupTestNetNS(t)() in libnetwrok_test)
- Fix bridge to  store endpoints per sandbox
- Fix bug in error.go which causes stack overflow
- Start bridge error string w/ lower case as per go convention

Signed-off-by: Alessandro Boch <aboch@docker.com>
This commit is contained in:
Alessandro Boch 2015-04-20 04:23:04 -07:00
parent 05a76a5444
commit 468ebf3816
4 changed files with 242 additions and 88 deletions

View File

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

View File

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

View File

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

View File

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