Merge pull request #85 from aboch/drv

Refactor NetworkController interface

libnetwork/cmd/readme_test/readme.go

@@ -10,15 +10,17 @@ func main() {
 	// Create a new controller instance
 	controller := libnetwork.New()
 
+	// Select and configure the network driver
+	networkType := "bridge"
 	option := options.Generic{}
-	driver, err := controller.NewNetworkDriver("bridge", option)
+	err := controller.ConfigureNetworkDriver(networkType, option)
 	if err != nil {
 		return
 	}
 
 	netOptions := options.Generic{}
 	// Create a network for containers to join.
-	network, err := controller.NewNetwork(driver, "network1", netOptions)
+	network, err := controller.NewNetwork(networkType, "network1", netOptions)
 	if err != nil {
 		return
 	}

libnetwork/cmd/test/main.go

@@ -15,8 +15,9 @@ func main() {
 
 	options := options.Generic{"AddressIPv4": net}
 	controller := libnetwork.New()
-	driver, _ := controller.NewNetworkDriver("bridge", options)
+	netType := "bridge"
-	netw, err := controller.NewNetwork(driver, "dummy", "")
+	err := controller.ConfigureNetworkDriver(netType, options)
+	netw, err := controller.NewNetwork(netType, "dummy", "")
 	if err != nil {
 		log.Fatal(err)
 	}

libnetwork/driverapi/driverapi.go

@@ -39,4 +39,7 @@ type Driver interface {
 	// DeleteEndpoint invokes the driver method to delete an endpoint
 	// passing the network id and endpoint id.
 	DeleteEndpoint(nid, eid types.UUID) error
+
+	// Type returns the the type of this driver, the network type this driver manages
+	Type() string
 }

libnetwork/drivers/bridge/bridge.go

@@ -475,6 +475,10 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error {
 	return nil
 }
 
+func (d *driver) Type() string {
+	return networkType
+}
+
 func parseEndpointOptions(epOptions interface{}) (*EndpointConfiguration, error) {
 	if epOptions == nil {
 		return nil, nil

libnetwork/libnetwork_test.go

@@ -6,22 +6,24 @@ import (
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/docker/libnetwork"
-	_ "github.com/docker/libnetwork/drivers/bridge"
 	"github.com/docker/libnetwork/netutils"
 	"github.com/docker/libnetwork/pkg/options"
 )
 
-var bridgeName = "dockertest0"
+const (
+	netType    = "bridge"
+	bridgeName = "dockertest0"
+)
 
 func createTestNetwork(networkType, networkName string, option options.Generic) (libnetwork.Network, error) {
 	controller := libnetwork.New()
 
-	driver, err := controller.NewNetworkDriver(networkType, option)
+	err := controller.ConfigureNetworkDriver(networkType, option)
 	if err != nil {
 		return nil, err
 	}
 
-	network, err := controller.NewNetwork(driver, networkName, "")
+	network, err := controller.NewNetwork(networkType, networkName, "")
 	if err != nil {
 		return nil, err
 	}
@@ -62,7 +64,7 @@ func Testbridge(t *testing.T) {
 		"EnableIPForwarding":    true,
 		"AllowNonDefaultBridge": true}
 
-	network, err := createTestNetwork("bridge", "testnetwork", option)
+	network, err := createTestNetwork(netType, "testnetwork", option)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -106,12 +108,12 @@ func TestNilDriver(t *testing.T) {
 	controller := libnetwork.New()
 
 	option := options.Generic{}
-	_, err := controller.NewNetwork(nil, "dummy", option)
+	_, err := controller.NewNetwork("framerelay", "dummy", option)
 	if err == nil {
 		t.Fatal("Expected to fail. But instead succeeded")
 	}
 
-	if err != libnetwork.ErrNilNetworkDriver {
+	if err != libnetwork.ErrInvalidNetworkDriver {
 		t.Fatalf("Did not fail with expected error. Actual error: %v", err)
 	}
 }
@@ -120,7 +122,7 @@ func TestNoInitDriver(t *testing.T) {
 	controller := libnetwork.New()
 
 	option := options.Generic{}
-	_, err := controller.NewNetwork(&libnetwork.NetworkDriver{}, "dummy", option)
+	_, err := controller.NewNetwork("ppp", "dummy", option)
 	if err == nil {
 		t.Fatal("Expected to fail. But instead succeeded")
 	}
@@ -135,17 +137,17 @@ func TestDuplicateNetwork(t *testing.T) {
 	controller := libnetwork.New()
 
 	option := options.Generic{}
-	driver, err := controller.NewNetworkDriver("bridge", option)
+	err := controller.ConfigureNetworkDriver(netType, option)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = controller.NewNetwork(driver, "testnetwork", "")
+	_, err = controller.NewNetwork(netType, "testnetwork", "")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	_, err = controller.NewNetwork(driver, "testnetwork", "")
+	_, err = controller.NewNetwork(netType, "testnetwork", "")
 	if err == nil {
 		t.Fatal("Expected to fail. But instead succeeded")
 	}
@@ -158,7 +160,7 @@ func TestDuplicateNetwork(t *testing.T) {
 func TestNetworkName(t *testing.T) {
 	networkName := "testnetwork"
 
-	n, err := createTestNetwork("bridge", networkName, options.Generic{})
+	n, err := createTestNetwork(netType, networkName, options.Generic{})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -169,7 +171,7 @@ func TestNetworkName(t *testing.T) {
 }
 
 func TestNetworkType(t *testing.T) {
-	networkType := "bridge"
+	networkType := netType
 
 	n, err := createTestNetwork(networkType, "testnetwork", options.Generic{})
 	if err != nil {
@@ -182,7 +184,7 @@ func TestNetworkType(t *testing.T) {
 }
 
 func TestNetworkID(t *testing.T) {
-	networkType := "bridge"
+	networkType := netType
 
 	n, err := createTestNetwork(networkType, "testnetwork", options.Generic{})
 	if err != nil {
@@ -200,7 +202,7 @@ func TestDeleteNetworkWithActiveEndpoints(t *testing.T) {
 		"BridgeName":            bridgeName,
 		"AllowNonDefaultBridge": true}
 
-	network, err := createTestNetwork("bridge", "testnetwork", option)
+	network, err := createTestNetwork(netType, "testnetwork", option)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -235,7 +237,7 @@ func TestUnknownNetwork(t *testing.T) {
 		"BridgeName":            bridgeName,
 		"AllowNonDefaultBridge": true}
 
-	network, err := createTestNetwork("bridge", "testnetwork", option)
+	network, err := createTestNetwork(netType, "testnetwork", option)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -268,7 +270,7 @@ func TestUnknownEndpoint(t *testing.T) {
 		"AddressIPv4":           subnet,
 		"AllowNonDefaultBridge": true}
 
-	network, err := createTestNetwork("bridge", "testnetwork", option)
+	network, err := createTestNetwork(netType, "testnetwork", option)
 	if err != nil {
 		t.Fatal(err)
 	}

libnetwork/network.go

@@ -50,11 +50,11 @@ import (
 // NetworkController provides the interface for controller instance which manages
 // networks.
 type NetworkController interface {
-	// NOTE: This method will go away when moving to plugin infrastructure
+	// ConfigureNetworkDriver applies the passed options to the driver instance for the specified network type
-	NewNetworkDriver(networkType string, options interface{}) (*NetworkDriver, error)
+	ConfigureNetworkDriver(networkType string, options interface{}) error
 	// Create a new network. The options parameter carries network specific options.
 	// Labels support will be added in the near future.
-	NewNetwork(d *NetworkDriver, name string, options interface{}) (Network, error)
+	NewNetwork(networkType, name string, options interface{}) (Network, error)
 }
 
 // A Network represents a logical connectivity zone that containers may
@@ -99,12 +99,6 @@ type Endpoint interface {
 	Delete() error
 }
 
-// NetworkDriver provides a reference to driver and way to push driver specific config
-type NetworkDriver struct {
-	networkType    string
-	internalDriver driverapi.Driver
-}
-
 type endpoint struct {
 	name        string
 	id          types.UUID
@@ -117,7 +111,7 @@ type network struct {
 	name        string
 	networkType string
 	id          types.UUID
-	driver      *NetworkDriver
+	driver      driverapi.Driver
 	endpoints   endpointTable
 	sync.Mutex
 }
@@ -136,26 +130,24 @@ func New() NetworkController {
 	return &controller{networkTable{}, enumerateDrivers(), sync.Mutex{}}
 }
 
-func (c *controller) NewNetworkDriver(networkType string, options interface{}) (*NetworkDriver, error) {
+func (c *controller) ConfigureNetworkDriver(networkType string, options interface{}) error {
 	d, ok := c.drivers[networkType]
 	if !ok {
-		return nil, NetworkTypeError(networkType)
+		return NetworkTypeError(networkType)
 	}
-
+	return d.Config(options)
-	if err := d.Config(options); err != nil {
-		return nil, err
-	}
-
-	return &NetworkDriver{networkType: networkType, internalDriver: d}, nil
 }
 
-// NewNetwork creates a new network of the specified NetworkDriver. The options
+// NewNetwork creates a new network of the specified network type. The options
-// are driver specific and modeled in a generic way.
+// are network specific and modeled in a generic way.
-func (c *controller) NewNetwork(nd *NetworkDriver, name string, options interface{}) (Network, error) {
+func (c *controller) NewNetwork(networkType, name string, options interface{}) (Network, error) {
-	if nd == nil {
+	// Check if a driver for the specified network type is available
-		return nil, ErrNilNetworkDriver
+	d, ok := c.drivers[networkType]
+	if !ok {
+		return nil, ErrInvalidNetworkDriver
 	}
 
+	// Check if a network already exists with the specified network name
 	c.Lock()
 	for _, n := range c.networks {
 		if n.name == name {
@@ -165,22 +157,21 @@ func (c *controller) NewNetwork(nd *NetworkDriver, name string, options interfac
 	}
 	c.Unlock()
 
+	// Construct the network object
 	network := &network{
 		name:      name,
 		id:        types.UUID(stringid.GenerateRandomID()),
 		ctrlr:     c,
-		driver: nd}
+		driver:    d,
-	network.endpoints = make(endpointTable)
+		endpoints: endpointTable{},
-
-	d := network.driver.internalDriver
-	if d == nil {
-		return nil, ErrInvalidNetworkDriver
 	}
 
+	// Create the network
 	if err := d.CreateNetwork(network.id, options); err != nil {
 		return nil, err
 	}
 
+	// Store the network handler in controller
 	c.Lock()
 	c.networks[network.id] = network
 	c.Unlock()
@@ -201,7 +192,7 @@ func (n *network) Type() string {
 		return ""
 	}
 
-	return n.driver.networkType
+	return n.driver.Type()
 }
 
 func (n *network) Delete() error {
@@ -232,8 +223,7 @@ func (n *network) Delete() error {
 		}
 	}()
 
-	d := n.driver.internalDriver
+	err = n.driver.DeleteNetwork(n.id)
-	err = d.DeleteNetwork(n.id)
 	return err
 }
 
@@ -242,7 +232,7 @@ func (n *network) CreateEndpoint(name string, sboxKey string, options interface{
 	ep.id = types.UUID(stringid.GenerateRandomID())
 	ep.network = n
 
-	d := n.driver.internalDriver
+	d := n.driver
 	sinfo, err := d.CreateEndpoint(n.id, ep.id, sboxKey, options)
 	if err != nil {
 		return nil, err
@@ -310,7 +300,6 @@ func (ep *endpoint) Delete() error {
 		}
 	}()
 
-	d := n.driver.internalDriver
+	err = n.driver.DeleteEndpoint(n.id, ep.id)
-	err = d.DeleteEndpoint(n.id, ep.id)
 	return err
 }