Merge pull request #26442 from vieux/add_warning_plugin_rm

add check plugin is not used before rm

integration-cli/docker_cli_plugins_test.go

@@ -58,6 +58,33 @@ func (s *DockerSuite) TestPluginForceRemove(c *check.C) {
 	c.Assert(out, checker.Contains, pNameWithTag)
 }
 
+func (s *DockerSuite) TestPluginActive(c *check.C) {
+	testRequires(c, DaemonIsLinux, ExperimentalDaemon)
+	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", pNameWithTag)
+	c.Assert(err, checker.IsNil)
+
+	out, _, err = dockerCmdWithError("volume", "create", "-d", pNameWithTag)
+	c.Assert(err, checker.IsNil)
+
+	vID := strings.TrimSpace(out)
+
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
+	c.Assert(out, checker.Contains, "is in use")
+
+	_, _, err = dockerCmdWithError("volume", "rm", vID)
+	c.Assert(err, checker.IsNil)
+
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
+	c.Assert(out, checker.Contains, "is enabled")
+
+	_, _, err = dockerCmdWithError("plugin", "disable", pNameWithTag)
+	c.Assert(err, checker.IsNil)
+
+	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, pNameWithTag)
+}
+
 func (s *DockerSuite) TestPluginInstallDisable(c *check.C) {
 	testRequires(c, DaemonIsLinux, ExperimentalDaemon)
 	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", "--disable", pName)

plugin/backend.go

@@ -147,14 +147,26 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error {
 	if err != nil {
 		return err
 	}
-	if p.IsEnabled() {
+
 	if !config.ForceRemove {
+		p.RLock()
+		if p.RefCount > 0 {
+			p.RUnlock()
+			return fmt.Errorf("plugin %s is in use", p.Name())
+		}
+		p.RUnlock()
+
+		if p.IsEnabled() {
 			return fmt.Errorf("plugin %s is enabled", p.Name())
 		}
+	}
+
+	if p.IsEnabled() {
 		if err := pm.disable(p); err != nil {
 			logrus.Errorf("failed to disable plugin '%s': %s", p.Name(), err)
 		}
 	}
+
 	pm.pluginStore.Remove(p)
 	pm.pluginEventLogger(p.GetID(), name, "remove")
 	return nil

plugin/store/interface.go

@@ -2,6 +2,15 @@ package store
 
 import "github.com/docker/docker/pkg/plugins"
 
+const (
+	// LOOKUP doesn't update RefCount
+	LOOKUP = 0
+	// CREATE increments RefCount
+	CREATE = 1
+	// REMOVE decrements RefCount
+	REMOVE = -1
+)
+
 // CompatPlugin is an abstraction to handle both new and legacy (v1) plugins.
 type CompatPlugin interface {
 	Client() *plugins.Client

plugin/store/legacy.go

@@ -20,6 +20,6 @@ func FindWithCapability(capability string) ([]CompatPlugin, error) {
 }
 
 // LookupWithCapability returns a plugin matching the given name and capability.
-func LookupWithCapability(name, capability string) (CompatPlugin, error) {
+func LookupWithCapability(name, capability string, _ int) (CompatPlugin, error) {
 	return plugins.Get(name, capability)
 }

plugin/store/store.go

@@ -160,7 +160,7 @@ func (ps *PluginStore) updatePluginDB() error {
 }
 
 // LookupWithCapability returns a plugin matching the given name and capability.
-func LookupWithCapability(name, capability string) (CompatPlugin, error) {
+func LookupWithCapability(name, capability string, mode int) (CompatPlugin, error) {
 	var (
 		p   *v2.Plugin
 		err error
@@ -181,6 +181,9 @@ func LookupWithCapability(name, capability string) (CompatPlugin, error) {
 		}
 		p, err = store.GetByName(fullName)
 		if err == nil {
+			p.Lock()
+			p.RefCount += mode
+			p.Unlock()
 			return p.FilterByCap(capability)
 		}
 		if _, ok := err.(ErrNotFound); !ok {

plugin/v2/plugin.go

@@ -35,6 +35,7 @@ type Plugin struct {
 	RestartManager    restartmanager.RestartManager `json:"-"`
 	RuntimeSourcePath string                        `json:"-"`
 	ExitChan          chan bool                     `json:"-"`
+	RefCount          int                           `json:"-"`
 }
 
 func newPluginObj(name, id, tag string) types.Plugin {

volume/drivers/extpoint.go

@@ -91,7 +91,7 @@ func Unregister(name string) bool {
 // lookup returns the driver associated with the given name. If a
 // driver with the given name has not been registered it checks if
 // there is a VolumeDriver plugin available with the given name.
-func lookup(name string) (volume.Driver, error) {
+func lookup(name string, mode int) (volume.Driver, error) {
 	drivers.driverLock.Lock(name)
 	defer drivers.driverLock.Unlock(name)
 
@@ -102,7 +102,7 @@ func lookup(name string) (volume.Driver, error) {
 		return ext, nil
 	}
 
-	p, err := pluginStore.LookupWithCapability(name, extName)
+	p, err := pluginStore.LookupWithCapability(name, extName, mode)
 	if err != nil {
 		return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err)
 	}
@@ -134,7 +134,25 @@ func GetDriver(name string) (volume.Driver, error) {
 	if name == "" {
 		name = volume.DefaultDriverName
 	}
-	return lookup(name)
+	return lookup(name, pluginStore.LOOKUP)
+}
+
+// CreateDriver returns a volume driver by its name and increments RefCount.
+// If the driver is empty, it looks for the local driver.
+func CreateDriver(name string) (volume.Driver, error) {
+	if name == "" {
+		name = volume.DefaultDriverName
+	}
+	return lookup(name, pluginStore.CREATE)
+}
+
+// RemoveDriver returns a volume driver by its name and decrements RefCount..
+// If the driver is empty, it looks for the local driver.
+func RemoveDriver(name string) (volume.Driver, error) {
+	if name == "" {
+		name = volume.DefaultDriverName
+	}
+	return lookup(name, pluginStore.REMOVE)
 }
 
 // GetDriverList returns list of volume drivers registered.

volume/store/store.go

@@ -264,7 +264,7 @@ func (s *VolumeStore) create(name, driverName string, opts, labels map[string]st
 		}
 	}
 
-	vd, err := volumedrivers.GetDriver(driverName)
+	vd, err := volumedrivers.CreateDriver(driverName)
 
 	if err != nil {
 		return nil, &OpErr{Op: "create", Name: name, Err: err}
@@ -416,7 +416,7 @@ func (s *VolumeStore) Remove(v volume.Volume) error {
 		return &OpErr{Err: errVolumeInUse, Name: v.Name(), Op: "remove", Refs: refs}
 	}
 
-	vd, err := volumedrivers.GetDriver(v.DriverName())
+	vd, err := volumedrivers.RemoveDriver(v.DriverName())
 	if err != nil {
 		return &OpErr{Err: err, Name: vd.Name(), Op: "remove"}
 	}