From 470f0b502efd57040d3315b1d782a6b64554b714 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Jun 2023 14:39:44 +0200 Subject: [PATCH 1/4] cnmallocator: RegisterManager does not require DriverCallback interface This function was not using the DriverCallback interface, and only required the Registerer interface. Signed-off-by: Sebastiaan van Stijn --- manager/allocator/cnmallocator/manager.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/manager/allocator/cnmallocator/manager.go b/manager/allocator/cnmallocator/manager.go index bfc0e9a7af..0f86efd4ad 100644 --- a/manager/allocator/cnmallocator/manager.go +++ b/manager/allocator/cnmallocator/manager.go @@ -17,13 +17,12 @@ func StubManagerInit(networkType string) func(dc driverapi.DriverCallback, confi } } -// Register registers a new instance of the manager driver for networkType with r. -func RegisterManager(r driverapi.DriverCallback, networkType string) error { - c := driverapi.Capability{ +// RegisterManager registers a new instance of the manager driver for networkType with r. +func RegisterManager(r driverapi.Registerer, networkType string) error { + return r.RegisterDriver(networkType, &manager{networkType: networkType}, driverapi.Capability{ DataScope: datastore.LocalScope, ConnectivityScope: datastore.LocalScope, - } - return r.RegisterDriver(networkType, &manager{networkType: networkType}, c) + }) } func (d *manager) NetworkAllocate(id string, option map[string]string, ipV4Data, ipV6Data []driverapi.IPAMData) (map[string]string, error) { From d98f63ba10c2e4f3b11345de6ebb609958bdc347 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Jun 2023 12:38:00 +0200 Subject: [PATCH 2/4] manager/allocator/cnmallocator: use a map for initializers Refactor the initializers slice to be a map (as driver-names should be unique, and they were not dynamically set here either way). Also inlining the code from initializeDrivers(), as it was only used in a single location, and the code lived far from where it was used. Signed-off-by: Sebastiaan van Stijn --- .../allocator/cnmallocator/drivers_darwin.go | 6 ++-- .../cnmallocator/drivers_network_linux.go | 15 +++++----- .../cnmallocator/drivers_network_windows.go | 13 +++++---- .../cnmallocator/networkallocator.go | 29 ++++--------------- 4 files changed, 24 insertions(+), 39 deletions(-) diff --git a/manager/allocator/cnmallocator/drivers_darwin.go b/manager/allocator/cnmallocator/drivers_darwin.go index e7d3dbe710..f183ac478c 100644 --- a/manager/allocator/cnmallocator/drivers_darwin.go +++ b/manager/allocator/cnmallocator/drivers_darwin.go @@ -6,9 +6,9 @@ import ( "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) -var initializers = []initializer{ - {remote.Init, "remote"}, - {ovmanager.Init, "overlay"}, +var initializers = map[string]driverRegisterFn{ + "remote": remote.Init, + "overlay": ovmanager.Init, } // PredefinedNetworks returns the list of predefined network structures diff --git a/manager/allocator/cnmallocator/drivers_network_linux.go b/manager/allocator/cnmallocator/drivers_network_linux.go index 5ae9196977..1f67e1e2ce 100644 --- a/manager/allocator/cnmallocator/drivers_network_linux.go +++ b/manager/allocator/cnmallocator/drivers_network_linux.go @@ -7,16 +7,17 @@ import ( "github.com/docker/docker/libnetwork/drivers/macvlan/mvmanager" "github.com/docker/docker/libnetwork/drivers/overlay/ovmanager" "github.com/docker/docker/libnetwork/drivers/remote" + "github.com/docker/docker/libnetwork/drvregistry" "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) -var initializers = []initializer{ - {remote.Init, "remote"}, - {ovmanager.Init, "overlay"}, - {mvmanager.Init, "macvlan"}, - {brmanager.Init, "bridge"}, - {ivmanager.Init, "ipvlan"}, - {host.Init, "host"}, +var initializers = map[string]drvregistry.InitFunc{ + "remote": remote.Init, + "overlay": ovmanager.Init, + "macvlan": mvmanager.Init, + "bridge": brmanager.Init, + "ipvlan": ivmanager.Init, + "host": host.Init, } // PredefinedNetworks returns the list of predefined network structures diff --git a/manager/allocator/cnmallocator/drivers_network_windows.go b/manager/allocator/cnmallocator/drivers_network_windows.go index abc6c44aac..126cb85ed8 100644 --- a/manager/allocator/cnmallocator/drivers_network_windows.go +++ b/manager/allocator/cnmallocator/drivers_network_windows.go @@ -3,15 +3,16 @@ package cnmallocator import ( "github.com/docker/docker/libnetwork/drivers/overlay/ovmanager" "github.com/docker/docker/libnetwork/drivers/remote" + "github.com/docker/docker/libnetwork/drvregistry" "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) -var initializers = []initializer{ - {remote.Init, "remote"}, - {ovmanager.Init, "overlay"}, - {StubManagerInit("internal"), "internal"}, - {StubManagerInit("l2bridge"), "l2bridge"}, - {StubManagerInit("nat"), "nat"}, +var initializers = map[string]drvregistry.InitFunc{ + "remote": remote.Init, + "overlay": ovmanager.Init, + "internal": StubManagerInit("internal"), + "l2bridge": StubManagerInit("l2bridge"), + "nat": StubManagerInit("nat"), } // PredefinedNetworks returns the list of predefined network structures diff --git a/manager/allocator/cnmallocator/networkallocator.go b/manager/allocator/cnmallocator/networkallocator.go index 9aae338fa2..deb0cc706d 100644 --- a/manager/allocator/cnmallocator/networkallocator.go +++ b/manager/allocator/cnmallocator/networkallocator.go @@ -81,11 +81,6 @@ type networkDriver struct { capability *driverapi.Capability } -type initializer struct { - fn drvregistry.InitFunc - ntype string -} - // NetworkConfig is used to store network related cluster config in the Manager. type NetworkConfig struct { // DefaultAddrPool specifies default subnet pool for global scope networks @@ -115,8 +110,10 @@ func New(pg plugingetter.PluginGetter, netConfig *NetworkConfig) (networkallocat return nil, err } - if err := initializeDrivers(reg); err != nil { - return nil, err + for name, register := range initializers { + if err := register(reg, nil); err != nil { + return nil, fmt.Errorf("failed to register %q driver: %w", name, err) + } } if err = initIPAMDrivers(reg, netConfig); err != nil { @@ -978,15 +975,6 @@ func (na *cnmNetworkAllocator) allocatePools(n *api.Network) (map[string]string, return pools, nil } -func initializeDrivers(reg *drvregistry.DrvRegistry) error { - for _, i := range initializers { - if err := reg.AddDriver(i.ntype, i.fn, nil); err != nil { - return err - } - } - return nil -} - func serviceNetworks(s *api.Service) []*api.NetworkAttachmentConfig { // Always prefer NetworkAttachmentConfig in the TaskSpec if len(s.Spec.Task.Networks) == 0 && len(s.Spec.Networks) != 0 { @@ -1010,13 +998,8 @@ func (na *cnmNetworkAllocator) IsVIPOnIngressNetwork(vip *api.Endpoint_VirtualIP // IsBuiltInDriver returns whether the passed driver is an internal network driver func IsBuiltInDriver(name string) bool { - n := strings.ToLower(name) - for _, d := range initializers { - if n == d.ntype { - return true - } - } - return false + _, ok := initializers[strings.ToLower(name)] + return ok } // setIPAMSerialAlloc sets the ipam allocation method to serial From df3505350c75490bebb5ec060793499d9bebf06c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Jun 2023 13:21:01 +0200 Subject: [PATCH 3/4] manager/allocator/cnmallocator: remove uses of deprecated Init() funcs These functions were deprecated in https://github.com/moby/moby/commit/28edc8e2d692cb47f1ec6bb32a6e346777e1250d in favor of the Register functions. Unfortunately, not all Register functions have the same signature, so some (temporary) adaptor code was needed for these. Signed-off-by: Sebastiaan van Stijn --- manager/allocator/cnmallocator/drivers.go | 25 +++++++++++++++++++ .../allocator/cnmallocator/drivers_darwin.go | 5 ++-- .../cnmallocator/drivers_network_linux.go | 16 ++++++------ .../cnmallocator/drivers_network_windows.go | 14 +++++------ 4 files changed, 40 insertions(+), 20 deletions(-) create mode 100644 manager/allocator/cnmallocator/drivers.go diff --git a/manager/allocator/cnmallocator/drivers.go b/manager/allocator/cnmallocator/drivers.go new file mode 100644 index 0000000000..9bdbea9bc7 --- /dev/null +++ b/manager/allocator/cnmallocator/drivers.go @@ -0,0 +1,25 @@ +package cnmallocator + +import ( + "fmt" + + "github.com/docker/docker/libnetwork/driverapi" + "github.com/docker/docker/libnetwork/drivers/remote" +) + +type driverRegisterFn func(r driverapi.Registerer, config map[string]interface{}) error + +func registerRemote(r driverapi.Registerer, _ map[string]interface{}) error { + dc, ok := r.(driverapi.DriverCallback) + if !ok { + return fmt.Errorf(`failed to register "remote" driver: driver does not implement driverapi.DriverCallback`) + } + return remote.Register(dc, dc.GetPluginGetter()) +} + +//nolint:unused // is currently only used on Windows, but keeping these adaptors together in one file. +func registerNetworkType(networkType string) func(dc driverapi.Registerer, config map[string]interface{}) error { + return func(r driverapi.Registerer, _ map[string]interface{}) error { + return RegisterManager(r, networkType) + } +} diff --git a/manager/allocator/cnmallocator/drivers_darwin.go b/manager/allocator/cnmallocator/drivers_darwin.go index f183ac478c..5002492e92 100644 --- a/manager/allocator/cnmallocator/drivers_darwin.go +++ b/manager/allocator/cnmallocator/drivers_darwin.go @@ -2,13 +2,12 @@ package cnmallocator import ( "github.com/docker/docker/libnetwork/drivers/overlay/ovmanager" - "github.com/docker/docker/libnetwork/drivers/remote" "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) var initializers = map[string]driverRegisterFn{ - "remote": remote.Init, - "overlay": ovmanager.Init, + "remote": registerRemote, + "overlay": ovmanager.Register, } // PredefinedNetworks returns the list of predefined network structures diff --git a/manager/allocator/cnmallocator/drivers_network_linux.go b/manager/allocator/cnmallocator/drivers_network_linux.go index 1f67e1e2ce..655cff846b 100644 --- a/manager/allocator/cnmallocator/drivers_network_linux.go +++ b/manager/allocator/cnmallocator/drivers_network_linux.go @@ -6,18 +6,16 @@ import ( "github.com/docker/docker/libnetwork/drivers/ipvlan/ivmanager" "github.com/docker/docker/libnetwork/drivers/macvlan/mvmanager" "github.com/docker/docker/libnetwork/drivers/overlay/ovmanager" - "github.com/docker/docker/libnetwork/drivers/remote" - "github.com/docker/docker/libnetwork/drvregistry" "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) -var initializers = map[string]drvregistry.InitFunc{ - "remote": remote.Init, - "overlay": ovmanager.Init, - "macvlan": mvmanager.Init, - "bridge": brmanager.Init, - "ipvlan": ivmanager.Init, - "host": host.Init, +var initializers = map[string]driverRegisterFn{ + "remote": registerRemote, + "overlay": ovmanager.Register, + "macvlan": mvmanager.Register, + "bridge": brmanager.Register, + "ipvlan": ivmanager.Register, + "host": host.Register, } // PredefinedNetworks returns the list of predefined network structures diff --git a/manager/allocator/cnmallocator/drivers_network_windows.go b/manager/allocator/cnmallocator/drivers_network_windows.go index 126cb85ed8..de4abe21e7 100644 --- a/manager/allocator/cnmallocator/drivers_network_windows.go +++ b/manager/allocator/cnmallocator/drivers_network_windows.go @@ -2,17 +2,15 @@ package cnmallocator import ( "github.com/docker/docker/libnetwork/drivers/overlay/ovmanager" - "github.com/docker/docker/libnetwork/drivers/remote" - "github.com/docker/docker/libnetwork/drvregistry" "github.com/moby/swarmkit/v2/manager/allocator/networkallocator" ) -var initializers = map[string]drvregistry.InitFunc{ - "remote": remote.Init, - "overlay": ovmanager.Init, - "internal": StubManagerInit("internal"), - "l2bridge": StubManagerInit("l2bridge"), - "nat": StubManagerInit("nat"), +var initializers = map[string]driverRegisterFn{ + "remote": registerRemote, + "overlay": ovmanager.Register, + "internal": registerNetworkType("internal"), + "l2bridge": registerNetworkType("l2bridge"), + "nat": registerNetworkType("nat"), } // PredefinedNetworks returns the list of predefined network structures From cecae4af3399e6bbbacb78df301551480b297020 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 30 Jun 2023 13:21:59 +0200 Subject: [PATCH 4/4] manager/allocator/cnmallocator: StubManagerInit: config arg is unused Make it clearer that the config argument is not used. Signed-off-by: Sebastiaan van Stijn --- manager/allocator/cnmallocator/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manager/allocator/cnmallocator/manager.go b/manager/allocator/cnmallocator/manager.go index 0f86efd4ad..e285ac4e0f 100644 --- a/manager/allocator/cnmallocator/manager.go +++ b/manager/allocator/cnmallocator/manager.go @@ -11,8 +11,8 @@ type manager struct { networkType string } -func StubManagerInit(networkType string) func(dc driverapi.DriverCallback, config map[string]interface{}) error { - return func(dc driverapi.DriverCallback, config map[string]interface{}) error { +func StubManagerInit(networkType string) func(dc driverapi.DriverCallback, _ map[string]interface{}) error { + return func(dc driverapi.DriverCallback, _ map[string]interface{}) error { return RegisterManager(dc, networkType) } }