Browse code

Merge pull request #29556 from mavenugo/refcount

Fixing a couple of network plugin life-cycle mgmt issues

Victor Vieux authored on 2017/01/04 04:13:22
Showing 24 changed files
... ...
@@ -12,8 +12,11 @@ import (
12 12
 	"github.com/docker/docker/api/types"
13 13
 	"github.com/docker/docker/api/types/network"
14 14
 	clustertypes "github.com/docker/docker/daemon/cluster/provider"
15
+	"github.com/docker/docker/pkg/plugingetter"
15 16
 	"github.com/docker/docker/runconfig"
16 17
 	"github.com/docker/libnetwork"
18
+	"github.com/docker/libnetwork/driverapi"
19
+	"github.com/docker/libnetwork/ipamapi"
17 20
 	networktypes "github.com/docker/libnetwork/types"
18 21
 	"github.com/pkg/errors"
19 22
 	"golang.org/x/net/context"
... ...
@@ -298,6 +301,10 @@ func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string
298 298
 		return nil, err
299 299
 	}
300 300
 
301
+	daemon.pluginRefCount(driver, driverapi.NetworkPluginEndpointType, plugingetter.ACQUIRE)
302
+	if create.IPAM != nil {
303
+		daemon.pluginRefCount(create.IPAM.Driver, ipamapi.PluginEndpointType, plugingetter.ACQUIRE)
304
+	}
301 305
 	daemon.LogNetworkEvent(n, "create")
302 306
 
303 307
 	return &types.NetworkCreateResponse{
... ...
@@ -306,6 +313,29 @@ func (daemon *Daemon) createNetwork(create types.NetworkCreateRequest, id string
306 306
 	}, nil
307 307
 }
308 308
 
309
+func (daemon *Daemon) pluginRefCount(driver, capability string, mode int) {
310
+	var builtinDrivers []string
311
+
312
+	if capability == driverapi.NetworkPluginEndpointType {
313
+		builtinDrivers = daemon.netController.BuiltinDrivers()
314
+	} else if capability == ipamapi.PluginEndpointType {
315
+		builtinDrivers = daemon.netController.BuiltinIPAMDrivers()
316
+	}
317
+
318
+	for _, d := range builtinDrivers {
319
+		if d == driver {
320
+			return
321
+		}
322
+	}
323
+
324
+	if daemon.PluginStore != nil {
325
+		_, err := daemon.PluginStore.Get(driver, capability, mode)
326
+		if err != nil {
327
+			logrus.WithError(err).WithFields(logrus.Fields{"mode": mode, "driver": driver}).Error("Error handling plugin refcount operation")
328
+		}
329
+	}
330
+}
331
+
309 332
 func getIpamConfig(data []network.IPAMConfig) ([]*libnetwork.IpamConf, []*libnetwork.IpamConf, error) {
310 333
 	ipamV4Cfg := []*libnetwork.IpamConf{}
311 334
 	ipamV6Cfg := []*libnetwork.IpamConf{}
... ...
@@ -420,6 +450,9 @@ func (daemon *Daemon) deleteNetwork(networkID string, dynamic bool) error {
420 420
 	if err := nw.Delete(); err != nil {
421 421
 		return err
422 422
 	}
423
+	daemon.pluginRefCount(nw.Type(), driverapi.NetworkPluginEndpointType, plugingetter.RELEASE)
424
+	ipamType, _, _, _ := nw.Info().IpamConfig()
425
+	daemon.pluginRefCount(ipamType, ipamapi.PluginEndpointType, plugingetter.RELEASE)
423 426
 	daemon.LogNetworkEvent(nw, "destroy")
424 427
 	return nil
425 428
 }
... ...
@@ -16,8 +16,10 @@ import (
16 16
 var (
17 17
 	pluginProcessName = "sample-volume-plugin"
18 18
 	pName             = "tonistiigi/sample-volume-plugin"
19
+	npName            = "tonistiigi/test-docker-netplugin"
19 20
 	pTag              = "latest"
20 21
 	pNameWithTag      = pName + ":" + pTag
22
+	npNameWithTag     = npName + ":" + pTag
21 23
 )
22 24
 
23 25
 func (s *DockerSuite) TestPluginBasicOps(c *check.C) {
... ...
@@ -87,6 +89,33 @@ func (s *DockerSuite) TestPluginActive(c *check.C) {
87 87
 	c.Assert(out, checker.Contains, pNameWithTag)
88 88
 }
89 89
 
90
+func (s *DockerSuite) TestPluginActiveNetwork(c *check.C) {
91
+	testRequires(c, DaemonIsLinux, IsAmd64, Network)
92
+	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", npNameWithTag)
93
+	c.Assert(err, checker.IsNil)
94
+
95
+	out, _, err = dockerCmdWithError("network", "create", "-d", npNameWithTag, "test")
96
+	c.Assert(err, checker.IsNil)
97
+
98
+	nID := strings.TrimSpace(out)
99
+
100
+	out, _, err = dockerCmdWithError("plugin", "remove", npNameWithTag)
101
+	c.Assert(out, checker.Contains, "is in use")
102
+
103
+	_, _, err = dockerCmdWithError("network", "rm", nID)
104
+	c.Assert(err, checker.IsNil)
105
+
106
+	out, _, err = dockerCmdWithError("plugin", "remove", npNameWithTag)
107
+	c.Assert(out, checker.Contains, "is enabled")
108
+
109
+	_, _, err = dockerCmdWithError("plugin", "disable", npNameWithTag)
110
+	c.Assert(err, checker.IsNil)
111
+
112
+	out, _, err = dockerCmdWithError("plugin", "remove", npNameWithTag)
113
+	c.Assert(err, checker.IsNil)
114
+	c.Assert(out, checker.Contains, npNameWithTag)
115
+}
116
+
90 117
 func (s *DockerSuite) TestPluginInstallDisable(c *check.C) {
91 118
 	testRequires(c, DaemonIsLinux, IsAmd64, Network)
92 119
 	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", "--disable", pName)
... ...
@@ -499,13 +499,6 @@ func (s *DockerSwarmSuite) TestPsListContainersFilterIsTask(c *check.C) {
499 499
 const globalNetworkPlugin = "global-network-plugin"
500 500
 const globalIPAMPlugin = "global-ipam-plugin"
501 501
 
502
-func (s *DockerSwarmSuite) SetUpSuite(c *check.C) {
503
-	mux := http.NewServeMux()
504
-	s.server = httptest.NewServer(mux)
505
-	c.Assert(s.server, check.NotNil, check.Commentf("Failed to start an HTTP Server"))
506
-	setupRemoteGlobalNetworkPlugin(c, mux, s.server.URL, globalNetworkPlugin, globalIPAMPlugin)
507
-}
508
-
509 502
 func setupRemoteGlobalNetworkPlugin(c *check.C, mux *http.ServeMux, url, netDrv, ipamDrv string) {
510 503
 
511 504
 	mux.HandleFunc("/Plugin.Activate", func(w http.ResponseWriter, r *http.Request) {
... ...
@@ -675,6 +668,16 @@ func setupRemoteGlobalNetworkPlugin(c *check.C, mux *http.ServeMux, url, netDrv,
675 675
 }
676 676
 
677 677
 func (s *DockerSwarmSuite) TestSwarmNetworkPlugin(c *check.C) {
678
+	mux := http.NewServeMux()
679
+	s.server = httptest.NewServer(mux)
680
+	c.Assert(s.server, check.NotNil, check.Commentf("Failed to start an HTTP Server"))
681
+	setupRemoteGlobalNetworkPlugin(c, mux, s.server.URL, globalNetworkPlugin, globalIPAMPlugin)
682
+	defer func() {
683
+		s.server.Close()
684
+		err := os.RemoveAll("/etc/docker/plugins")
685
+		c.Assert(err, checker.IsNil)
686
+	}()
687
+
678 688
 	d := s.AddDaemon(c, true, true)
679 689
 
680 690
 	out, err := d.Cmd("network", "create", "-d", globalNetworkPlugin, "foo")
... ...
@@ -23,7 +23,7 @@ github.com/RackSec/srslog 456df3a81436d29ba874f3590eeeee25d666f8a5
23 23
 github.com/imdario/mergo 0.2.1
24 24
 
25 25
 #get libnetwork packages
26
-github.com/docker/libnetwork b908488a139e81cb8c4091cd836745aeb4d813a4
26
+github.com/docker/libnetwork 61f01cdbbda7e32e651198b04889f6d825064fa6
27 27
 github.com/docker/go-events 18b43f1bc85d9cdd42c05a6cd2d444c7a200a894
28 28
 github.com/armon/go-radix e39d623f12e8e41c7b5529e9a9dd67a1e2261f80
29 29
 github.com/armon/go-metrics eb0af217e5e9747e41dd5303755356b62d28e3ec
... ...
@@ -79,6 +79,9 @@ type NetworkController interface {
79 79
 	// BuiltinDrivers returns list of builtin drivers
80 80
 	BuiltinDrivers() []string
81 81
 
82
+	// BuiltinIPAMDrivers returns list of builtin ipam drivers
83
+	BuiltinIPAMDrivers() []string
84
+
82 85
 	// Config method returns the bootup configuration for the controller
83 86
 	Config() config.Config
84 87
 
... ...
@@ -476,12 +479,23 @@ func (c *controller) ID() string {
476 476
 
477 477
 func (c *controller) BuiltinDrivers() []string {
478 478
 	drivers := []string{}
479
-	for _, i := range getInitializers(c.cfg.Daemon.Experimental) {
480
-		if i.ntype == "remote" {
481
-			continue
479
+	c.drvRegistry.WalkDrivers(func(name string, driver driverapi.Driver, capability driverapi.Capability) bool {
480
+		if driver.IsBuiltIn() {
481
+			drivers = append(drivers, name)
482 482
 		}
483
-		drivers = append(drivers, i.ntype)
484
-	}
483
+		return false
484
+	})
485
+	return drivers
486
+}
487
+
488
+func (c *controller) BuiltinIPAMDrivers() []string {
489
+	drivers := []string{}
490
+	c.drvRegistry.WalkIPAMs(func(name string, driver ipamapi.Ipam, cap *ipamapi.Capability) bool {
491
+		if driver.IsBuiltIn() {
492
+			drivers = append(drivers, name)
493
+		}
494
+		return false
495
+	})
485 496
 	return drivers
486 497
 }
487 498
 
... ...
@@ -74,6 +74,9 @@ type Driver interface {
74 74
 
75 75
 	// Type returns the the type of this driver, the network type this driver manages
76 76
 	Type() string
77
+
78
+	// IsBuiltIn returns true if it is a built-in driver
79
+	IsBuiltIn() bool
77 80
 }
78 81
 
79 82
 // NetworkInfo provides a go interface for drivers to provide network
... ...
@@ -1424,6 +1424,10 @@ func (d *driver) Type() string {
1424 1424
 	return networkType
1425 1425
 }
1426 1426
 
1427
+func (d *driver) IsBuiltIn() bool {
1428
+	return true
1429
+}
1430
+
1427 1431
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
1428 1432
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
1429 1433
 	return nil
... ...
@@ -86,6 +86,10 @@ func (d *driver) Type() string {
86 86
 	return networkType
87 87
 }
88 88
 
89
+func (d *driver) IsBuiltIn() bool {
90
+	return true
91
+}
92
+
89 93
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
90 94
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
91 95
 	return nil
... ...
@@ -84,6 +84,10 @@ func (d *driver) Type() string {
84 84
 	return ipvlanType
85 85
 }
86 86
 
87
+func (d *driver) IsBuiltIn() bool {
88
+	return true
89
+}
90
+
87 91
 func (d *driver) ProgramExternalConnectivity(nid, eid string, options map[string]interface{}) error {
88 92
 	return nil
89 93
 }
... ...
@@ -86,6 +86,10 @@ func (d *driver) Type() string {
86 86
 	return macvlanType
87 87
 }
88 88
 
89
+func (d *driver) IsBuiltIn() bool {
90
+	return true
91
+}
92
+
89 93
 func (d *driver) ProgramExternalConnectivity(nid, eid string, options map[string]interface{}) error {
90 94
 	return nil
91 95
 }
... ...
@@ -86,6 +86,10 @@ func (d *driver) Type() string {
86 86
 	return networkType
87 87
 }
88 88
 
89
+func (d *driver) IsBuiltIn() bool {
90
+	return true
91
+}
92
+
89 93
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
90 94
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
91 95
 	return nil
... ...
@@ -211,6 +211,10 @@ func (d *driver) Type() string {
211 211
 	return networkType
212 212
 }
213 213
 
214
+func (d *driver) IsBuiltIn() bool {
215
+	return true
216
+}
217
+
214 218
 func validateSelf(node string) error {
215 219
 	advIP := net.ParseIP(node)
216 220
 	if advIP == nil {
... ...
@@ -229,6 +229,10 @@ func (d *driver) Type() string {
229 229
 	return networkType
230 230
 }
231 231
 
232
+func (d *driver) IsBuiltIn() bool {
233
+	return true
234
+}
235
+
232 236
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
233 237
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
234 238
 	return types.NotImplementedErrorf("not implemented")
... ...
@@ -29,12 +29,7 @@ func newDriver(name string, client *plugins.Client) driverapi.Driver {
29 29
 // Init makes sure a remote driver is registered when a network driver
30 30
 // plugin is activated.
31 31
 func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
32
-	// Unit test code is unaware of a true PluginStore. So we fall back to v1 plugins.
33
-	handleFunc := plugins.Handle
34
-	if pg := dc.GetPluginGetter(); pg != nil {
35
-		handleFunc = pg.Handle
36
-	}
37
-	handleFunc(driverapi.NetworkPluginEndpointType, func(name string, client *plugins.Client) {
32
+	newPluginHandler := func(name string, client *plugins.Client) {
38 33
 		// negotiate driver capability with client
39 34
 		d := newDriver(name, client)
40 35
 		c, err := d.(*driver).getCapabilities()
... ...
@@ -45,7 +40,19 @@ func Init(dc driverapi.DriverCallback, config map[string]interface{}) error {
45 45
 		if err = dc.RegisterDriver(name, d, *c); err != nil {
46 46
 			logrus.Errorf("error registering driver for %s due to %v", name, err)
47 47
 		}
48
-	})
48
+	}
49
+
50
+	// Unit test code is unaware of a true PluginStore. So we fall back to v1 plugins.
51
+	handleFunc := plugins.Handle
52
+	if pg := dc.GetPluginGetter(); pg != nil {
53
+		handleFunc = pg.Handle
54
+		activePlugins := pg.GetAllManagedPluginsByCap(driverapi.NetworkPluginEndpointType)
55
+		for _, ap := range activePlugins {
56
+			newPluginHandler(ap.Name(), ap.Client())
57
+		}
58
+	}
59
+	handleFunc(driverapi.NetworkPluginEndpointType, newPluginHandler)
60
+
49 61
 	return nil
50 62
 }
51 63
 
... ...
@@ -305,6 +312,10 @@ func (d *driver) Type() string {
305 305
 	return d.networkType
306 306
 }
307 307
 
308
+func (d *driver) IsBuiltIn() bool {
309
+	return false
310
+}
311
+
308 312
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
309 313
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
310 314
 	if dType != discoverapi.NodeDiscovery {
... ...
@@ -916,6 +916,10 @@ func (d *driver) Type() string {
916 916
 	return networkType
917 917
 }
918 918
 
919
+func (d *driver) IsBuiltIn() bool {
920
+	return true
921
+}
922
+
919 923
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
920 924
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
921 925
 	return nil
... ...
@@ -200,6 +200,10 @@ func (d *driver) Type() string {
200 200
 	return networkType
201 201
 }
202 202
 
203
+func (d *driver) IsBuiltIn() bool {
204
+	return true
205
+}
206
+
203 207
 func validateSelf(node string) error {
204 208
 	advIP := net.ParseIP(node)
205 209
 	if advIP == nil {
... ...
@@ -176,6 +176,10 @@ func (d *driver) Type() string {
176 176
 	return networkType
177 177
 }
178 178
 
179
+func (d *driver) IsBuiltIn() bool {
180
+	return true
181
+}
182
+
179 183
 func validateSelf(node string) error {
180 184
 	advIP := net.ParseIP(node)
181 185
 	if advIP == nil {
... ...
@@ -698,6 +698,10 @@ func (d *driver) Type() string {
698 698
 	return d.name
699 699
 }
700 700
 
701
+func (d *driver) IsBuiltIn() bool {
702
+	return true
703
+}
704
+
701 705
 // DiscoverNew is a notification for a new discovery event, such as a new node joining a cluster
702 706
 func (d *driver) DiscoverNew(dType discoverapi.DiscoveryType, data interface{}) error {
703 707
 	return nil
... ...
@@ -164,10 +164,10 @@ func (r *DrvRegistry) RegisterDriver(ntype string, driver driverapi.Driver, capa
164 164
 	}
165 165
 
166 166
 	r.Lock()
167
-	_, ok := r.drivers[ntype]
167
+	dd, ok := r.drivers[ntype]
168 168
 	r.Unlock()
169 169
 
170
-	if ok {
170
+	if ok && dd.driver.IsBuiltIn() {
171 171
 		return driverapi.ErrActiveRegistration(ntype)
172 172
 	}
173 173
 
... ...
@@ -192,9 +192,9 @@ func (r *DrvRegistry) registerIpamDriver(name string, driver ipamapi.Ipam, caps
192 192
 	}
193 193
 
194 194
 	r.Lock()
195
-	_, ok := r.ipamDrivers[name]
195
+	dd, ok := r.ipamDrivers[name]
196 196
 	r.Unlock()
197
-	if ok {
197
+	if ok && dd.driver.IsBuiltIn() {
198 198
 		return types.ForbiddenErrorf("ipam driver %q already registered", name)
199 199
 	}
200 200
 
... ...
@@ -594,3 +594,8 @@ func (a *Allocator) DumpDatabase() string {
594 594
 
595 595
 	return s
596 596
 }
597
+
598
+// IsBuiltIn returns true for builtin drivers
599
+func (a *Allocator) IsBuiltIn() bool {
600
+	return true
601
+}
... ...
@@ -80,6 +80,9 @@ type Ipam interface {
80 80
 	RequestAddress(string, net.IP, map[string]string) (*net.IPNet, map[string]string, error)
81 81
 	// Release the address from the specified pool ID
82 82
 	ReleaseAddress(string, net.IP) error
83
+
84
+	//IsBuiltIn returns true if it is a built-in driver.
85
+	IsBuiltIn() bool
83 86
 }
84 87
 
85 88
 // Capability represents the requirements and capabilities of the IPAM driver
... ...
@@ -65,6 +65,10 @@ func (a *allocator) DiscoverDelete(dType discoverapi.DiscoveryType, data interfa
65 65
 	return nil
66 66
 }
67 67
 
68
+func (a *allocator) IsBuiltIn() bool {
69
+	return true
70
+}
71
+
68 72
 // Init registers a remote ipam when its plugin is activated
69 73
 func Init(ic ipamapi.Callback, l, g interface{}) error {
70 74
 	return ic.RegisterIpamDriver(ipamapi.NullIPAM, &allocator{})
... ...
@@ -31,12 +31,7 @@ func newAllocator(name string, client *plugins.Client) ipamapi.Ipam {
31 31
 // Init registers a remote ipam when its plugin is activated
32 32
 func Init(cb ipamapi.Callback, l, g interface{}) error {
33 33
 
34
-	// Unit test code is unaware of a true PluginStore. So we fall back to v1 plugins.
35
-	handleFunc := plugins.Handle
36
-	if pg := cb.GetPluginGetter(); pg != nil {
37
-		handleFunc = pg.Handle
38
-	}
39
-	handleFunc(ipamapi.PluginEndpointType, func(name string, client *plugins.Client) {
34
+	newPluginHandler := func(name string, client *plugins.Client) {
40 35
 		a := newAllocator(name, client)
41 36
 		if cps, err := a.(*allocator).getCapabilities(); err == nil {
42 37
 			if err := cb.RegisterIpamDriverWithCapabilities(name, a, cps); err != nil {
... ...
@@ -49,7 +44,18 @@ func Init(cb ipamapi.Callback, l, g interface{}) error {
49 49
 				logrus.Errorf("error registering remote ipam driver %s due to %v", name, err)
50 50
 			}
51 51
 		}
52
-	})
52
+	}
53
+
54
+	// Unit test code is unaware of a true PluginStore. So we fall back to v1 plugins.
55
+	handleFunc := plugins.Handle
56
+	if pg := cb.GetPluginGetter(); pg != nil {
57
+		handleFunc = pg.Handle
58
+		activePlugins := pg.GetAllManagedPluginsByCap(ipamapi.PluginEndpointType)
59
+		for _, ap := range activePlugins {
60
+			newPluginHandler(ap.Name(), ap.Client())
61
+		}
62
+	}
63
+	handleFunc(ipamapi.PluginEndpointType, newPluginHandler)
53 64
 	return nil
54 65
 }
55 66
 
... ...
@@ -143,3 +149,7 @@ func (a *allocator) DiscoverNew(dType discoverapi.DiscoveryType, data interface{
143 143
 func (a *allocator) DiscoverDelete(dType discoverapi.DiscoveryType, data interface{}) error {
144 144
 	return nil
145 145
 }
146
+
147
+func (a *allocator) IsBuiltIn() bool {
148
+	return false
149
+}
... ...
@@ -101,3 +101,7 @@ func (a *allocator) DiscoverNew(dType discoverapi.DiscoveryType, data interface{
101 101
 func (a *allocator) DiscoverDelete(dType discoverapi.DiscoveryType, data interface{}) error {
102 102
 	return nil
103 103
 }
104
+
105
+func (a *allocator) IsBuiltIn() bool {
106
+	return true
107
+}