Merge pull request #58 from mrjana/cnm

Change all the naked error returns in bridge driver to proper error types

libnetwork/drivers/bridge/bridge.go

@@ -1,8 +1,6 @@
 package bridge
 
 import (
-	"errors"
-	"fmt"
 	"net"
 	"strings"
 	"sync"
@@ -78,14 +76,14 @@ func (d *driver) Config(option interface{}) error {
 	defer d.Unlock()
 
 	if d.config != nil {
-		return fmt.Errorf("configuration already exists, simplebridge configuration can be applied only once")
+		return ErrConfigExists
 	}
 
 	switch opt := option.(type) {
 	case options.Generic:
 		opaqueConfig, err := options.GenerateFromModel(opt, &Configuration{})
 		if err != nil {
-			return fmt.Errorf("failed to generate driver config: %v", err)
+			return err
 		}
 		config = opaqueConfig.(*Configuration)
 	case *Configuration:
@@ -106,13 +104,13 @@ func (d *driver) CreateNetwork(id driverapi.UUID, option interface{}) error {
 	d.Lock()
 	if d.config == nil {
 		d.Unlock()
-		return fmt.Errorf("trying to create a network on a driver without valid config")
+		return ErrInvalidConfig
 	}
 	config := d.config
 
 	if d.network != nil {
 		d.Unlock()
-		return fmt.Errorf("network already exists, simplebridge can only have one network")
+		return ErrNetworkExists
 	}
 	d.network = &bridgeNetwork{id: id}
 	d.Unlock()
@@ -205,7 +203,7 @@ func (d *driver) DeleteNetwork(nid driverapi.UUID) error {
 	}
 
 	if n.endpoint != nil {
-		err = fmt.Errorf("Network %s has active endpoint %s", n.id, n.endpoint.id)
+		err = &ActiveEndpointsError{nid: string(n.id), eid: string(n.endpoint.id)}
 		return err
 	}
 
@@ -230,7 +228,7 @@ func (d *driver) CreateEndpoint(nid, eid driverapi.UUID, sboxKey string, epOptio
 	n.Lock()
 	if n.id != nid {
 		n.Unlock()
-		return nil, fmt.Errorf("invalid network id %s", nid)
+		return nil, InvalidNetworkIDError(nid)
 	}
 
 	if n.endpoint != nil {
@@ -338,7 +336,7 @@ func (d *driver) DeleteEndpoint(nid, eid driverapi.UUID) error {
 	n.Lock()
 	if n.id != nid {
 		n.Unlock()
-		return fmt.Errorf("invalid network id %s", nid)
+		return InvalidNetworkIDError(nid)
 	}
 
 	if n.endpoint == nil {
@@ -349,7 +347,7 @@ func (d *driver) DeleteEndpoint(nid, eid driverapi.UUID) error {
 	ep := n.endpoint
 	if ep.id != eid {
 		n.Unlock()
-		return fmt.Errorf("invalid endpoint id %s", eid)
+		return InvalidEndpointIDError(eid)
 	}
 
 	n.endpoint = nil
@@ -398,5 +396,5 @@ func generateIfaceName() (string, error) {
 			return "", err
 		}
 	}
-	return "", errors.New("Failed to find name for new interface")
+	return "", ErrIfaceName
 }

libnetwork/drivers/bridge/error.go

@@ -0,0 +1,138 @@
+package bridge
+
+import (
+	"errors"
+	"fmt"
+	"net"
+)
+
+var (
+	// ErrConfigExists error is returned when driver already has a config applied.
+	ErrConfigExists = errors.New("configuration already exists, simplebridge configuration can be applied only once")
+
+	// ErrInvalidConfig error is returned when a network is created on a driver without valid config.
+	ErrInvalidConfig = errors.New("trying to create a network on a driver without valid config")
+
+	// ErrNetworkExists error is returned when a network already exists and another network is created.
+	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")
+
+	// ErrNoIPAddr error is returned when bridge has no IPv4 addrss 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
+}
+
+func (aee *ActiveEndpointsError) Error() string {
+	return fmt.Sprintf("Network %s has active endpoint %s", aee.nid, aee.eid)
+}
+
+// InvalidNetworkIDError is returned when the passed
+// network id for an existing network is not a known id.
+type InvalidNetworkIDError string
+
+func (inie InvalidNetworkIDError) Error() string {
+	return fmt.Sprintf("invalid network id %s", inie)
+}
+
+// InvalidEndpointIDError is returned when the passed
+// endpoint id for an existing endpoint is not a known id.
+type InvalidEndpointIDError string
+
+func (ieie InvalidEndpointIDError) Error() string {
+	return fmt.Sprintf("invalid endpoint id %s", ieie)
+}
+
+// NonDefaultBridgeExistError is returned when a non-default
+// bridge config is passed but it does not already exist.
+type NonDefaultBridgeExistError string
+
+func (ndbee NonDefaultBridgeExistError) Error() string {
+	return fmt.Sprintf("bridge device with non default name %s must be created manually", ndbee)
+}
+
+// FixedCIDRv4Error is returned when fixed-cidrv4 configuration
+// failed.
+type FixedCIDRv4Error struct {
+	net    *net.IPNet
+	subnet *net.IPNet
+	err    error
+}
+
+func (fcv4 *FixedCIDRv4Error) Error() string {
+	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
+// failed.
+type FixedCIDRv6Error struct {
+	net *net.IPNet
+	err error
+}
+
+func (fcv6 *FixedCIDRv6Error) Error() string {
+	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)
+}
+
+type ipTableCfgError string
+
+func (name ipTableCfgError) Error() string {
+	return fmt.Sprintf("Unexpected request to set IP tables for interface: %s", 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)
+}
+
+// IPv4AddrAddError is returned when IPv4 address could not be added to the bridge.
+type IPv4AddrAddError struct {
+	ip  *net.IPNet
+	err error
+}
+
+func (ipv4 *IPv4AddrAddError) Error() string {
+	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.
+type IPv6AddrAddError struct {
+	ip  *net.IPNet
+	err error
+}
+
+func (ipv6 *IPv6AddrAddError) Error() string {
+	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.
+type IPv4AddrNoMatchError struct {
+	ip    net.IP
+	cfgIP net.IP
+}
+
+func (ipv4 *IPv4AddrNoMatchError) Error() string {
+	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)
+}

libnetwork/drivers/bridge/setup_device.go

@@ -1,8 +1,6 @@
 package bridge
 
 import (
-	"fmt"
-
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/docker/pkg/parsers/kernel"
 	"github.com/docker/libnetwork/netutils"
@@ -14,7 +12,7 @@ func setupDevice(config *Configuration, i *bridgeInterface) error {
 	// We only attempt to create the bridge when the requested device name is
 	// the default one.
 	if config.BridgeName != DefaultBridgeName && !config.AllowNonDefaultBridge {
-		return fmt.Errorf("bridge device with non default name %q must be created manually", config.BridgeName)
+		return NonDefaultBridgeExistError(config.BridgeName)
 	}
 
 	// Set the bridgeInterface netlink.Bridge.

libnetwork/drivers/bridge/setup_device_test.go

@@ -3,7 +3,6 @@ package bridge
 import (
 	"bytes"
 	"net"
-	"strings"
 	"testing"
 
 	"github.com/docker/libnetwork/netutils"
@@ -36,8 +35,13 @@ func TestSetupNewNonDefaultBridge(t *testing.T) {
 	config := &Configuration{BridgeName: "test0"}
 	br := &bridgeInterface{}
 
-	if err := setupDevice(config, br); err == nil || !strings.Contains(err.Error(), "non default name") {
-		t.Fatalf("Expected bridge creation failure with \"non default name\", got: %v", err)
+	err := setupDevice(config, br)
+	if err == nil {
+		t.Fatal("Expected bridge creation failure with \"non default name\", succeeded")
+	}
+
+	if _, ok := err.(NonDefaultBridgeExistError); !ok {
+		t.Fatalf("Did not fail with expected error. Actual error: %v", err)
 	}
 }
 

libnetwork/drivers/bridge/setup_fixedcidrv4.go

@@ -1,10 +1,6 @@
 package bridge
 
-import (
-	"fmt"
-
-	log "github.com/Sirupsen/logrus"
-)
+import log "github.com/Sirupsen/logrus"
 
 func setupFixedCIDRv4(config *Configuration, i *bridgeInterface) error {
 	addrv4, _, err := i.addresses()
@@ -14,7 +10,7 @@ func setupFixedCIDRv4(config *Configuration, i *bridgeInterface) error {
 
 	log.Debugf("Using IPv4 subnet: %v", config.FixedCIDR)
 	if err := ipAllocator.RegisterSubnet(addrv4.IPNet, config.FixedCIDR); err != nil {
-		return fmt.Errorf("Setup FixedCIDRv4 failed for subnet %s in %s: %v", config.FixedCIDR, addrv4.IPNet, err)
+		return &FixedCIDRv4Error{subnet: config.FixedCIDR, net: addrv4.IPNet, err: err}
 	}
 
 	return nil

libnetwork/drivers/bridge/setup_fixedcidrv4_test.go

@@ -50,7 +50,13 @@ func TestSetupBadFixedCIDRv4(t *testing.T) {
 		t.Fatalf("Assign IPv4 to bridge failed: %v", err)
 	}
 
-	if err := setupFixedCIDRv4(config, br); err == nil {
+	err := setupFixedCIDRv4(config, br)
+	if err == nil {
 		t.Fatal("Setup bridge FixedCIDRv4 should have failed")
 	}
+
+	if _, ok := err.(*FixedCIDRv4Error); !ok {
+		t.Fatalf("Did not fail with expected error. Actual error: %v", err)
+	}
+
 }

libnetwork/drivers/bridge/setup_fixedcidrv6.go

@@ -1,15 +1,11 @@
 package bridge
 
-import (
-	"fmt"
-
-	log "github.com/Sirupsen/logrus"
-)
+import log "github.com/Sirupsen/logrus"
 
 func setupFixedCIDRv6(config *Configuration, i *bridgeInterface) error {
 	log.Debugf("Using IPv6 subnet: %v", config.FixedCIDRv6)
 	if err := ipAllocator.RegisterSubnet(config.FixedCIDRv6, config.FixedCIDRv6); err != nil {
-		return fmt.Errorf("Setup FixedCIDRv6 failed for subnet %s in %s: %v", config.FixedCIDRv6, config.FixedCIDRv6, err)
+		return &FixedCIDRv6Error{net: config.FixedCIDRv6, err: err}
 	}
 
 	return nil

libnetwork/drivers/bridge/setup_ip_forwarding.go

@@ -13,7 +13,7 @@ const (
 func setupIPForwarding(config *Configuration, i *bridgeInterface) error {
 	// Sanity Check
 	if config.EnableIPForwarding == false {
-		return fmt.Errorf("Unexpected request to enable IP Forwarding for: %v", *i)
+		return (*ipForwardCfgError)(i)
 	}
 
 	// Enable IPv4 forwarding

libnetwork/drivers/bridge/setup_ip_forwarding_test.go

@@ -3,7 +3,6 @@ package bridge
 import (
 	"bytes"
 	"io/ioutil"
-	"strings"
 	"testing"
 )
 
@@ -47,9 +46,12 @@ func TestUnexpectedSetupIPForwarding(t *testing.T) {
 	br := &bridgeInterface{}
 
 	// Attempt Set IP Forwarding
-	if err := setupIPForwarding(config, br); err == nil {
+	err := setupIPForwarding(config, br)
+	if err == nil {
 		t.Fatal("Setup IP forwarding was expected to fail")
-	} else if !strings.Contains(err.Error(), "Unexpected request") {
+	}
+
+	if _, ok := err.(*ipForwardCfgError); !ok {
 		t.Fatalf("Setup IP forwarding failed with unexpected error: %v", err)
 	}
 }

libnetwork/drivers/bridge/setup_ip_tables.go

@@ -16,7 +16,7 @@ const (
 func setupIPTables(config *Configuration, i *bridgeInterface) error {
 	// Sanity check.
 	if config.EnableIPTables == false {
-		return fmt.Errorf("Unexpected request to set IP tables for interface: %s", config.BridgeName)
+		return ipTableCfgError(config.BridgeName)
 	}
 
 	addrv4, _, err := netutils.GetIfaceAddr(config.BridgeName)

libnetwork/drivers/bridge/setup_ipv4.go

@@ -1,7 +1,6 @@
 package bridge
 
 import (
-	"fmt"
 	"net"
 
 	log "github.com/Sirupsen/logrus"
@@ -49,7 +48,7 @@ func setupBridgeIPv4(config *Configuration, i *bridgeInterface) error {
 
 	log.Debugf("Creating bridge interface %q with network %s", config.BridgeName, bridgeIPv4)
 	if err := netlink.AddrAdd(i.Link, &netlink.Addr{IPNet: bridgeIPv4}); err != nil {
-		return fmt.Errorf("Failed to add IPv4 address %s to bridge: %v", bridgeIPv4, err)
+		return &IPv4AddrAddError{ip: bridgeIPv4, err: err}
 	}
 
 	i.bridgeIPv4 = bridgeIPv4
@@ -80,5 +79,5 @@ func electBridgeIPv4(config *Configuration) (*net.IPNet, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("'t find an address range for interface %q", config.BridgeName)
+	return nil, IPv4AddrRangeError(config.BridgeName)
 }

libnetwork/drivers/bridge/setup_ipv6.go

@@ -30,7 +30,7 @@ func setupBridgeIPv6(config *Configuration, i *bridgeInterface) error {
 	}
 
 	if err := netlink.AddrAdd(i.Link, &netlink.Addr{IPNet: bridgeIPv6}); err != nil {
-		return fmt.Errorf("Failed to add IPv6 address %s to bridge: %v", bridgeIPv6, err)
+		return &IPv6AddrAddError{ip: bridgeIPv6, err: err}
 	}
 
 	i.bridgeIPv6 = bridgeIPv6

libnetwork/drivers/bridge/setup_verify.go

@@ -1,10 +1,6 @@
 package bridge
 
-import (
-	"fmt"
-
-	"github.com/vishvananda/netlink"
-)
+import "github.com/vishvananda/netlink"
 
 func setupVerifyAndReconcile(config *Configuration, i *bridgeInterface) error {
 	// Fetch a single IPv4 and a slice of IPv6 addresses from the bridge.
@@ -15,18 +11,18 @@ func setupVerifyAndReconcile(config *Configuration, i *bridgeInterface) error {
 
 	// Verify that the bridge does have an IPv4 address.
 	if addrv4.IPNet == nil {
-		return fmt.Errorf("Bridge has no IPv4 address configured")
+		return ErrNoIPAddr
 	}
 
 	// Verify that the bridge IPv4 address matches the requested configuration.
 	if config.AddressIPv4 != nil && !addrv4.IP.Equal(config.AddressIPv4.IP) {
-		return fmt.Errorf("Bridge IPv4 (%s) does not match requested configuration %s", addrv4.IP, config.AddressIPv4.IP)
+		return &IPv4AddrNoMatchError{ip: addrv4.IP, cfgIP: config.AddressIPv4.IP}
 	}
 
 	// Verify that one of the bridge IPv6 addresses matches the requested
 	// configuration.
 	if config.EnableIPv6 && !findIPv6Address(netlink.Addr{IPNet: bridgeIPv6}, addrsv6) {
-		return fmt.Errorf("Bridge IPv6 addresses do not match the expected bridge configuration %s", bridgeIPv6)
+		return (*IPv6AddrNoMatchError)(bridgeIPv6)
 	}
 
 	// By this time we have either configured a new bridge with an IP address