libnet: convert to new-style driver registration

Per the Interface Segregation Principle, network drivers should not have to depend on GetPluginGetter methods they do not use. The remote network driver is the only one which needs a PluginGetter, and it is already special-cased in Controller so there is no sense warping the interfaces to achieve a foolish consistency. Replace all other network drivers' Init functions with Register functions which take a driverapi.Registerer argument instead of a driverapi.DriverCallback. Add back in Init wrapper functions for only the drivers which Swarmkit references so that Swarmkit can continue to build. Refactor the libnetwork Controller to use the new drvregistry.Networks and drvregistry.IPAMs driver registries in place of the legacy ones. Signed-off-by: Cory Snider <csnider@mirantis.com>
moby · Jan 27, 2023 · 28edc8e · 28edc8e
1 parent 5595311
commit 28edc8e
Show file tree

Hide file tree

Showing 23 changed files with 124 additions and 104 deletions.
diff --git a/libnetwork/cmd/ovrouter/ovrouter.go b/libnetwork/cmd/ovrouter/ovrouter.go
@@ -13,7 +13,6 @@ import (
 	"github.com/docker/docker/libnetwork/drivers/overlay"
 	"github.com/docker/docker/libnetwork/netlabel"
 	"github.com/docker/docker/libnetwork/types"
-	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/vishvananda/netlink"
 )
@@ -28,10 +27,6 @@ type endpoint struct {
 	name string
 }
 
-func (r *router) GetPluginGetter() plugingetter.PluginGetter {
-	return nil
-}
-
 func (r *router) RegisterDriver(name string, driver driverapi.Driver, c driverapi.Capability) error {
 	r.d = driver
 	return nil
@@ -126,7 +121,7 @@ func main() {
 	}
 
 	r := &router{}
-	if err := overlay.Init(r, opt); err != nil {
+	if err := overlay.Register(r, opt); err != nil {
 		fmt.Printf("Failed to initialize overlay driver: %v\n", err)
 		os.Exit(1)
 	}

diff --git a/libnetwork/controller.go b/libnetwork/controller.go
@@ -58,6 +58,7 @@ import (
 	"github.com/docker/docker/libnetwork/diagnostic"
 	"github.com/docker/docker/libnetwork/discoverapi"
 	"github.com/docker/docker/libnetwork/driverapi"
+	remotedriver "github.com/docker/docker/libnetwork/drivers/remote"
 	"github.com/docker/docker/libnetwork/drvregistry"
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/netlabel"
@@ -85,7 +86,8 @@ type sandboxTable map[string]*Sandbox
 // Controller manages networks.
 type Controller struct {
 	id               string
-	drvRegistry      *drvregistry.DrvRegistry
+	drvRegistry      drvregistry.Networks
+	ipamRegistry     drvregistry.IPAMs
 	sandboxes        sandboxTable
 	cfg              *config.Config
 	store            datastore.DataStore
@@ -108,7 +110,7 @@ type Controller struct {
 }
 
 type initializer struct {
-	fn    drvregistry.InitFunc
+	fn    func(driverapi.Registerer, map[string]interface{}) error
 	ntype string
 }
 
@@ -130,31 +132,24 @@ func New(cfgOptions ...config.Option) (*Controller, error) {
 		return nil, err
 	}
 
-	drvRegistry, err := drvregistry.New(nil, nil, c.RegisterDriver, nil, c.cfg.PluginGetter)
-	if err != nil {
+	c.drvRegistry.Notify = c.RegisterDriver
+
+	// External plugins don't need config passed through daemon. They can
+	// bootstrap themselves.
+	if err := remotedriver.Register(&c.drvRegistry, c.cfg.PluginGetter); err != nil {
 		return nil, err
 	}
 
 	for _, i := range getInitializers() {
-		var dcfg map[string]interface{}
-
-		// External plugins don't need config passed through daemon. They can
-		// bootstrap themselves
-		if i.ntype != "remote" {
-			dcfg = c.makeDriverConfig(i.ntype)
-		}
-
-		if err := drvRegistry.AddDriver(i.ntype, i.fn, dcfg); err != nil {
+		if err := i.fn(&c.drvRegistry, c.makeDriverConfig(i.ntype)); err != nil {
 			return nil, err
 		}
 	}
 
-	if err = initIPAMDrivers(drvRegistry, c.cfg.PluginGetter, c.cfg.DefaultAddressPool); err != nil {
+	if err := initIPAMDrivers(&c.ipamRegistry, c.cfg.PluginGetter, c.cfg.DefaultAddressPool); err != nil {
 		return nil, err
 	}
 
-	c.drvRegistry = drvRegistry
-
 	c.WalkNetworks(populateSpecial)
 
 	// Reserve pools first before doing cleanup. Otherwise the
@@ -387,7 +382,7 @@ func (c *Controller) BuiltinDrivers() []string {
 // BuiltinIPAMDrivers returns the list of builtin ipam drivers.
 func (c *Controller) BuiltinIPAMDrivers() []string {
 	drivers := []string{}
-	c.drvRegistry.WalkIPAMs(func(name string, driver ipamapi.Ipam, cap *ipamapi.Capability) bool {
+	c.ipamRegistry.WalkIPAMs(func(name string, driver ipamapi.Ipam, cap *ipamapi.Capability) bool {
 		if driver.IsBuiltIn() {
 			drivers = append(drivers, name)
 		}
@@ -461,7 +456,7 @@ func (c *Controller) isDistributedControl() bool {
 }
 
 func (c *Controller) GetPluginGetter() plugingetter.PluginGetter {
-	return c.drvRegistry.GetPluginGetter()
+	return c.cfg.PluginGetter
 }
 
 func (c *Controller) RegisterDriver(networkType string, driver driverapi.Driver, capability driverapi.Capability) error {
@@ -476,7 +471,7 @@ const overlayDSROptionString = "dsr"
 // are network specific and modeled in a generic way.
 func (c *Controller) NewNetwork(networkType, name string, id string, options ...NetworkOption) (Network, error) {
 	var (
-		caps           *driverapi.Capability
+		caps           driverapi.Capability
 		err            error
 		t              *network
 		skipCfgEpCount bool
@@ -1101,15 +1096,15 @@ func (c *Controller) loadIPAMDriver(name string) error {
 }
 
 func (c *Controller) getIPAMDriver(name string) (ipamapi.Ipam, *ipamapi.Capability, error) {
-	id, cap := c.drvRegistry.IPAM(name)
+	id, cap := c.ipamRegistry.IPAM(name)
 	if id == nil {
 		// Might be a plugin name. Try loading it
 		if err := c.loadIPAMDriver(name); err != nil {
 			return nil, nil, err
 		}
 
 		// Now that we resolved the plugin, try again looking up the registry
-		id, cap = c.drvRegistry.IPAM(name)
+		id, cap = c.ipamRegistry.IPAM(name)
 		if id == nil {
 			return nil, nil, types.BadRequestErrorf("invalid ipam driver: %q", name)
 		}

diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go
@@ -166,8 +166,8 @@ func newDriver() *driver {
 	}
 }
 
-// Init registers a new instance of bridge driver
-func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+// Register registers a new instance of bridge driver.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	d := newDriver()
 	if err := d.configure(config); err != nil {
 		return err
@@ -177,7 +177,7 @@ func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.LocalScope,
 	}
-	return dc.RegisterDriver(networkType, d, c)
+	return r.RegisterDriver(networkType, d, c)
 }
 
 // Validate performs a static validation on the network configuration parameters.

diff --git a/libnetwork/drivers/bridge/brmanager/brmanager.go b/libnetwork/drivers/bridge/brmanager/brmanager.go
@@ -11,13 +11,20 @@ const networkType = "bridge"
 
 type driver struct{}
 
-// Init registers a new instance of bridge manager driver
+// Init registers a new instance of bridge manager driver.
+//
+// Deprecated: use [Register].
 func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+	return Register(dc, config)
+}
+
+// Register registers a new instance of the bridge manager driver with r.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.LocalScope,
 	}
-	return dc.RegisterDriver(networkType, &driver{}, c)
+	return r.RegisterDriver(networkType, &driver{}, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/host/host.go b/libnetwork/drivers/host/host.go
@@ -16,13 +16,19 @@ type driver struct {
 	sync.Mutex
 }
 
-// Init registers a new instance of host driver
+// Init registers a new instance of host driver.
+//
+// Deprecated: use [Register].
 func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+	return Register(dc, config)
+}
+
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.LocalScope,
 	}
-	return dc.RegisterDriver(networkType, &driver{}, c)
+	return r.RegisterDriver(networkType, &driver{}, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/ipvlan/ipvlan.go b/libnetwork/drivers/ipvlan/ipvlan.go
@@ -62,8 +62,8 @@ type network struct {
 	sync.Mutex
 }
 
-// Init initializes and registers the libnetwork ipvlan driver
-func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+// Register initializes and registers the libnetwork ipvlan driver.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.GlobalScope,
@@ -75,7 +75,7 @@ func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
 		return err
 	}
 
-	return dc.RegisterDriver(driverName, d, c)
+	return r.RegisterDriver(driverName, d, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/ipvlan/ipvlan_test.go b/libnetwork/drivers/ipvlan/ipvlan_test.go
@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/docker/docker/libnetwork/driverapi"
-	"github.com/docker/docker/pkg/plugingetter"
 )
 
 const testNetworkType = "ipvlan"
@@ -17,10 +16,6 @@ type driverTester struct {
 	d *driver
 }
 
-func (dt *driverTester) GetPluginGetter() plugingetter.PluginGetter {
-	return nil
-}
-
 func (dt *driverTester) RegisterDriver(name string, drv driverapi.Driver,
 	cap driverapi.Capability) error {
 	if name != testNetworkType {
@@ -37,15 +32,15 @@ func (dt *driverTester) RegisterDriver(name string, drv driverapi.Driver,
 	return nil
 }
 
-func TestIpvlanInit(t *testing.T) {
-	if err := Init(&driverTester{t: t}, nil); err != nil {
+func TestIpvlanRegister(t *testing.T) {
+	if err := Register(&driverTester{t: t}, nil); err != nil {
 		t.Fatal(err)
 	}
 }
 
 func TestIpvlanNilConfig(t *testing.T) {
 	dt := &driverTester{t: t}
-	if err := Init(dt, nil); err != nil {
+	if err := Register(dt, nil); err != nil {
 		t.Fatal(err)
 	}
 
@@ -56,7 +51,7 @@ func TestIpvlanNilConfig(t *testing.T) {
 
 func TestIpvlanType(t *testing.T) {
 	dt := &driverTester{t: t}
-	if err := Init(dt, nil); err != nil {
+	if err := Register(dt, nil); err != nil {
 		t.Fatal(err)
 	}
 

diff --git a/libnetwork/drivers/ipvlan/ivmanager/ivmanager.go b/libnetwork/drivers/ipvlan/ivmanager/ivmanager.go
@@ -11,13 +11,20 @@ const networkType = "ipvlan"
 
 type driver struct{}
 
-// Init registers a new instance of ipvlan manager driver
+// Init registers a new instance of the ipvlan manager driver.
+//
+// Deprecated: use [Register].
 func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+	return Register(dc, config)
+}
+
+// Register registers a new instance of the ipvlan manager driver.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.GlobalScope,
 	}
-	return dc.RegisterDriver(networkType, &driver{}, c)
+	return r.RegisterDriver(networkType, &driver{}, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/macvlan/macvlan.go b/libnetwork/drivers/macvlan/macvlan.go
@@ -56,8 +56,8 @@ type network struct {
 	sync.Mutex
 }
 
-// Init initializes and registers the libnetwork macvlan driver
-func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+// Register initializes and registers the libnetwork macvlan driver
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.GlobalScope,
@@ -69,7 +69,7 @@ func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
 		return err
 	}
 
-	return dc.RegisterDriver(driverName, d, c)
+	return r.RegisterDriver(driverName, d, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/macvlan/macvlan_test.go b/libnetwork/drivers/macvlan/macvlan_test.go
@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/docker/docker/libnetwork/driverapi"
-	"github.com/docker/docker/pkg/plugingetter"
 )
 
 const testNetworkType = "macvlan"
@@ -17,10 +16,6 @@ type driverTester struct {
 	d *driver
 }
 
-func (dt *driverTester) GetPluginGetter() plugingetter.PluginGetter {
-	return nil
-}
-
 func (dt *driverTester) RegisterDriver(name string, drv driverapi.Driver,
 	cap driverapi.Capability) error {
 	if name != testNetworkType {
@@ -37,15 +32,15 @@ func (dt *driverTester) RegisterDriver(name string, drv driverapi.Driver,
 	return nil
 }
 
-func TestMacvlanInit(t *testing.T) {
-	if err := Init(&driverTester{t: t}, nil); err != nil {
+func TestMacvlanRegister(t *testing.T) {
+	if err := Register(&driverTester{t: t}, nil); err != nil {
 		t.Fatal(err)
 	}
 }
 
 func TestMacvlanNilConfig(t *testing.T) {
 	dt := &driverTester{t: t}
-	if err := Init(dt, nil); err != nil {
+	if err := Register(dt, nil); err != nil {
 		t.Fatal(err)
 	}
 
@@ -56,7 +51,7 @@ func TestMacvlanNilConfig(t *testing.T) {
 
 func TestMacvlanType(t *testing.T) {
 	dt := &driverTester{t: t}
-	if err := Init(dt, nil); err != nil {
+	if err := Register(dt, nil); err != nil {
 		t.Fatal(err)
 	}
 

diff --git a/libnetwork/drivers/macvlan/mvmanager/mvmanager.go b/libnetwork/drivers/macvlan/mvmanager/mvmanager.go
@@ -11,13 +11,20 @@ const networkType = "macvlan"
 
 type driver struct{}
 
-// Init registers a new instance of macvlan manager driver
+// Init registers a new instance of the macvlan manager driver.
+//
+// Deprecated: use [Register].
 func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+	return Register(dc, config)
+}
+
+// Register registers a new instance of the macvlan manager driver.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope:         datastore.LocalScope,
 		ConnectivityScope: datastore.GlobalScope,
 	}
-	return dc.RegisterDriver(networkType, &driver{}, c)
+	return r.RegisterDriver(networkType, &driver{}, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {

diff --git a/libnetwork/drivers/null/null.go b/libnetwork/drivers/null/null.go
@@ -16,12 +16,12 @@ type driver struct {
 	sync.Mutex
 }
 
-// Init registers a new instance of null driver
-func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
+// Register registers a new instance of the null driver.
+func Register(r driverapi.Registerer, config map[string]interface{}) error {
 	c := driverapi.Capability{
 		DataScope: datastore.LocalScope,
 	}
-	return dc.RegisterDriver(networkType, &driver{}, c)
+	return r.RegisterDriver(networkType, &driver{}, c)
 }
 
 func (d *driver) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) {