Browse code

add check plugin is not used before rm

Signed-off-by: Victor Vieux <vieux@docker.com>

Victor Vieux authored on 2016/09/07 22:59:15
Showing 8 changed files
... ...
@@ -58,6 +58,33 @@ func (s *DockerSuite) TestPluginForceRemove(c *check.C) {
58 58
 	c.Assert(out, checker.Contains, pNameWithTag)
59 59
 }
60 60
 
61
+func (s *DockerSuite) TestPluginActive(c *check.C) {
62
+	testRequires(c, DaemonIsLinux, ExperimentalDaemon)
63
+	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", pNameWithTag)
64
+	c.Assert(err, checker.IsNil)
65
+
66
+	out, _, err = dockerCmdWithError("volume", "create", "-d", pNameWithTag)
67
+	c.Assert(err, checker.IsNil)
68
+
69
+	vID := strings.TrimSpace(out)
70
+
71
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
72
+	c.Assert(out, checker.Contains, "is in use")
73
+
74
+	_, _, err = dockerCmdWithError("volume", "rm", vID)
75
+	c.Assert(err, checker.IsNil)
76
+
77
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
78
+	c.Assert(out, checker.Contains, "is enabled")
79
+
80
+	_, _, err = dockerCmdWithError("plugin", "disable", pNameWithTag)
81
+	c.Assert(err, checker.IsNil)
82
+
83
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
84
+	c.Assert(err, checker.IsNil)
85
+	c.Assert(out, checker.Contains, pNameWithTag)
86
+}
87
+
61 88
 func (s *DockerSuite) TestPluginInstallDisable(c *check.C) {
62 89
 	testRequires(c, DaemonIsLinux, ExperimentalDaemon)
63 90
 	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", "--disable", pName)
... ...
@@ -147,14 +147,26 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error {
147 147
 	if err != nil {
148 148
 		return err
149 149
 	}
150
-	if p.IsEnabled() {
151
-		if !config.ForceRemove {
150
+
151
+	if !config.ForceRemove {
152
+		p.RLock()
153
+		if p.RefCount > 0 {
154
+			p.RUnlock()
155
+			return fmt.Errorf("plugin %s is in use", p.Name())
156
+		}
157
+		p.RUnlock()
158
+
159
+		if p.IsEnabled() {
152 160
 			return fmt.Errorf("plugin %s is enabled", p.Name())
153 161
 		}
162
+	}
163
+
164
+	if p.IsEnabled() {
154 165
 		if err := pm.disable(p); err != nil {
155 166
 			logrus.Errorf("failed to disable plugin '%s': %s", p.Name(), err)
156 167
 		}
157 168
 	}
169
+
158 170
 	pm.pluginStore.Remove(p)
159 171
 	pm.pluginEventLogger(p.GetID(), name, "remove")
160 172
 	return nil
... ...
@@ -2,6 +2,15 @@ package store
2 2
 
3 3
 import "github.com/docker/docker/pkg/plugins"
4 4
 
5
+const (
6
+	// LOOKUP doesn't update RefCount
7
+	LOOKUP = 0
8
+	// CREATE increments RefCount
9
+	CREATE = 1
10
+	// REMOVE decrements RefCount
11
+	REMOVE = -1
12
+)
13
+
5 14
 // CompatPlugin is an abstraction to handle both new and legacy (v1) plugins.
6 15
 type CompatPlugin interface {
7 16
 	Client() *plugins.Client
... ...
@@ -20,6 +20,6 @@ func FindWithCapability(capability string) ([]CompatPlugin, error) {
20 20
 }
21 21
 
22 22
 // LookupWithCapability returns a plugin matching the given name and capability.
23
-func LookupWithCapability(name, capability string) (CompatPlugin, error) {
23
+func LookupWithCapability(name, capability string, _ int) (CompatPlugin, error) {
24 24
 	return plugins.Get(name, capability)
25 25
 }
... ...
@@ -160,7 +160,7 @@ func (ps *PluginStore) updatePluginDB() error {
160 160
 }
161 161
 
162 162
 // LookupWithCapability returns a plugin matching the given name and capability.
163
-func LookupWithCapability(name, capability string) (CompatPlugin, error) {
163
+func LookupWithCapability(name, capability string, mode int) (CompatPlugin, error) {
164 164
 	var (
165 165
 		p   *v2.Plugin
166 166
 		err error
... ...
@@ -181,6 +181,9 @@ func LookupWithCapability(name, capability string) (CompatPlugin, error) {
181 181
 		}
182 182
 		p, err = store.GetByName(fullName)
183 183
 		if err == nil {
184
+			p.Lock()
185
+			p.RefCount += mode
186
+			p.Unlock()
184 187
 			return p.FilterByCap(capability)
185 188
 		}
186 189
 		if _, ok := err.(ErrNotFound); !ok {
... ...
@@ -35,6 +35,7 @@ type Plugin struct {
35 35
 	RestartManager    restartmanager.RestartManager `json:"-"`
36 36
 	RuntimeSourcePath string                        `json:"-"`
37 37
 	ExitChan          chan bool                     `json:"-"`
38
+	RefCount          int                           `json:"-"`
38 39
 }
39 40
 
40 41
 func newPluginObj(name, id, tag string) types.Plugin {
... ...
@@ -91,7 +91,7 @@ func Unregister(name string) bool {
91 91
 // lookup returns the driver associated with the given name. If a
92 92
 // driver with the given name has not been registered it checks if
93 93
 // there is a VolumeDriver plugin available with the given name.
94
-func lookup(name string) (volume.Driver, error) {
94
+func lookup(name string, mode int) (volume.Driver, error) {
95 95
 	drivers.driverLock.Lock(name)
96 96
 	defer drivers.driverLock.Unlock(name)
97 97
 
... ...
@@ -102,7 +102,7 @@ func lookup(name string) (volume.Driver, error) {
102 102
 		return ext, nil
103 103
 	}
104 104
 
105
-	p, err := pluginStore.LookupWithCapability(name, extName)
105
+	p, err := pluginStore.LookupWithCapability(name, extName, mode)
106 106
 	if err != nil {
107 107
 		return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err)
108 108
 	}
... ...
@@ -134,7 +134,25 @@ func GetDriver(name string) (volume.Driver, error) {
134 134
 	if name == "" {
135 135
 		name = volume.DefaultDriverName
136 136
 	}
137
-	return lookup(name)
137
+	return lookup(name, pluginStore.LOOKUP)
138
+}
139
+
140
+// CreateDriver returns a volume driver by its name and increments RefCount.
141
+// If the driver is empty, it looks for the local driver.
142
+func CreateDriver(name string) (volume.Driver, error) {
143
+	if name == "" {
144
+		name = volume.DefaultDriverName
145
+	}
146
+	return lookup(name, pluginStore.CREATE)
147
+}
148
+
149
+// RemoveDriver returns a volume driver by its name and decrements RefCount..
150
+// If the driver is empty, it looks for the local driver.
151
+func RemoveDriver(name string) (volume.Driver, error) {
152
+	if name == "" {
153
+		name = volume.DefaultDriverName
154
+	}
155
+	return lookup(name, pluginStore.REMOVE)
138 156
 }
139 157
 
140 158
 // GetDriverList returns list of volume drivers registered.
... ...
@@ -264,7 +264,7 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st
264 264
 		}
265 265
 	}
266 266
 
267
-	vd, err := volumedrivers.GetDriver(driverName)
267
+	vd, err := volumedrivers.CreateDriver(driverName)
268 268
 
269 269
 	if err != nil {
270 270
 		return nil, &OpErr{Op: "create", Name: name, Err: err}
... ...
@@ -416,7 +416,7 @@ func (s *VolumeStore) Remove(v volume.Volume) error {
416 416
 		return &OpErr{Err: errVolumeInUse, Name: v.Name(), Op: "remove", Refs: refs}
417 417
 	}
418 418
 
419
-	vd, err := volumedrivers.GetDriver(v.DriverName())
419
+	vd, err := volumedrivers.RemoveDriver(v.DriverName())
420 420
 	if err != nil {
421 421
 		return &OpErr{Err: err, Name: vd.Name(), Op: "remove"}
422 422
 	}