Browse code

Enforce zero plugin refcount during disable.

When plugins have a positive refcount, they were not allowed to be
removed. However, plugins could still be disabled when volumes
referenced it and containers using them were running.

This change fixes that by enforcing plugin refcount during disable.
A "force" disable option is also added to ignore reference refcounting.

Signed-off-by: Anusha Ragunathan <anusha@docker.com>

Anusha Ragunathan authored on 2016/12/21 01:26:58
Showing 13 changed files
... ...
@@ -10,7 +10,7 @@ import (
10 10
 
11 11
 // Backend for Plugin
12 12
 type Backend interface {
13
-	Disable(name string) error
13
+	Disable(name string, config *enginetypes.PluginDisableConfig) error
14 14
 	Enable(name string, config *enginetypes.PluginEnableConfig) error
15 15
 	List() ([]enginetypes.Plugin, error)
16 16
 	Inspect(name string) (enginetypes.Plugin, error)
... ...
@@ -99,7 +99,16 @@ func (pr *pluginRouter) enablePlugin(ctx context.Context, w http.ResponseWriter,
99 99
 }
100 100
 
101 101
 func (pr *pluginRouter) disablePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
102
-	return pr.backend.Disable(vars["name"])
102
+	if err := httputils.ParseForm(r); err != nil {
103
+		return err
104
+	}
105
+
106
+	name := vars["name"]
107
+	config := &types.PluginDisableConfig{
108
+		ForceDisable: httputils.BoolValue(r, "force"),
109
+	}
110
+
111
+	return pr.backend.Disable(name, config)
103 112
 }
104 113
 
105 114
 func (pr *pluginRouter) removePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
... ...
@@ -340,6 +340,11 @@ type PluginEnableOptions struct {
340 340
 	Timeout int
341 341
 }
342 342
 
343
+// PluginDisableOptions holds parameters to disable plugins.
344
+type PluginDisableOptions struct {
345
+	Force bool
346
+}
347
+
343 348
 // PluginInstallOptions holds parameters to install a plugin.
344 349
 type PluginInstallOptions struct {
345 350
 	Disabled              bool
... ...
@@ -53,14 +53,17 @@ type ExecConfig struct {
53 53
 	Cmd          []string // Execution commands and args
54 54
 }
55 55
 
56
-// PluginRmConfig holds arguments for the plugin remove
57
-// operation. This struct is used to tell the backend what operations
58
-// to perform.
56
+// PluginRmConfig holds arguments for plugin remove.
59 57
 type PluginRmConfig struct {
60 58
 	ForceRemove bool
61 59
 }
62 60
 
63
-// PluginEnableConfig holds arguments for the plugin enable
61
+// PluginEnableConfig holds arguments for plugin enable
64 62
 type PluginEnableConfig struct {
65 63
 	Timeout int
66 64
 }
65
+
66
+// PluginDisableConfig holds arguments for plugin disable.
67
+type PluginDisableConfig struct {
68
+	ForceDisable bool
69
+}
... ...
@@ -3,6 +3,7 @@ package plugin
3 3
 import (
4 4
 	"fmt"
5 5
 
6
+	"github.com/docker/docker/api/types"
6 7
 	"github.com/docker/docker/cli"
7 8
 	"github.com/docker/docker/cli/command"
8 9
 	"github.com/docker/docker/reference"
... ...
@@ -11,19 +12,23 @@ import (
11 11
 )
12 12
 
13 13
 func newDisableCommand(dockerCli *command.DockerCli) *cobra.Command {
14
+	var force bool
15
+
14 16
 	cmd := &cobra.Command{
15 17
 		Use:   "disable PLUGIN",
16 18
 		Short: "Disable a plugin",
17 19
 		Args:  cli.ExactArgs(1),
18 20
 		RunE: func(cmd *cobra.Command, args []string) error {
19
-			return runDisable(dockerCli, args[0])
21
+			return runDisable(dockerCli, args[0], force)
20 22
 		},
21 23
 	}
22 24
 
25
+	flags := cmd.Flags()
26
+	flags.BoolVarP(&force, "force", "f", false, "Force the disable of an active plugin")
23 27
 	return cmd
24 28
 }
25 29
 
26
-func runDisable(dockerCli *command.DockerCli, name string) error {
30
+func runDisable(dockerCli *command.DockerCli, name string, force bool) error {
27 31
 	named, err := reference.ParseNamed(name) // FIXME: validate
28 32
 	if err != nil {
29 33
 		return err
... ...
@@ -35,7 +40,7 @@ func runDisable(dockerCli *command.DockerCli, name string) error {
35 35
 	if !ok {
36 36
 		return fmt.Errorf("invalid name: %s", named.String())
37 37
 	}
38
-	if err := dockerCli.Client().PluginDisable(context.Background(), ref.String()); err != nil {
38
+	if err := dockerCli.Client().PluginDisable(context.Background(), ref.String(), types.PluginDisableOptions{Force: force}); err != nil {
39 39
 		return err
40 40
 	}
41 41
 	fmt.Fprintln(dockerCli.Out(), name)
... ...
@@ -110,7 +110,7 @@ type PluginAPIClient interface {
110 110
 	PluginList(ctx context.Context) (types.PluginsListResponse, error)
111 111
 	PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error
112 112
 	PluginEnable(ctx context.Context, name string, options types.PluginEnableOptions) error
113
-	PluginDisable(ctx context.Context, name string) error
113
+	PluginDisable(ctx context.Context, name string, options types.PluginDisableOptions) error
114 114
 	PluginInstall(ctx context.Context, name string, options types.PluginInstallOptions) error
115 115
 	PluginPush(ctx context.Context, name string, registryAuth string) error
116 116
 	PluginSet(ctx context.Context, name string, args []string) error
... ...
@@ -1,12 +1,19 @@
1 1
 package client
2 2
 
3 3
 import (
4
+	"net/url"
5
+
6
+	"github.com/docker/docker/api/types"
4 7
 	"golang.org/x/net/context"
5 8
 )
6 9
 
7 10
 // PluginDisable disables a plugin
8
-func (cli *Client) PluginDisable(ctx context.Context, name string) error {
9
-	resp, err := cli.post(ctx, "/plugins/"+name+"/disable", nil, nil, nil)
11
+func (cli *Client) PluginDisable(ctx context.Context, name string, options types.PluginDisableOptions) error {
12
+	query := url.Values{}
13
+	if options.Force {
14
+		query.Set("force", "1")
15
+	}
16
+	resp, err := cli.post(ctx, "/plugins/"+name+"/disable", query, nil, nil)
10 17
 	ensureReaderClosed(resp)
11 18
 	return err
12 19
 }
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"strings"
9 9
 	"testing"
10 10
 
11
+	"github.com/docker/docker/api/types"
11 12
 	"golang.org/x/net/context"
12 13
 )
13 14
 
... ...
@@ -16,7 +17,7 @@ func TestPluginDisableError(t *testing.T) {
16 16
 		client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
17 17
 	}
18 18
 
19
-	err := client.PluginDisable(context.Background(), "plugin_name")
19
+	err := client.PluginDisable(context.Background(), "plugin_name", types.PluginDisableOptions{})
20 20
 	if err == nil || err.Error() != "Error response from daemon: Server error" {
21 21
 		t.Fatalf("expected a Server Error, got %v", err)
22 22
 	}
... ...
@@ -40,7 +41,7 @@ func TestPluginDisable(t *testing.T) {
40 40
 		}),
41 41
 	}
42 42
 
43
-	err := client.PluginDisable(context.Background(), "plugin_name")
43
+	err := client.PluginDisable(context.Background(), "plugin_name", types.PluginDisableOptions{})
44 44
 	if err != nil {
45 45
 		t.Fatal(err)
46 46
 	}
... ...
@@ -21,11 +21,13 @@ Usage:  docker plugin disable PLUGIN
21 21
 Disable a plugin
22 22
 
23 23
 Options:
24
-      --help   Print usage
24
+  -f, --force   Force the disable of an active plugin
25
+      --help    Print usage
25 26
 ```
26 27
 
27 28
 Disables a plugin. The plugin must be installed before it can be disabled,
28
-see [`docker plugin install`](plugin_install.md).
29
+see [`docker plugin install`](plugin_install.md). Without the `-f` option,
30
+a plugin that has references (eg, volumes, networks) cannot be disabled.
29 31
 
30 32
 
31 33
 The following example shows that the `no-remove` plugin is installed
... ...
@@ -268,10 +268,17 @@ func (s *DockerDaemonSuite) TestPluginVolumeRemoveOnRestart(c *check.C) {
268 268
 	s.d.Restart(c, "--live-restore=true")
269 269
 
270 270
 	out, err = s.d.Cmd("plugin", "disable", pName)
271
-	c.Assert(err, checker.IsNil, check.Commentf(out))
272
-	out, err = s.d.Cmd("plugin", "rm", pName)
273 271
 	c.Assert(err, checker.NotNil, check.Commentf(out))
274 272
 	c.Assert(out, checker.Contains, "in use")
273
+
274
+	out, err = s.d.Cmd("volume", "rm", "test")
275
+	c.Assert(err, checker.IsNil, check.Commentf(out))
276
+
277
+	out, err = s.d.Cmd("plugin", "disable", pName)
278
+	c.Assert(err, checker.IsNil, check.Commentf(out))
279
+
280
+	out, err = s.d.Cmd("plugin", "rm", pName)
281
+	c.Assert(err, checker.IsNil, check.Commentf(out))
275 282
 }
276 283
 
277 284
 func existsMountpointWithPrefix(mountpointPrefix string) (bool, error) {
... ...
@@ -64,23 +64,18 @@ func (s *DockerSuite) TestPluginForceRemove(c *check.C) {
64 64
 
65 65
 func (s *DockerSuite) TestPluginActive(c *check.C) {
66 66
 	testRequires(c, DaemonIsLinux, IsAmd64, Network)
67
-	out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", pNameWithTag)
67
+	_, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", pNameWithTag)
68 68
 	c.Assert(err, checker.IsNil)
69 69
 
70
-	out, _, err = dockerCmdWithError("volume", "create", "-d", pNameWithTag)
70
+	_, _, err = dockerCmdWithError("volume", "create", "-d", pNameWithTag, "--name", "testvol1")
71 71
 	c.Assert(err, checker.IsNil)
72 72
 
73
-	vID := strings.TrimSpace(out)
74
-
75
-	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
76
-	c.Assert(out, checker.Contains, "is in use")
73
+	out, _, err := dockerCmdWithError("plugin", "disable", pNameWithTag)
74
+	c.Assert(out, checker.Contains, "in use")
77 75
 
78
-	_, _, err = dockerCmdWithError("volume", "rm", vID)
76
+	_, _, err = dockerCmdWithError("volume", "rm", "testvol1")
79 77
 	c.Assert(err, checker.IsNil)
80 78
 
81
-	out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag)
82
-	c.Assert(out, checker.Contains, "is enabled")
83
-
84 79
 	_, _, err = dockerCmdWithError("plugin", "disable", pNameWithTag)
85 80
 	c.Assert(err, checker.IsNil)
86 81
 
... ...
@@ -31,8 +31,8 @@ var (
31 31
 	validPartialID = regexp.MustCompile(`^([a-f0-9]{1,64})$`)
32 32
 )
33 33
 
34
-// Disable deactivates a plugin, which implies that they cannot be used by containers.
35
-func (pm *Manager) Disable(name string) error {
34
+// Disable deactivates a plugin. This means resources (volumes, networks) cant use them.
35
+func (pm *Manager) Disable(name string, config *types.PluginDisableConfig) error {
36 36
 	p, err := pm.pluginStore.GetByName(name)
37 37
 	if err != nil {
38 38
 		return err
... ...
@@ -41,6 +41,10 @@ func (pm *Manager) Disable(name string) error {
41 41
 	c := pm.cMap[p]
42 42
 	pm.mu.RUnlock()
43 43
 
44
+	if !config.ForceDisable && p.GetRefCount() > 0 {
45
+		return fmt.Errorf("plugin %s is in use", p.Name())
46
+	}
47
+
44 48
 	if err := pm.disable(p, c); err != nil {
45 49
 		return err
46 50
 	}
... ...
@@ -15,7 +15,7 @@ import (
15 15
 var errNotSupported = errors.New("plugins are not supported on this platform")
16 16
 
17 17
 // Disable deactivates a plugin, which implies that they cannot be used by containers.
18
-func (pm *Manager) Disable(name string) error {
18
+func (pm *Manager) Disable(name string, config *types.PluginDisableConfig) error {
19 19
 	return errNotSupported
20 20
 }
21 21