Merge pull request #143 from squaremo/driver_init_not_new

Make driver packages register themselves via DriverCallback

libnetwork/cmd/readme_test/readme.go

@@ -11,7 +11,10 @@ import (
 
 func main() {
 	// Create a new controller instance
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		return
+	}
 
 	// Select and configure the network driver
 	networkType := "bridge"
@@ -19,7 +22,7 @@ func main() {
 	driverOptions := options.Generic{}
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = driverOptions
-	err := controller.ConfigureNetworkDriver(networkType, genericOption)
+	err = controller.ConfigureNetworkDriver(networkType, genericOption)
 	if err != nil {
 		return
 	}

libnetwork/cmd/test/main.go

@@ -14,9 +14,12 @@ func main() {
 	net.IP = ip
 
 	options := options.Generic{"AddressIPv4": net}
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		log.Fatal(err)
+	}
 	netType := "bridge"
-	err := controller.ConfigureNetworkDriver(netType, options)
+	err = controller.ConfigureNetworkDriver(netType, options)
 	netw, err := controller.NewNetwork(netType, "dummy")
 	if err != nil {
 		log.Fatal(err)

libnetwork/controller.go

@@ -3,7 +3,7 @@ Package libnetwork provides the basic functionality and extension points to
 create network namespaces and allocate interfaces for containers to use.
 
         // Create a new controller instance
-        controller := libnetwork.New()
+        controller, _err := libnetwork.New()
 
         // Select and configure the network driver
         networkType := "bridge"
@@ -98,11 +98,15 @@ type controller struct {
 }
 
 // New creates a new instance of network controller.
-func New() NetworkController {
-	c := &controller{networks: networkTable{}, sandboxes: sandboxTable{}}
-	c.drivers = enumerateDrivers(c)
-	return c
-
+func New() (NetworkController, error) {
+	c := &controller{
+		networks:  networkTable{},
+		sandboxes: sandboxTable{},
+		drivers:   driverTable{}}
+	if err := initDrivers(c); err != nil {
+		return nil, err
+	}
+	return c, nil
 }
 
 func (c *controller) ConfigureNetworkDriver(networkType string, options map[string]interface{}) error {

libnetwork/docs/design.md

@@ -119,7 +119,7 @@ The APIs are still work in progress and there can be changes to these based on t
 
 ## Implementations
 
-Libnetwork includes the following drivers:
+Libnetwork includes the following driver packages:
 
 - null
 - bridge
@@ -142,7 +142,7 @@ For more details on its design, please see the [Overlay Driver Design](overlay.m
 
 ### Remote
 
-The `remote` driver, provides a means of supporting drivers over a remote transport.
+The `remote` package does not provide a driver, but provides a means of supporting drivers over a remote transport.
 This allows a driver to be written in a language of your choice.
 For further details, please see the [Remote Driver Design](remote.md)
 

libnetwork/docs/remote.md

@@ -1,17 +1,13 @@
-Remote Driver
-=============
+Remote Drivers
+==============
 
-Remote driver is a special built-in driver. This driver in itself doesn't provide any networking functionality. But it depends on actual remote drivers aka `Dynamic Drivers` to provide the required networking between the containers. The dynamic drivers (such as : Weave, OVS, OVN, ACI, Calico and external networking plugins) registers itself with the Build-In `Remote` Driver.
+The remote driver package provides the integration point for dynamically-registered drivers.
 
 ## LibNetwork Integration
 
-When LibNetwork creates an instance of the Built-in `Remote` Driver via the `New()` function, it passes a `DriverCallback` as a parameter which implements the `RegisterDriver()`. The Built-in Remote Driver can use this interface to register any of the `Dynamic` Drivers/Plugins with LibNetwork's `NetworkController`
+When LibNetwork initialises the `Remote` package with the `Init()` function, it passes a `DriverCallback` as a parameter, which implements the `RegisterDriver()`. The Remote Driver package can use this interface to register any of the `Dynamic` Drivers/Plugins with LibNetwork's `NetworkController`.
 
-Please Refer to [Remote Driver Test](https://github.com/docker/libnetwork/blob/master/drivers/remote/driver_test.go) which provides an example of registering a Dynamic driver with LibNetwork.
-
-This design ensures that the implementation details of Dynamic Driver Registration mechanism is completely owned by the inbuilt-Remote driver and it also doesnt expose any of the driver layer to the North of LibNetwork (none of the LibNetwork client APIs are impacted).
-
-When the inbuilt `Remote` driver detects a `Dynamic` Driver it can call the `registerRemoteDriver` method. This method will take care of creating a new `Remote` Driver instance with the passed 'NetworkType' and register it with Libnetwork's 'NetworkController
+This design ensures that the implementation details (TBD) of Dynamic Driver Registration mechanism is completely owned by the inbuilt-Remote driver, and it doesn't expose any of the driver layer to the North of LibNetwork (none of the LibNetwork client APIs are impacted).
 
 ## Implementation
 

libnetwork/drivers.go

@@ -10,18 +10,16 @@ import (
 
 type driverTable map[string]driverapi.Driver
 
-func enumerateDrivers(dc driverapi.DriverCallback) driverTable {
-	drivers := make(driverTable)
-
-	for _, fn := range [](func(driverapi.DriverCallback) (string, driverapi.Driver)){
-		bridge.New,
-		host.New,
-		null.New,
-		remote.New,
+func initDrivers(dc driverapi.DriverCallback) error {
+	for _, fn := range [](func(driverapi.DriverCallback) error){
+		bridge.Init,
+		host.Init,
+		null.Init,
+		remote.Init,
 	} {
-		name, driver := fn(dc)
-		drivers[name] = driver
+		if err := fn(dc); err != nil {
+			return err
+		}
 	}
-
-	return drivers
+	return nil
 }

libnetwork/drivers/bridge/bridge.go

@@ -92,9 +92,14 @@ func init() {
 	portMapper = portmapper.New()
 }
 
-// New provides a new instance of bridge driver
-func New(dc driverapi.DriverCallback) (string, driverapi.Driver) {
-	return networkType, &driver{}
+// New constructs a new bridge driver
+func newDriver() driverapi.Driver {
+	return &driver{}
+}
+
+// Init registers a new instance of bridge driver
+func Init(dc driverapi.DriverCallback) error {
+	return dc.RegisterDriver(networkType, newDriver())
 }
 
 // Validate performs a static validation on the network configuration parameters.

libnetwork/drivers/bridge/bridge_test.go

@@ -15,7 +15,7 @@ import (
 
 func TestCreateFullOptions(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &Configuration{
 		EnableIPForwarding: true,
@@ -46,7 +46,7 @@ func TestCreateFullOptions(t *testing.T) {
 
 func TestCreate(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{BridgeName: DefaultBridgeName}
 	genericOption := make(map[string]interface{})
@@ -59,7 +59,7 @@ func TestCreate(t *testing.T) {
 
 func TestCreateFail(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{BridgeName: "dummy0"}
 	genericOption := make(map[string]interface{})
@@ -72,8 +72,8 @@ func TestCreateFail(t *testing.T) {
 
 func TestQueryEndpointInfo(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-
-	_, d := New(nil)
+	d := newDriver()
+	dd, _ := d.(*driver)
 
 	config := &NetworkConfiguration{
 		BridgeName:     DefaultBridgeName,
@@ -97,7 +97,6 @@ func TestQueryEndpointInfo(t *testing.T) {
 		t.Fatalf("Failed to create an endpoint : %s", err.Error())
 	}
 
-	dd := d.(*driver)
 	ep, _ := dd.network.endpoints["ep1"]
 	data, err := d.EndpointInfo(dd.network.id, ep.id)
 	if err != nil {
@@ -129,8 +128,7 @@ func TestQueryEndpointInfo(t *testing.T) {
 
 func TestCreateLinkWithOptions(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{BridgeName: DefaultBridgeName}
 	netOptions := make(map[string]interface{})
@@ -180,7 +178,7 @@ func getPortMapping() []netutils.PortBinding {
 func TestLinkContainers(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
 
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{
 		BridgeName:     DefaultBridgeName,
@@ -385,7 +383,7 @@ func TestValidateConfig(t *testing.T) {
 
 func TestSetDefaultGw(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	_, subnetv6, _ := net.ParseCIDR("2001:db8:ea9:9abc:b0c4::/80")
 	gw4 := bridgeNetworks[0].IP.To4()

libnetwork/drivers/bridge/network_test.go

@@ -11,7 +11,7 @@ import (
 
 func TestLinkCreate(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 	dr := d.(*driver)
 
 	mtu := 1490
@@ -97,7 +97,7 @@ func TestLinkCreate(t *testing.T) {
 
 func TestLinkCreateTwo(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{
 		BridgeName: DefaultBridgeName,
@@ -127,7 +127,7 @@ func TestLinkCreateTwo(t *testing.T) {
 
 func TestLinkCreateNoEnableIPv6(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{
 		BridgeName: DefaultBridgeName}
@@ -156,7 +156,7 @@ func TestLinkCreateNoEnableIPv6(t *testing.T) {
 
 func TestLinkDelete(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	config := &NetworkConfiguration{
 		BridgeName: DefaultBridgeName,

libnetwork/drivers/bridge/port_mapping_test.go

@@ -18,7 +18,7 @@ func TestMain(m *testing.M) {
 
 func TestPortMappingConfig(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	_, d := New(nil)
+	d := newDriver()
 
 	binding1 := netutils.PortBinding{Proto: netutils.UDP, Port: uint16(400), HostPort: uint16(54000)}
 	binding2 := netutils.PortBinding{Proto: netutils.TCP, Port: uint16(500), HostPort: uint16(65000)}

libnetwork/drivers/host/host.go

@@ -10,9 +10,9 @@ const networkType = "host"
 
 type driver struct{}
 
-// New provides a new instance of host driver
-func New(dc driverapi.DriverCallback) (string, driverapi.Driver) {
-	return networkType, &driver{}
+// Init registers a new instance of host driver
+func Init(dc driverapi.DriverCallback) error {
+	return dc.RegisterDriver(networkType, &driver{})
 }
 
 func (d *driver) Config(option map[string]interface{}) error {

libnetwork/drivers/null/null.go

@@ -10,9 +10,9 @@ const networkType = "null"
 
 type driver struct{}
 
-// New provides a new instance of null driver
-func New(dc driverapi.DriverCallback) (string, driverapi.Driver) {
-	return networkType, &driver{}
+// Init registers a new instance of null driver
+func Init(dc driverapi.DriverCallback) error {
+	return dc.RegisterDriver(networkType, &driver{})
 }
 
 func (d *driver) Config(option map[string]interface{}) error {

libnetwork/drivers/remote/driver.go

@@ -13,28 +13,11 @@ var errNoCallback = errors.New("No Callback handler registered with Driver")
 const remoteNetworkType = "remote"
 
 type driver struct {
-	networkType string
-	callback    driverapi.DriverCallback
 }
 
-// New instance of remote driver returned to LibNetwork
-func New(dc driverapi.DriverCallback) (string, driverapi.Driver) {
-	d := &driver{networkType: remoteNetworkType}
-	d.callback = dc
-	return d.networkType, d
-}
-
-// Internal Convenience method to register a remote driver.
-// The implementation of this method will change based on the dynamic driver registration design
-func (d *driver) registerRemoteDriver(networkType string) (driverapi.Driver, error) {
-	newDriver := &driver{networkType: networkType}
-	if d.callback == nil {
-		return nil, errNoCallback
-	}
-	if err := d.callback.RegisterDriver(networkType, newDriver); err != nil {
-		return nil, err
-	}
-	return newDriver, nil
+// Init does the necessary work to register remote drivers
+func Init(dc driverapi.DriverCallback) error {
+	return nil
 }
 
 func (d *driver) Config(option map[string]interface{}) error {
@@ -72,5 +55,5 @@ func (d *driver) Leave(nid, eid types.UUID, options map[string]interface{}) erro
 }
 
 func (d *driver) Type() string {
-	return d.networkType
+	return remoteNetworkType
 }

libnetwork/drivers/remote/driver_test.go

@@ -1,29 +0,0 @@
-package remote
-
-import (
-	"testing"
-
-	"github.com/docker/libnetwork/driverapi"
-)
-
-type testCallbackStruct struct {
-	networkType string
-}
-
-func (t *testCallbackStruct) RegisterDriver(networkType string, driver driverapi.Driver) error {
-	t.networkType = networkType
-	return nil
-}
-
-func TestCallback(t *testing.T) {
-	tc := &testCallbackStruct{}
-	_, d := New(tc)
-	expected := "test-dummy"
-	_, err := d.(*driver).registerRemoteDriver(expected)
-	if err != nil {
-		t.Fatalf("Remote Driver callback registration failed with Error : %v", err)
-	}
-	if tc.networkType != expected {
-		t.Fatal("Remote Driver Callback Registration failed")
-	}
-}

libnetwork/libnetwork_internal_test.go

@@ -8,8 +8,11 @@ import (
 
 func TestDriverRegistration(t *testing.T) {
 	bridgeNetType := "bridge"
-	c := New()
-	err := c.(*controller).RegisterDriver(bridgeNetType, nil)
+	c, err := New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = c.(*controller).RegisterDriver(bridgeNetType, nil)
 	if err == nil {
 		t.Fatalf("Expecting the RegisterDriver to fail for %s", bridgeNetType)
 	}

libnetwork/libnetwork_test.go

@@ -34,11 +34,14 @@ func TestMain(m *testing.M) {
 }
 
 func createTestNetwork(networkType, networkName string, option options.Generic, netOption options.Generic) (libnetwork.Network, error) {
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		return nil, err
+	}
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = option
 
-	err := controller.ConfigureNetworkDriver(networkType, genericOption)
+	err = controller.ConfigureNetworkDriver(networkType, genericOption)
 	if err != nil {
 		return nil, err
 	}
@@ -219,9 +222,12 @@ func TestUnknownDriver(t *testing.T) {
 }
 
 func TestNilDriver(t *testing.T) {
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	_, err := controller.NewNetwork("framerelay", "dummy",
+	_, err = controller.NewNetwork("framerelay", "dummy",
 		libnetwork.NetworkOptionGeneric(getEmptyGenericOption()))
 	if err == nil {
 		t.Fatal("Expected to fail. But instead succeeded")
@@ -233,9 +239,12 @@ func TestNilDriver(t *testing.T) {
 }
 
 func TestNoInitDriver(t *testing.T) {
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	_, err := controller.NewNetwork("ppp", "dummy",
+	_, err = controller.NewNetwork("ppp", "dummy",
 		libnetwork.NetworkOptionGeneric(getEmptyGenericOption()))
 	if err == nil {
 		t.Fatal("Expected to fail. But instead succeeded")
@@ -248,12 +257,15 @@ func TestNoInitDriver(t *testing.T) {
 
 func TestDuplicateNetwork(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	genericOption := make(map[string]interface{})
 	genericOption[netlabel.GenericData] = options.Generic{}
 
-	err := controller.ConfigureNetworkDriver(bridgeNetType, genericOption)
+	err = controller.ConfigureNetworkDriver(bridgeNetType, genericOption)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -435,9 +447,12 @@ func TestUnknownEndpoint(t *testing.T) {
 
 func TestNetworkEndpointsWalkers(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	err := controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
+	err = controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -513,9 +528,12 @@ func TestNetworkEndpointsWalkers(t *testing.T) {
 
 func TestControllerQuery(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	err := controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
+	err = controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -573,9 +591,12 @@ func TestControllerQuery(t *testing.T) {
 
 func TestNetworkQuery(t *testing.T) {
 	defer netutils.SetupTestNetNS(t)()
-	controller := libnetwork.New()
+	controller, err := libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	err := controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
+	err = controller.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1031,7 +1052,10 @@ func createGlobalInstance(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ctrlr = libnetwork.New()
+	ctrlr, err = libnetwork.New()
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	err = ctrlr.ConfigureNetworkDriver(bridgeNetType, getEmptyGenericOption())
 	if err != nil {