Browse code

Merge pull request #28949 from yongtang/28717-docker-plugin-create

Use GetByName to check for collision before create any context in plugin creation

Anusha Ragunathan authored on 2016/12/10 02:12:08
Showing 2 changed files
... ...
@@ -311,15 +311,29 @@ func (pm *Manager) Set(name string, args []string) error {
311 311
 // CreateFromContext creates a plugin from the given pluginDir which contains
312 312
 // both the rootfs and the config.json and a repoName with optional tag.
313 313
 func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, options *types.PluginCreateOptions) error {
314
+	repoName := options.RepoName
315
+	ref, err := distribution.GetRef(repoName)
316
+	if err != nil {
317
+		return err
318
+	}
319
+
320
+	name := ref.Name()
321
+	tag := distribution.GetTag(ref)
314 322
 	pluginID := stringid.GenerateNonCryptoID()
315 323
 
324
+	p := v2.NewPlugin(name, pluginID, pm.runRoot, pm.libRoot, tag)
325
+
326
+	if v, _ := pm.pluginStore.GetByName(p.Name()); v != nil {
327
+		return fmt.Errorf("plugin %q already exists", p.Name())
328
+	}
329
+
316 330
 	pluginDir := filepath.Join(pm.libRoot, pluginID)
317 331
 	if err := os.MkdirAll(pluginDir, 0755); err != nil {
318 332
 		return err
319 333
 	}
320 334
 
321 335
 	// In case an error happens, remove the created directory.
322
-	if err := pm.createFromContext(ctx, pluginID, pluginDir, tarCtx, options); err != nil {
336
+	if err := pm.createFromContext(ctx, tarCtx, pluginDir, repoName, p); err != nil {
323 337
 		if err := os.RemoveAll(pluginDir); err != nil {
324 338
 			logrus.Warnf("unable to remove %q from failed plugin creation: %v", pluginDir, err)
325 339
 		}
... ...
@@ -329,20 +343,11 @@ func (pm *Manager) CreateFromContext(ctx context.Context, tarCtx io.Reader, opti
329 329
 	return nil
330 330
 }
331 331
 
332
-func (pm *Manager) createFromContext(ctx context.Context, pluginID, pluginDir string, tarCtx io.Reader, options *types.PluginCreateOptions) error {
332
+func (pm *Manager) createFromContext(ctx context.Context, tarCtx io.Reader, pluginDir, repoName string, p *v2.Plugin) error {
333 333
 	if err := chrootarchive.Untar(tarCtx, pluginDir, nil); err != nil {
334 334
 		return err
335 335
 	}
336 336
 
337
-	repoName := options.RepoName
338
-	ref, err := distribution.GetRef(repoName)
339
-	if err != nil {
340
-		return err
341
-	}
342
-	name := ref.Name()
343
-	tag := distribution.GetTag(ref)
344
-
345
-	p := v2.NewPlugin(name, pluginID, pm.runRoot, pm.libRoot, tag)
346 337
 	if err := p.InitPlugin(); err != nil {
347 338
 		return err
348 339
 	}
... ...
@@ -113,6 +113,13 @@ func (ps *Store) Add(p *v2.Plugin) error {
113 113
 	if v, exist := ps.plugins[p.GetID()]; exist {
114 114
 		return fmt.Errorf("plugin %q has the same ID %s as %q", p.Name(), p.GetID(), v.Name())
115 115
 	}
116
+	// Since both Pull() and CreateFromContext() calls GetByName() before any plugin
117
+	// to search for collision (to fail fast), it is unlikely the following check
118
+	// will return an error.
119
+	// However, in case two CreateFromContext() are called at the same time,
120
+	// there is still a remote possibility that a collision might happen.
121
+	// For that reason we still perform the collision check below as it is protected
122
+	// by ps.Lock() and ps.Unlock() above.
116 123
 	if _, exist := ps.nameToID[p.Name()]; exist {
117 124
 		return fmt.Errorf("plugin %q already exists", p.Name())
118 125
 	}