Skip to content

Commit 8cb2229

Browse files
anusha-ragunathananusha-ragunathan
committed
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 <[email protected]>
1 parent e6d06a5 commit 8cb2229

13 files changed

Lines changed: 69 additions & 31 deletions

File tree

‎api/server/router/plugin/backend.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010

1111
// Backend for Plugin
1212
type Backend interface {
13-
Disable(name string) error
13+
Disable(name string, config *enginetypes.PluginDisableConfig) error
1414
Enable(name string, config *enginetypes.PluginEnableConfig) error
1515
List() ([]enginetypes.Plugin, error)
1616
Inspect(name string) (enginetypes.Plugin, error)

‎api/server/router/plugin/plugin_routes.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,16 @@ func (pr *pluginRouter) enablePlugin(ctx context.Context, w http.ResponseWriter,
9999
}
100100

101101
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)
103112
}
104113

105114
func (pr *pluginRouter) removePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {

‎api/types/client.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,11 @@ type PluginEnableOptions struct {
340340
Timeout int
341341
}
342342

343+
// PluginDisableOptions holds parameters to disable plugins.
344+
type PluginDisableOptions struct {
345+
Force bool
346+
}
347+
343348
// PluginInstallOptions holds parameters to install a plugin.
344349
type PluginInstallOptions struct {
345350
Disabled bool

‎api/types/configs.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,17 @@ type ExecConfig struct {
5353
Cmd []string // Execution commands and args
5454
}
5555

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.
5957
type PluginRmConfig struct {
6058
ForceRemove bool
6159
}
6260

63-
// PluginEnableConfig holds arguments for the plugin enable
61+
// PluginEnableConfig holds arguments for plugin enable
6462
type PluginEnableConfig struct {
6563
Timeout int
6664
}
65+
66+
// PluginDisableConfig holds arguments for plugin disable.
67+
type PluginDisableConfig struct {
68+
ForceDisable bool
69+
}

‎cli/command/plugin/disable.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package plugin
33
import (
44
"fmt"
55

6+
"github.com/docker/docker/api/types"
67
"github.com/docker/docker/cli"
78
"github.com/docker/docker/cli/command"
89
"github.com/docker/docker/reference"
@@ -11,19 +12,23 @@ import (
1112
)
1213

1314
func newDisableCommand(dockerCli *command.DockerCli) *cobra.Command {
15+
var force bool
16+
1417
cmd := &cobra.Command{
1518
Use: "disable PLUGIN",
1619
Short: "Disable a plugin",
1720
Args: cli.ExactArgs(1),
1821
RunE: func(cmd *cobra.Command, args []string) error {
19-
return runDisable(dockerCli, args[0])
22+
return runDisable(dockerCli, args[0], force)
2023
},
2124
}
2225

26+
flags := cmd.Flags()
27+
flags.BoolVarP(&force, "force", "f", false, "Force the disable of an active plugin")
2328
return cmd
2429
}
2530

26-
func runDisable(dockerCli *command.DockerCli, name string) error {
31+
func runDisable(dockerCli *command.DockerCli, name string, force bool) error {
2732
named, err := reference.ParseNamed(name) // FIXME: validate
2833
if err != nil {
2934
return err
@@ -35,7 +40,7 @@ func runDisable(dockerCli *command.DockerCli, name string) error {
3540
if !ok {
3641
return fmt.Errorf("invalid name: %s", named.String())
3742
}
38-
if err := dockerCli.Client().PluginDisable(context.Background(), ref.String()); err != nil {
43+
if err := dockerCli.Client().PluginDisable(context.Background(), ref.String(), types.PluginDisableOptions{Force: force}); err != nil {
3944
return err
4045
}
4146
fmt.Fprintln(dockerCli.Out(), name)

‎client/interface.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ type PluginAPIClient interface {
110110
PluginList(ctx context.Context) (types.PluginsListResponse, error)
111111
PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error
112112
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
114114
PluginInstall(ctx context.Context, name string, options types.PluginInstallOptions) error
115115
PluginPush(ctx context.Context, name string, registryAuth string) error
116116
PluginSet(ctx context.Context, name string, args []string) error

‎client/plugin_disable.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
package client
22

33
import (
4+
"net/url"
5+
6+
"github.com/docker/docker/api/types"
47
"golang.org/x/net/context"
58
)
69

710
// 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)
1017
ensureReaderClosed(resp)
1118
return err
1219
}

‎client/plugin_disable_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99
"testing"
1010

11+
"github.com/docker/docker/api/types"
1112
"golang.org/x/net/context"
1213
)
1314

@@ -16,7 +17,7 @@ func TestPluginDisableError(t *testing.T) {
1617
client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
1718
}
1819

19-
err := client.PluginDisable(context.Background(), "plugin_name")
20+
err := client.PluginDisable(context.Background(), "plugin_name", types.PluginDisableOptions{})
2021
if err == nil || err.Error() != "Error response from daemon: Server error" {
2122
t.Fatalf("expected a Server Error, got %v", err)
2223
}
@@ -40,7 +41,7 @@ func TestPluginDisable(t *testing.T) {
4041
}),
4142
}
4243

43-
err := client.PluginDisable(context.Background(), "plugin_name")
44+
err := client.PluginDisable(context.Background(), "plugin_name", types.PluginDisableOptions{})
4445
if err != nil {
4546
t.Fatal(err)
4647
}

‎docs/reference/commandline/plugin_disable.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ Usage: docker plugin disable PLUGIN
2121
Disable a plugin
2222

2323
Options:
24-
--help Print usage
24+
-f, --force Force the disable of an active plugin
25+
--help Print usage
2526
```
2627

2728
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.
2931

3032

3133
The following example shows that the `no-remove` plugin is installed

‎integration-cli/docker_cli_daemon_plugins_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,17 @@ func (s *DockerDaemonSuite) TestPluginVolumeRemoveOnRestart(c *check.C) {
268268
s.d.Restart(c, "--live-restore=true")
269269

270270
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)
273271
c.Assert(err, checker.NotNil, check.Commentf(out))
274272
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))
275282
}
276283

277284
func existsMountpointWithPrefix(mountpointPrefix string) (bool, error) {

0 commit comments

Comments
 (0)