From 614d82390ccc135aa9f31a3ff3953759d511d9d4 Mon Sep 17 00:00:00 2001
From: Michael Bridgen <mikeb@squaremobius.net>
Date: Mon, 11 May 2015 13:46:29 +0100
Subject: [PATCH] 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 | 7 ++-
libnetwork/cmd/test/main.go | 7 ++-
libnetwork/controller.go | 16 +++---
libnetwork/docs/design.md | 4 +-
libnetwork/docs/remote.md | 14 ++---
libnetwork/drivers.go | 22 ++++----
libnetwork/drivers/bridge/bridge.go | 11 ++--
libnetwork/drivers/bridge/bridge_test.go | 18 +++----
libnetwork/drivers/bridge/network_test.go | 8 +--
.../drivers/bridge/port_mapping_test.go | 2 +-
libnetwork/drivers/host/host.go | 6 +--
libnetwork/drivers/null/null.go | 6 +--
libnetwork/drivers/remote/driver.go | 25 ++-------
libnetwork/drivers/remote/driver_test.go | 29 ----------
libnetwork/libnetwork_internal_test.go | 7 ++-
libnetwork/libnetwork_test.go | 54 +++++++++++++------
16 files changed, 112 insertions(+), 124 deletions(-)
delete mode 100644 libnetwork/drivers/remote/driver_test.go
diff --git a/libnetwork/cmd/readme_test/readme.go b/libnetwork/cmd/readme_test/readme.go
index 319399ae30..ef21f4887b 100644
--- a/libnetwork/cmd/readme_test/readme.go
+++ b/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
}
diff --git a/libnetwork/cmd/test/main.go b/libnetwork/cmd/test/main.go
index 07ae996327..98d7cff358 100644
--- a/libnetwork/cmd/test/main.go
+++ b/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)
diff --git a/libnetwork/controller.go b/libnetwork/controller.go
index f4b5dd781d..2fcb115f07 100644
--- a/libnetwork/controller.go
+++ b/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 {
diff --git a/libnetwork/docs/design.md b/libnetwork/docs/design.md
index 5f43250967..657227e262 100644
--- a/libnetwork/docs/design.md
+++ b/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)
diff --git a/libnetwork/docs/remote.md b/libnetwork/docs/remote.md
index 5d552972a4..c34a1cd0b0 100644
--- a/libnetwork/docs/remote.md
+++ b/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
diff --git a/libnetwork/drivers.go b/libnetwork/drivers.go
index febc2988f2..130f7ab343 100644
--- a/libnetwork/drivers.go
+++ b/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
}
diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go
index 81180fcf55..4c0355e1b9 100644
--- a/libnetwork/drivers/bridge/bridge.go
+++ b/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.
diff --git a/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go
index 2e3ecebfd3..4bc77d0d04 100644
--- a/libnetwork/drivers/bridge/bridge_test.go
+++ b/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()
diff --git a/libnetwork/drivers/bridge/network_test.go b/libnetwork/drivers/bridge/network_test.go
index 8f0b3a6f34..daa9c58588 100644
--- a/libnetwork/drivers/bridge/network_test.go
+++ b/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,
diff --git a/libnetwork/drivers/bridge/port_mapping_test.go b/libnetwork/drivers/bridge/port_mapping_test.go
index 93c5788364..eaa5479135 100644
--- a/libnetwork/drivers/bridge/port_mapping_test.go
+++ b/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)}
diff --git a/libnetwork/drivers/host/host.go b/libnetwork/drivers/host/host.go
index cd39ad0977..79c94abba7 100644
--- a/libnetwork/drivers/host/host.go
+++ b/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 {
diff --git a/libnetwork/drivers/null/null.go b/libnetwork/drivers/null/null.go
index e2ba62b5d9..493fa483a1 100644
--- a/libnetwork/drivers/null/null.go
+++ b/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 {
diff --git a/libnetwork/drivers/remote/driver.go b/libnetwork/drivers/remote/driver.go
index c23ee0e332..9ff7c37cd6 100644
--- a/libnetwork/drivers/remote/driver.go
+++ b/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
}
diff --git a/libnetwork/drivers/remote/driver_test.go b/libnetwork/drivers/remote/driver_test.go
deleted file mode 100644
index 42c7992fb2..0000000000
--- a/libnetwork/drivers/remote/driver_test.go
+++ /dev/null
@@ -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")
- }
-}
diff --git a/libnetwork/libnetwork_internal_test.go b/libnetwork/libnetwork_internal_test.go
index 2eeda80bb5..6a9a7fdc43 100644
--- a/libnetwork/libnetwork_internal_test.go
+++ b/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)
}
diff --git a/libnetwork/libnetwork_test.go b/libnetwork/libnetwork_test.go
index f5edbd8bb6..5140b92b12 100644
--- a/libnetwork/libnetwork_test.go
+++ b/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 {