Fix issue caused by duplicate `docker plugin create` with same names

This fix tries to fix the issue raised in 28684:
1. Duplicate plugin create with the same name will override the old plugin reference
2. In case an error happens in the middle of the plugin creation, plugin directories
   in `/var/lib/docker/plugins` are not cleaned up.

This fix update the plugin store so that `Add()` will return an error if a plugin
with the same name already exist.

This fix also will clean up the directory in `/var/lib/docker/plugins` in case
an error happens in the middle of the plugin creation.

This fix fixes 28684.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

integration-cli/docker_cli_plugins_test.go

@@ -4,6 +4,7 @@ import (
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/go-check/check"
 
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -169,3 +170,34 @@ func (s *DockerSuite) TestPluginEnableDisableNegative(c *check.C) {
 	_, _, err = dockerCmdWithError("plugin", "remove", pName)
 	c.Assert(err, checker.IsNil)
 }
+
+func (s *DockerSuite) TestPluginCreate(c *check.C) {
+	testRequires(c, DaemonIsLinux, Network)
+
+	name := "foo/bar-driver"
+	temp, err := ioutil.TempDir("", "foo")
+	c.Assert(err, checker.IsNil)
+	defer os.RemoveAll(temp)
+
+	data := `{"description": "foo plugin"}`
+	err = ioutil.WriteFile(filepath.Join(temp, "config.json"), []byte(data), 0644)
+	c.Assert(err, checker.IsNil)
+
+	out, _, err := dockerCmdWithError("plugin", "create", name, temp)
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, name)
+
+	out, _, err = dockerCmdWithError("plugin", "ls")
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, name)
+
+	out, _, err = dockerCmdWithError("plugin", "create", name, temp)
+	c.Assert(err, checker.NotNil)
+	c.Assert(out, checker.Contains, "already exist")
+
+	out, _, err = dockerCmdWithError("plugin", "ls")
+	c.Assert(err, checker.IsNil)
+	c.Assert(out, checker.Contains, name)
+	// The output will consists of one HEADER line and one line of foo/bar-driver
+	c.Assert(len(strings.Split(strings.TrimSpace(out), "\n")), checker.Equals, 2)
+}

plugin/backend_linux.go

@@ -194,6 +194,18 @@ func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, opti
 		return err
 	}
 
+	// In case an error happens, remove the created directory.
+	if err := pm.createFromContext(ctx, pluginID, pluginDir, tarCtx, options); err != nil {
+		if err := os.RemoveAll(pluginDir); err != nil {
+			logrus.Warnf("unable to remove %q from failed plugin creation: %v", pluginDir, err)
+		}
+		return err
+	}
+
+	return nil
+}
+
+func (pm *Manager) createFromContext(ctx context.Context, pluginID, pluginDir string, tarCtx io.Reader, options *types.PluginCreateOptions) error {
 	if err := chrootarchive.Untar(tarCtx, pluginDir, nil); err != nil {
 		return err
 	}
@@ -211,7 +223,9 @@ func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, opti
 		return err
 	}
 
-	pm.pluginStore.Add(p)
+	if err := pm.pluginStore.Add(p); err != nil {
+		return err
+	}
 
 	pm.pluginEventLogger(p.GetID(), repoName, "create")
 

plugin/manager.go

@@ -124,7 +124,7 @@ func (pm *Manager) init() error {
 				return
 			}
 
-			pm.pluginStore.Add(p)
+			pm.pluginStore.Update(p)
 			requiresManualRestore := !pm.liveRestore && p.IsEnabled()
 
 			if requiresManualRestore {

plugin/store/store.go

@@ -98,12 +98,31 @@ func (ps *Store) SetState(p *v2.Plugin, state bool) {
 }
 
 // Add adds a plugin to memory and plugindb.
-func (ps *Store) Add(p *v2.Plugin) {
+// An error will be returned if there is a collision.
+func (ps *Store) Add(p *v2.Plugin) error {
 	ps.Lock()
+	defer ps.Unlock()
+
+	if v, exist := ps.plugins[p.GetID()]; exist {
+		return fmt.Errorf("plugin %q has the same ID %s as %q", p.Name(), p.GetID(), v.Name())
+	}
+	if _, exist := ps.nameToID[p.Name()]; exist {
+		return fmt.Errorf("plugin %q already exists", p.Name())
+	}
+	ps.plugins[p.GetID()] = p
+	ps.nameToID[p.Name()] = p.GetID()
+	ps.updatePluginDB()
+	return nil
+}
+
+// Update updates a plugin to memory and plugindb.
+func (ps *Store) Update(p *v2.Plugin) {
+	ps.Lock()
+	defer ps.Unlock()
+
 	ps.plugins[p.GetID()] = p
 	ps.nameToID[p.Name()] = p.GetID()
 	ps.updatePluginDB()
-	ps.Unlock()
 }
 
 // Remove removes a plugin from memory and plugindb.