Merge pull request #23681 from tiborvass/plugins-fixes

Plugins fixes

api/client/plugin/disable.go

@@ -3,21 +3,39 @@
 package plugin
 
 import (
+	"fmt"
+
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/reference"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 )
 
 func newDisableCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "disable",
+		Use:   "disable PLUGIN",
 		Short: "Disable a plugin",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return dockerCli.Client().PluginDisable(context.Background(), args[0])
+			return runDisable(dockerCli, args[0])
 		},
 	}
 
 	return cmd
 }
+
+func runDisable(dockerCli *client.DockerCli, name string) error {
+	named, err := reference.ParseNamed(name) // FIXME: validate
+	if err != nil {
+		return err
+	}
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
+	ref, ok := named.(reference.NamedTagged)
+	if !ok {
+		return fmt.Errorf("invalid name: %s", named.String())
+	}
+	return dockerCli.Client().PluginDisable(context.Background(), ref.String())
+}

api/client/plugin/enable.go

@@ -3,21 +3,39 @@
 package plugin
 
 import (
+	"fmt"
+
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/reference"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 )
 
 func newEnableCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "enable",
+		Use:   "enable PLUGIN",
 		Short: "Enable a plugin",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return dockerCli.Client().PluginEnable(context.Background(), args[0])
+			return runEnable(dockerCli, args[0])
 		},
 	}
 
 	return cmd
 }
+
+func runEnable(dockerCli *client.DockerCli, name string) error {
+	named, err := reference.ParseNamed(name) // FIXME: validate
+	if err != nil {
+		return err
+	}
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
+	ref, ok := named.(reference.NamedTagged)
+	if !ok {
+		return fmt.Errorf("invalid name: %s", named.String())
+	}
+	return dockerCli.Client().PluginEnable(context.Background(), ref.String())
+}

api/client/plugin/inspect.go

@@ -4,16 +4,18 @@ package plugin
 
 import (
 	"encoding/json"
+	"fmt"
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/reference"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 )
 
 func newInspectCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "inspect",
+		Use:   "inspect PLUGIN",
 		Short: "Inspect a plugin",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -25,7 +27,18 @@ func newInspectCommand(dockerCli *client.DockerCli) *cobra.Command {
 }
 
 func runInspect(dockerCli *client.DockerCli, name string) error {
-	p, err := dockerCli.Client().PluginInspect(context.Background(), name)
+	named, err := reference.ParseNamed(name) // FIXME: validate
+	if err != nil {
+		return err
+	}
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
+	ref, ok := named.(reference.NamedTagged)
+	if !ok {
+		return fmt.Errorf("invalid name: %s", named.String())
+	}
+	p, err := dockerCli.Client().PluginInspect(context.Background(), ref.String())
 	if err != nil {
 		return err
 	}

api/client/plugin/install.go

@@ -25,7 +25,7 @@ type pluginOptions struct {
 func newInstallCommand(dockerCli *client.DockerCli) *cobra.Command {
 	var options pluginOptions
 	cmd := &cobra.Command{
-		Use:   "install",
+		Use:   "install PLUGIN",
 		Short: "Install a plugin",
 		Args:  cli.RequiresMinArgs(1), // TODO: allow for set args
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -35,7 +35,7 @@ func newInstallCommand(dockerCli *client.DockerCli) *cobra.Command {
 	}
 
 	flags := cmd.Flags()
-	flags.BoolVar(&options.grantPerms, "grant-all-permissions", true, "grant all permissions necessary to run the plugin")
+	flags.BoolVar(&options.grantPerms, "grant-all-permissions", false, "grant all permissions necessary to run the plugin")
 	flags.BoolVar(&options.disable, "disable", false, "do not enable the plugin on install")
 
 	return cmd
@@ -46,7 +46,9 @@ func runInstall(dockerCli *client.DockerCli, opts pluginOptions) error {
 	if err != nil {
 		return err
 	}
-	named = reference.WithDefaultTag(named)
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
 	ref, ok := named.(reference.NamedTagged)
 	if !ok {
 		return fmt.Errorf("invalid name: %s", named.String())
@@ -62,14 +64,15 @@ func runInstall(dockerCli *client.DockerCli, opts pluginOptions) error {
 		return err
 	}
 
-	requestPrivilege := dockerCli.RegistryAuthenticationPrivilegedFunc(repoInfo.Index, "plugin install")
+	registryAuthFunc := dockerCli.RegistryAuthenticationPrivilegedFunc(repoInfo.Index, "plugin install")
 
 	options := types.PluginInstallOptions{
 		RegistryAuth:          encodedAuth,
 		Disabled:              opts.disable,
 		AcceptAllPermissions:  opts.grantPerms,
 		AcceptPermissionsFunc: acceptPrivileges(dockerCli, opts.name),
-		PrivilegeFunc:         requestPrivilege,
+		// TODO: Rename PrivilegeFunc, it has nothing to do with privileges
+		PrivilegeFunc: registryAuthFunc,
 	}
 
 	return dockerCli.Client().PluginInstall(ctx, ref.String(), options)
@@ -77,7 +80,7 @@ func runInstall(dockerCli *client.DockerCli, opts pluginOptions) error {
 
 func acceptPrivileges(dockerCli *client.DockerCli, name string) func(privileges types.PluginPrivileges) (bool, error) {
 	return func(privileges types.PluginPrivileges) (bool, error) {
-		fmt.Fprintf(dockerCli.Out(), "Plugin %q requested the following privileges:\n", name)
+		fmt.Fprintf(dockerCli.Out(), "Plugin %q is requesting the following privileges:\n", name)
 		for _, privilege := range privileges {
 			fmt.Fprintf(dockerCli.Out(), " - %s: %v\n", privilege.Name, privilege.Value)
 		}

api/client/plugin/push.go

@@ -16,7 +16,7 @@ import (
 
 func newPushCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "push",
+		Use:   "push PLUGIN",
 		Short: "Push a plugin",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -31,7 +31,9 @@ func runPush(dockerCli *client.DockerCli, name string) error {
 	if err != nil {
 		return err
 	}
-	named = reference.WithDefaultTag(named)
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
 	ref, ok := named.(reference.NamedTagged)
 	if !ok {
 		return fmt.Errorf("invalid name: %s", named.String())

api/client/plugin/remove.go

@@ -7,13 +7,14 @@ import (
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/reference"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 )
 
 func newRemoveCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "rm",
+		Use:     "rm PLUGIN",
 		Short:   "Remove a plugin",
 		Aliases: []string{"remove"},
 		Args:    cli.RequiresMinArgs(1),
@@ -28,8 +29,19 @@ func newRemoveCommand(dockerCli *client.DockerCli) *cobra.Command {
 func runRemove(dockerCli *client.DockerCli, names []string) error {
 	var errs cli.Errors
 	for _, name := range names {
+		named, err := reference.ParseNamed(name) // FIXME: validate
+		if err != nil {
+			return err
+		}
+		if reference.IsNameOnly(named) {
+			named = reference.WithDefaultTag(named)
+		}
+		ref, ok := named.(reference.NamedTagged)
+		if !ok {
+			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(), name); err != nil {
+		if err := dockerCli.Client().PluginRemove(context.Background(), ref.String()); err != nil {
 			errs = append(errs, err)
 			continue
 		}

api/client/plugin/set.go

@@ -3,16 +3,19 @@
 package plugin
 
 import (
+	"fmt"
+
 	"golang.org/x/net/context"
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/reference"
 	"github.com/spf13/cobra"
 )
 
 func newSetCommand(dockerCli *client.DockerCli) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "set",
+		Use:   "set PLUGIN key1=value1 [key2=value2...]",
 		Short: "Change settings for a plugin",
 		Args:  cli.RequiresMinArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -24,5 +27,16 @@ func newSetCommand(dockerCli *client.DockerCli) *cobra.Command {
 }
 
 func runSet(dockerCli *client.DockerCli, name string, args []string) error {
-	return dockerCli.Client().PluginSet(context.Background(), name, args)
+	named, err := reference.ParseNamed(name) // FIXME: validate
+	if err != nil {
+		return err
+	}
+	if reference.IsNameOnly(named) {
+		named = reference.WithDefaultTag(named)
+	}
+	ref, ok := named.(reference.NamedTagged)
+	if !ok {
+		return fmt.Errorf("invalid name: %s", named.String())
+	}
+	return dockerCli.Client().PluginSet(context.Background(), ref.String(), args)
 }

integration-cli/docker_cli_plugins_test.go

@@ -11,7 +11,7 @@ func (s *DockerSuite) TestPluginBasicOps(c *check.C) {
 	tag := "latest"
 	nameWithTag := name + ":" + tag
 
-	_, _, err := dockerCmdWithError("plugin", "install", name)
+	_, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", name)
 	c.Assert(err, checker.IsNil)
 
 	out, _, err := dockerCmdWithError("plugin", "ls")
@@ -41,7 +41,7 @@ func (s *DockerSuite) TestPluginInstallDisable(c *check.C) {
 	tag := "latest"
 	nameWithTag := name + ":" + tag
 
-	_, _, err := dockerCmdWithError("plugin", "install", name, "--disable")
+	_, _, err := dockerCmdWithError("plugin", "install", "--grant-all-permissions", "--disable", name)
 	c.Assert(err, checker.IsNil)
 
 	out, _, err := dockerCmdWithError("plugin", "ls")

plugin/manager.go

@@ -22,10 +22,7 @@ import (
 	"github.com/docker/engine-api/types"
 )
 
-const (
-	defaultPluginRuntimeDestination = "/run/docker/plugins"
-	defaultPluginStateDestination   = "/state"
-)
+const defaultPluginRuntimeDestination = "/run/docker/plugins"
 
 var manager *Manager
 
@@ -49,7 +46,6 @@ type plugin struct {
 	P                 types.Plugin `json:"plugin"`
 	client            *plugins.Client
 	restartManager    restartmanager.RestartManager
-	stateSourcePath   string
 	runtimeSourcePath string
 }
 
@@ -72,7 +68,6 @@ func (pm *Manager) newPlugin(ref reference.Named, id string) *plugin {
 			Name: ref.Name(),
 			ID:   id,
 		},
-		stateSourcePath:   filepath.Join(pm.libRoot, id, "state"),
 		runtimeSourcePath: filepath.Join(pm.runRoot, id),
 	}
 	if ref, ok := ref.(reference.NamedTagged); ok {
@@ -82,7 +77,6 @@ func (pm *Manager) newPlugin(ref reference.Named, id string) *plugin {
 }
 
 func (pm *Manager) restorePlugin(p *plugin) error {
-	p.stateSourcePath = filepath.Join(pm.libRoot, p.P.ID, "state")
 	p.runtimeSourcePath = filepath.Join(pm.runRoot, p.P.ID)
 	if p.P.Active {
 		return pm.restore(p)
@@ -210,7 +204,18 @@ func LookupWithCapability(name, capability string) (Plugin, error) {
 	)
 	handleLegacy := true
 	if manager != nil {
-		p, err = manager.get(name)
+		fullName := name
+		if named, err := reference.ParseNamed(fullName); err == nil { // FIXME: validate
+			if reference.IsNameOnly(named) {
+				named = reference.WithDefaultTag(named)
+			}
+			ref, ok := named.(reference.NamedTagged)
+			if !ok {
+				return nil, fmt.Errorf("invalid name: %s", named.String())
+			}
+			fullName = ref.String()
+		}
+		p, err = manager.get(fullName)
 		if err != nil {
 			if _, ok := err.(ErrNotFound); !ok {
 				return nil, err
@@ -342,7 +347,6 @@ func (pm *Manager) remove(p *plugin) error {
 	}
 	pm.Lock() // fixme: lock single record
 	defer pm.Unlock()
-	os.RemoveAll(p.stateSourcePath)
 	delete(pm.plugins, p.P.ID)
 	delete(pm.nameToID, p.Name())
 	pm.save()

plugin/manager_linux.go

@@ -70,11 +70,6 @@ func (pm *Manager) initSpec(p *plugin) (*specs.Spec, error) {
 		Destination: defaultPluginRuntimeDestination,
 		Type:        "bind",
 		Options:     []string{"rbind", "rshared"},
-	}, types.PluginMount{
-		Source:      &p.stateSourcePath,
-		Destination: defaultPluginStateDestination,
-		Type:        "bind",
-		Options:     []string{"rbind", "rshared"},
 	})
 	for _, mount := range mounts {
 		m := specs.Mount{
@@ -105,10 +100,14 @@ func (pm *Manager) initSpec(p *plugin) (*specs.Spec, error) {
 	envs = append(envs, p.P.Config.Env...)
 
 	args := append(p.P.Manifest.Entrypoint, p.P.Config.Args...)
+	cwd := p.P.Manifest.Workdir
+	if len(cwd) == 0 {
+		cwd = "/"
+	}
 	s.Process = specs.Process{
 		Terminal: false,
 		Args:     args,
-		Cwd:      "/", // TODO: add in manifest?
+		Cwd:      cwd,
 		Env:      envs,
 	}