Make driver packages register themselves via DriverCallback

In the present code, each driver package provides a `New()` method
which constructs a driver of its type, which is then registered with
the controller.

However, this is not suitable for the `drivers/remote` package, since
it does not provide a (singleton) driver, but a mechanism for drivers
to be added dynamically. As a result, the implementation is oddly
dual-purpose, and a spurious `"remote"` driver is added to the
controller's list of available drivers.

Instead, it is better to provide the registration callback to each
package and let it register its own driver or drivers. That way, the
singleton driver packages can construct one and register it, and the
remote package can hook the callback up with whatever the dynamic
driver mechanism turns out to be.

NB there are some method signature changes; in particular to
controller.New, which can return an error if the built-in driver
packages fail to initialise.

Signed-off-by: Michael Bridgen <mikeb@squaremobius.net>

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)
 	}
@@ -557,9 +575,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)
 	}
@@ -1004,7 +1025,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 {