From 0016b331dac94661678fd7676c7b6ccc9ec2d147 Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Fri, 22 Jul 2016 08:24:54 -0700 Subject: [PATCH] Add --force to docker plugin remove Signed-off-by: Victor Vieux --- api/client/plugin/remove.go | 26 ++++++++++++++----- api/server/router/plugin/backend.go | 2 +- api/server/router/plugin/plugin_routes.go | 10 ++++++- docs/reference/commandline/plugin_rm.md | 8 +++--- hack/vendor.sh | 2 +- integration-cli/docker_cli_plugins_test.go | 13 ++++++++++ plugin/backend.go | 4 +-- plugin/manager.go | 17 +++++++----- .../client/interface_experimental.go | 2 +- .../docker/engine-api/client/plugin_remove.go | 12 +++++++-- .../docker/engine-api/types/client.go | 5 ++++ .../docker/engine-api/types/configs.go | 7 +++++ .../engine-api/types/container/config.go | 2 +- 13 files changed, 86 insertions(+), 24 deletions(-) diff --git a/api/client/plugin/remove.go b/api/client/plugin/remove.go index 5aba24415b..1b447b8709 100644 --- a/api/client/plugin/remove.go +++ b/api/client/plugin/remove.go @@ -8,27 +8,41 @@ import ( "github.com/docker/docker/api/client" "github.com/docker/docker/cli" "github.com/docker/docker/reference" + "github.com/docker/engine-api/types" "github.com/spf13/cobra" "golang.org/x/net/context" ) +type rmOptions struct { + force bool + + plugins []string +} + func newRemoveCommand(dockerCli *client.DockerCli) *cobra.Command { + var opts rmOptions + cmd := &cobra.Command{ - Use: "rm PLUGIN", - Short: "Remove a plugin", + Use: "rm [OPTIONS] PLUGIN [PLUGIN...]", + Short: "Remove one or more plugins", Aliases: []string{"remove"}, Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRemove(dockerCli, args) + opts.plugins = args + return runRemove(dockerCli, &opts) }, } + flags := cmd.Flags() + flags.BoolVarP(&opts.force, "force", "f", false, "Force the removal of an active plugin") return cmd } -func runRemove(dockerCli *client.DockerCli, names []string) error { +func runRemove(dockerCli *client.DockerCli, opts *rmOptions) error { + ctx := context.Background() + var errs cli.Errors - for _, name := range names { + for _, name := range opts.plugins { named, err := reference.ParseNamed(name) // FIXME: validate if err != nil { return err @@ -41,7 +55,7 @@ func runRemove(dockerCli *client.DockerCli, names []string) error { return fmt.Errorf("invalid name: %s", named.String()) } // TODO: pass names to api instead of making multiple api calls - if err := dockerCli.Client().PluginRemove(context.Background(), ref.String()); err != nil { + if err := dockerCli.Client().PluginRemove(ctx, ref.String(), types.PluginRemoveOptions{Force: opts.force}); err != nil { errs = append(errs, err) continue } diff --git a/api/server/router/plugin/backend.go b/api/server/router/plugin/backend.go index 0eb4f5b8f1..42392fec3c 100644 --- a/api/server/router/plugin/backend.go +++ b/api/server/router/plugin/backend.go @@ -14,7 +14,7 @@ type Backend interface { Enable(name string) error List() ([]enginetypes.Plugin, error) Inspect(name string) (enginetypes.Plugin, error) - Remove(name string) error + Remove(name string, config *enginetypes.PluginRmConfig) error Set(name string, args []string) error Pull(name string, metaHeaders http.Header, authConfig *enginetypes.AuthConfig) (enginetypes.PluginPrivileges, error) Push(name string, metaHeaders http.Header, authConfig *enginetypes.AuthConfig) error diff --git a/api/server/router/plugin/plugin_routes.go b/api/server/router/plugin/plugin_routes.go index dfdde72483..42fa32983a 100644 --- a/api/server/router/plugin/plugin_routes.go +++ b/api/server/router/plugin/plugin_routes.go @@ -51,7 +51,15 @@ func (pr *pluginRouter) disablePlugin(ctx context.Context, w http.ResponseWriter } func (pr *pluginRouter) removePlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - return pr.backend.Remove(vars["name"]) + if err := httputils.ParseForm(r); err != nil { + return err + } + + name := vars["name"] + config := &types.PluginRmConfig{ + ForceRemove: httputils.BoolValue(r, "force"), + } + return pr.backend.Remove(name, config) } func (pr *pluginRouter) pushPlugin(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { diff --git a/docs/reference/commandline/plugin_rm.md b/docs/reference/commandline/plugin_rm.md index 4225b5fad2..6535b5e102 100644 --- a/docs/reference/commandline/plugin_rm.md +++ b/docs/reference/commandline/plugin_rm.md @@ -12,7 +12,7 @@ parent = "smn_cli" # plugin rm (experimental) ```markdown -Usage: docker plugin rm PLUGIN +Usage: docker plugin rm [OPTIONS] PLUGIN [PLUGIN...] Remove a plugin @@ -20,12 +20,14 @@ Aliases: rm, remove Options: - --help Print usage + -f, --force Force the removal of an active plugin + --help Print usage ``` Removes a plugin. You cannot remove a plugin if it is active, you must disable a plugin using the [`docker plugin disable`](plugin_disable.md) before removing -it. +it (or use --force, use of force is not recommended, since it can affect +functioning of running containers using the plugin). The following example disables and removes the `no-remove:latest` plugin; diff --git a/hack/vendor.sh b/hack/vendor.sh index 87738fced2..54a4c50ce0 100755 --- a/hack/vendor.sh +++ b/hack/vendor.sh @@ -61,7 +61,7 @@ clone git golang.org/x/sys eb2c74142fd19a79b3f237334c7384d5167b1b46 https://gith clone git github.com/docker/go-units 651fc226e7441360384da338d0fd37f2440ffbe3 clone git github.com/docker/go-connections fa2850ff103453a9ad190da0df0af134f0314b3d -clone git github.com/docker/engine-api 228c7390a733320d48697cb41ae8cde4942cd3e5 +clone git github.com/docker/engine-api 603ec41824c63d1e6498a22271987fa1f268c0c0 clone git github.com/RackSec/srslog 259aed10dfa74ea2961eddd1d9847619f6e98837 clone git github.com/imdario/mergo 0.2.1 diff --git a/integration-cli/docker_cli_plugins_test.go b/integration-cli/docker_cli_plugins_test.go index c873b75b5d..e16d8e0017 100644 --- a/integration-cli/docker_cli_plugins_test.go +++ b/integration-cli/docker_cli_plugins_test.go @@ -57,6 +57,19 @@ func (s *DockerSuite) TestPluginBasicOps(c *check.C) { } } +func (s *DockerSuite) TestPluginForceRemove(c *check.C) { + testRequires(c, DaemonIsLinux, ExperimentalDaemon) + out, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", pNameWithTag) + c.Assert(err, checker.IsNil) + + out, _, err = dockerCmdWithError("plugin", "remove", pNameWithTag) + c.Assert(out, checker.Contains, "is active") + + out, _, err = dockerCmdWithError("plugin", "remove", "--force", 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) diff --git a/plugin/backend.go b/plugin/backend.go index ca31620602..5b2d9f46a6 100644 --- a/plugin/backend.go +++ b/plugin/backend.go @@ -131,12 +131,12 @@ func (pm *Manager) Push(name string, metaHeader http.Header, authConfig *types.A } // Remove deletes plugin's root directory. -func (pm *Manager) Remove(name string) error { +func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error { p, err := pm.get(name) if err != nil { return err } - if err := pm.remove(p); err != nil { + if err := pm.remove(p, config.ForceRemove); err != nil { return err } pm.pluginEventLogger(p.PluginObj.ID, name, "remove") diff --git a/plugin/manager.go b/plugin/manager.go index fae852b1d1..84d616179c 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -310,7 +310,7 @@ func (pm *Manager) init() error { go func(p *plugin) { defer group.Done() if err := pm.restorePlugin(p); err != nil { - logrus.Errorf("Error restoring plugin '%s': %s", p.Name(), err) + logrus.Errorf("failed to restore plugin '%s': %s", p.Name(), err) return } @@ -322,7 +322,7 @@ func (pm *Manager) init() error { if requiresManualRestore { // if liveRestore is not enabled, the plugin will be stopped now so we should enable it if err := pm.enable(p); err != nil { - logrus.Errorf("Error enabling plugin '%s': %s", p.Name(), err) + logrus.Errorf("failed to enable plugin '%s': %s", p.Name(), err) } } }(p) @@ -363,9 +363,14 @@ func (pm *Manager) initPlugin(p *plugin) error { return err } -func (pm *Manager) remove(p *plugin) error { +func (pm *Manager) remove(p *plugin, force bool) error { if p.PluginObj.Active { - return fmt.Errorf("plugin %s is active", p.Name()) + if !force { + return fmt.Errorf("plugin %s is active", p.Name()) + } + if err := pm.disable(p); err != nil { + logrus.Errorf("failed to disable plugin '%s': %s", p.Name(), err) + } } pm.Lock() // fixme: lock single record defer pm.Unlock() @@ -380,7 +385,7 @@ func (pm *Manager) set(p *plugin, args []string) error { for _, arg := range args { i := strings.Index(arg, "=") if i < 0 { - return fmt.Errorf("No equal sign '=' found in %s", arg) + return fmt.Errorf("no equal sign '=' found in %s", arg) } m[arg[:i]] = arg[i+1:] } @@ -393,7 +398,7 @@ func (pm *Manager) save() error { jsonData, err := json.Marshal(pm.plugins) if err != nil { - logrus.Debugf("Error in json.Marshal: %v", err) + logrus.Debugf("failure in json.Marshal: %v", err) return err } ioutils.AtomicWriteFile(filePath, jsonData, 0600) diff --git a/vendor/src/github.com/docker/engine-api/client/interface_experimental.go b/vendor/src/github.com/docker/engine-api/client/interface_experimental.go index eb0cd7bf14..199619fbd2 100644 --- a/vendor/src/github.com/docker/engine-api/client/interface_experimental.go +++ b/vendor/src/github.com/docker/engine-api/client/interface_experimental.go @@ -24,7 +24,7 @@ type CheckpointAPIClient interface { // PluginAPIClient defines API client methods for the plugins type PluginAPIClient interface { PluginList(ctx context.Context) (types.PluginsListResponse, error) - PluginRemove(ctx context.Context, name string) error + PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error PluginEnable(ctx context.Context, name string) error PluginDisable(ctx context.Context, name string) error PluginInstall(ctx context.Context, name string, options types.PluginInstallOptions) error diff --git a/vendor/src/github.com/docker/engine-api/client/plugin_remove.go b/vendor/src/github.com/docker/engine-api/client/plugin_remove.go index baf666556b..9fe18b50ad 100644 --- a/vendor/src/github.com/docker/engine-api/client/plugin_remove.go +++ b/vendor/src/github.com/docker/engine-api/client/plugin_remove.go @@ -3,12 +3,20 @@ package client import ( + "net/url" + + "github.com/docker/engine-api/types" "golang.org/x/net/context" ) // PluginRemove removes a plugin -func (cli *Client) PluginRemove(ctx context.Context, name string) error { - resp, err := cli.delete(ctx, "/plugins/"+name, nil, nil) +func (cli *Client) PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error { + query := url.Values{} + if options.Force { + query.Set("force", "1") + } + + resp, err := cli.delete(ctx, "/plugins/"+name, query, nil) ensureReaderClosed(resp) return err } diff --git a/vendor/src/github.com/docker/engine-api/types/client.go b/vendor/src/github.com/docker/engine-api/types/client.go index c6d244d316..1820f8bc8c 100644 --- a/vendor/src/github.com/docker/engine-api/types/client.go +++ b/vendor/src/github.com/docker/engine-api/types/client.go @@ -289,3 +289,8 @@ type ServiceListOptions struct { type TaskListOptions struct { Filter filters.Args } + +// PluginRemoveOptions holds parameters to remove plugins. +type PluginRemoveOptions struct { + Force bool +} diff --git a/vendor/src/github.com/docker/engine-api/types/configs.go b/vendor/src/github.com/docker/engine-api/types/configs.go index 93384b9fad..13e73cb9bd 100644 --- a/vendor/src/github.com/docker/engine-api/types/configs.go +++ b/vendor/src/github.com/docker/engine-api/types/configs.go @@ -51,3 +51,10 @@ type ExecConfig struct { DetachKeys string // Escape keys for detach Cmd []string // Execution commands and args } + +// PluginRmConfig holds arguments for the plugin remove +// operation. This struct is used to tell the backend what operations +// to perform. +type PluginRmConfig struct { + ForceRemove bool +} diff --git a/vendor/src/github.com/docker/engine-api/types/container/config.go b/vendor/src/github.com/docker/engine-api/types/container/config.go index 707fc8c174..e300e119e7 100644 --- a/vendor/src/github.com/docker/engine-api/types/container/config.go +++ b/vendor/src/github.com/docker/engine-api/types/container/config.go @@ -36,7 +36,7 @@ type HealthConfig struct { type Config struct { Hostname string // Hostname Domainname string // Domainname - User string // User that will run the command(s) inside the container + User string // User that will run the command(s) inside the container, also support user:group AttachStdin bool // Attach the standard input, makes possible user interaction AttachStdout bool // Attach the standard output AttachStderr bool // Attach the standard error