Browse code
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>
(cherry picked from commit 662d456928e47162a3af5931356cb05b6a3f9918)
Signed-off-by: Victor Vieux <victorvieux@gmail.com>
Yong Tang authored on 2016/11/23 02:42:58Showing 4 changed files
- integration-cli/docker_cli_plugins_test.go
- plugin/backend_linux.go
- plugin/manager.go
- plugin/store/store.go
| ... | ... |
@@ -4,6 +4,7 @@ import ( |
| 4 | 4 |
"github.com/docker/docker/pkg/integration/checker" |
| 5 | 5 |
"github.com/go-check/check" |
| 6 | 6 |
|
| 7 |
+ "io/ioutil" |
|
| 7 | 8 |
"os" |
| 8 | 9 |
"path/filepath" |
| 9 | 10 |
"strings" |
| ... | ... |
@@ -169,3 +170,34 @@ func (s *DockerSuite) TestPluginEnableDisableNegative(c *check.C) {
|
| 169 | 169 |
_, _, err = dockerCmdWithError("plugin", "remove", pName)
|
| 170 | 170 |
c.Assert(err, checker.IsNil) |
| 171 | 171 |
} |
| 172 |
+ |
|
| 173 |
+func (s *DockerSuite) TestPluginCreate(c *check.C) {
|
|
| 174 |
+ testRequires(c, DaemonIsLinux, Network) |
|
| 175 |
+ |
|
| 176 |
+ name := "foo/bar-driver" |
|
| 177 |
+ temp, err := ioutil.TempDir("", "foo")
|
|
| 178 |
+ c.Assert(err, checker.IsNil) |
|
| 179 |
+ defer os.RemoveAll(temp) |
|
| 180 |
+ |
|
| 181 |
+ data := `{"description": "foo plugin"}`
|
|
| 182 |
+ err = ioutil.WriteFile(filepath.Join(temp, "config.json"), []byte(data), 0644) |
|
| 183 |
+ c.Assert(err, checker.IsNil) |
|
| 184 |
+ |
|
| 185 |
+ out, _, err := dockerCmdWithError("plugin", "create", name, temp)
|
|
| 186 |
+ c.Assert(err, checker.IsNil) |
|
| 187 |
+ c.Assert(out, checker.Contains, name) |
|
| 188 |
+ |
|
| 189 |
+ out, _, err = dockerCmdWithError("plugin", "ls")
|
|
| 190 |
+ c.Assert(err, checker.IsNil) |
|
| 191 |
+ c.Assert(out, checker.Contains, name) |
|
| 192 |
+ |
|
| 193 |
+ out, _, err = dockerCmdWithError("plugin", "create", name, temp)
|
|
| 194 |
+ c.Assert(err, checker.NotNil) |
|
| 195 |
+ c.Assert(out, checker.Contains, "already exist") |
|
| 196 |
+ |
|
| 197 |
+ out, _, err = dockerCmdWithError("plugin", "ls")
|
|
| 198 |
+ c.Assert(err, checker.IsNil) |
|
| 199 |
+ c.Assert(out, checker.Contains, name) |
|
| 200 |
+ // The output will consists of one HEADER line and one line of foo/bar-driver |
|
| 201 |
+ c.Assert(len(strings.Split(strings.TrimSpace(out), "\n")), checker.Equals, 2) |
|
| 202 |
+} |
| ... | ... |
@@ -194,6 +194,18 @@ func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, opti |
| 194 | 194 |
return err |
| 195 | 195 |
} |
| 196 | 196 |
|
| 197 |
+ // In case an error happens, remove the created directory. |
|
| 198 |
+ if err := pm.createFromContext(ctx, pluginID, pluginDir, tarCtx, options); err != nil {
|
|
| 199 |
+ if err := os.RemoveAll(pluginDir); err != nil {
|
|
| 200 |
+ logrus.Warnf("unable to remove %q from failed plugin creation: %v", pluginDir, err)
|
|
| 201 |
+ } |
|
| 202 |
+ return err |
|
| 203 |
+ } |
|
| 204 |
+ |
|
| 205 |
+ return nil |
|
| 206 |
+} |
|
| 207 |
+ |
|
| 208 |
+func (pm *Manager) createFromContext(ctx context.Context, pluginID, pluginDir string, tarCtx io.Reader, options *types.PluginCreateOptions) error {
|
|
| 197 | 209 |
if err := chrootarchive.Untar(tarCtx, pluginDir, nil); err != nil {
|
| 198 | 210 |
return err |
| 199 | 211 |
} |
| ... | ... |
@@ -211,7 +223,9 @@ func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, opti |
| 211 | 211 |
return err |
| 212 | 212 |
} |
| 213 | 213 |
|
| 214 |
- pm.pluginStore.Add(p) |
|
| 214 |
+ if err := pm.pluginStore.Add(p); err != nil {
|
|
| 215 |
+ return err |
|
| 216 |
+ } |
|
| 215 | 217 |
|
| 216 | 218 |
pm.pluginEventLogger(p.GetID(), repoName, "create") |
| 217 | 219 |
|
| ... | ... |
@@ -98,12 +98,31 @@ func (ps *Store) SetState(p *v2.Plugin, state bool) {
|
| 98 | 98 |
} |
| 99 | 99 |
|
| 100 | 100 |
// Add adds a plugin to memory and plugindb. |
| 101 |
-func (ps *Store) Add(p *v2.Plugin) {
|
|
| 101 |
+// An error will be returned if there is a collision. |
|
| 102 |
+func (ps *Store) Add(p *v2.Plugin) error {
|
|
| 102 | 103 |
ps.Lock() |
| 104 |
+ defer ps.Unlock() |
|
| 105 |
+ |
|
| 106 |
+ if v, exist := ps.plugins[p.GetID()]; exist {
|
|
| 107 |
+ return fmt.Errorf("plugin %q has the same ID %s as %q", p.Name(), p.GetID(), v.Name())
|
|
| 108 |
+ } |
|
| 109 |
+ if _, exist := ps.nameToID[p.Name()]; exist {
|
|
| 110 |
+ return fmt.Errorf("plugin %q already exists", p.Name())
|
|
| 111 |
+ } |
|
| 112 |
+ ps.plugins[p.GetID()] = p |
|
| 113 |
+ ps.nameToID[p.Name()] = p.GetID() |
|
| 114 |
+ ps.updatePluginDB() |
|
| 115 |
+ return nil |
|
| 116 |
+} |
|
| 117 |
+ |
|
| 118 |
+// Update updates a plugin to memory and plugindb. |
|
| 119 |
+func (ps *Store) Update(p *v2.Plugin) {
|
|
| 120 |
+ ps.Lock() |
|
| 121 |
+ defer ps.Unlock() |
|
| 122 |
+ |
|
| 103 | 123 |
ps.plugins[p.GetID()] = p |
| 104 | 124 |
ps.nameToID[p.Name()] = p.GetID() |
| 105 | 125 |
ps.updatePluginDB() |
| 106 |
- ps.Unlock() |
|
| 107 | 126 |
} |
| 108 | 127 |
|
| 109 | 128 |
// Remove removes a plugin from memory and plugindb. |