Browse code

Improve name generation on concurrent requests

Fixes #2586

This fixes a few races where the name generator asks if a name is free
but another container takes the name before it can be reserved. This
solves this by generating the name and setting it. If the set fails
with a non unique error then we try again.
Docker-DCO-1.1-Signed-off-by: Michael Crosby <michael@crosbymichael.com> (github: crosbymichael)

Michael Crosby authored on 2014/05/24 09:51:16
Showing 4 changed files
... ...
@@ -29,6 +29,7 @@ import (
29 29
 	"github.com/dotcloud/docker/pkg/graphdb"
30 30
 	"github.com/dotcloud/docker/pkg/label"
31 31
 	"github.com/dotcloud/docker/pkg/mount"
32
+	"github.com/dotcloud/docker/pkg/namesgenerator"
32 33
 	"github.com/dotcloud/docker/pkg/networkfs/resolvconf"
33 34
 	"github.com/dotcloud/docker/pkg/selinux"
34 35
 	"github.com/dotcloud/docker/pkg/sysinfo"
... ...
@@ -250,20 +251,15 @@ func (daemon *Daemon) register(container *Container, updateSuffixarray bool) err
250 250
 
251 251
 func (daemon *Daemon) ensureName(container *Container) error {
252 252
 	if container.Name == "" {
253
-		name, err := generateRandomName(daemon)
253
+		name, err := daemon.generateNewName(container.ID)
254 254
 		if err != nil {
255
-			name = utils.TruncateID(container.ID)
255
+			return err
256 256
 		}
257 257
 		container.Name = name
258 258
 
259 259
 		if err := container.ToDisk(); err != nil {
260 260
 			utils.Debugf("Error saving container name %s", err)
261 261
 		}
262
-		if !daemon.containerGraph.Exists(name) {
263
-			if _, err := daemon.containerGraph.Set(name, container.ID); err != nil {
264
-				utils.Debugf("Setting default id - %s", err)
265
-			}
266
-		}
267 262
 	}
268 263
 	return nil
269 264
 }
... ...
@@ -370,12 +366,8 @@ func (daemon *Daemon) restore() error {
370 370
 	// Any containers that are left over do not exist in the graph
371 371
 	for _, container := range containers {
372 372
 		// Try to set the default name for a container if it exists prior to links
373
-		container.Name, err = generateRandomName(daemon)
373
+		container.Name, err = daemon.generateNewName(container.ID)
374 374
 		if err != nil {
375
-			container.Name = utils.TruncateID(container.ID)
376
-		}
377
-
378
-		if _, err := daemon.containerGraph.Set(container.Name, container.ID); err != nil {
379 375
 			utils.Debugf("Setting default id - %s", err)
380 376
 		}
381 377
 		registerContainer(container)
... ...
@@ -470,42 +462,75 @@ func (daemon *Daemon) generateIdAndName(name string) (string, string, error) {
470 470
 	)
471 471
 
472 472
 	if name == "" {
473
-		name, err = generateRandomName(daemon)
474
-		if err != nil {
475
-			name = utils.TruncateID(id)
476
-		}
477
-	} else {
478
-		if !validContainerNamePattern.MatchString(name) {
479
-			return "", "", fmt.Errorf("Invalid container name (%s), only %s are allowed", name, validContainerNameChars)
473
+		if name, err = daemon.generateNewName(id); err != nil {
474
+			return "", "", err
480 475
 		}
476
+		return id, name, nil
481 477
 	}
478
+
479
+	if name, err = daemon.reserveName(id, name); err != nil {
480
+		return "", "", err
481
+	}
482
+
483
+	return id, name, nil
484
+}
485
+
486
+func (daemon *Daemon) reserveName(id, name string) (string, error) {
487
+	if !validContainerNamePattern.MatchString(name) {
488
+		return "", fmt.Errorf("Invalid container name (%s), only %s are allowed", name, validContainerNameChars)
489
+	}
490
+
482 491
 	if name[0] != '/' {
483 492
 		name = "/" + name
484 493
 	}
485
-	// Set the enitity in the graph using the default name specified
494
+
486 495
 	if _, err := daemon.containerGraph.Set(name, id); err != nil {
487 496
 		if !graphdb.IsNonUniqueNameError(err) {
488
-			return "", "", err
497
+			return "", err
489 498
 		}
490 499
 
491 500
 		conflictingContainer, err := daemon.GetByName(name)
492 501
 		if err != nil {
493 502
 			if strings.Contains(err.Error(), "Could not find entity") {
494
-				return "", "", err
503
+				return "", err
495 504
 			}
496 505
 
497 506
 			// Remove name and continue starting the container
498 507
 			if err := daemon.containerGraph.Delete(name); err != nil {
499
-				return "", "", err
508
+				return "", err
500 509
 			}
501 510
 		} else {
502 511
 			nameAsKnownByUser := strings.TrimPrefix(name, "/")
503
-			return "", "", fmt.Errorf(
512
+			return "", fmt.Errorf(
504 513
 				"Conflict, The name %s is already assigned to %s. You have to delete (or rename) that container to be able to assign %s to a container again.", nameAsKnownByUser,
505 514
 				utils.TruncateID(conflictingContainer.ID), nameAsKnownByUser)
506 515
 		}
507 516
 	}
508
-	return id, name, nil
517
+	return name, nil
518
+}
519
+
520
+func (daemon *Daemon) generateNewName(id string) (string, error) {
521
+	var name string
522
+	for i := 1; i < 6; i++ {
523
+		name = namesgenerator.GetRandomName(i)
524
+		if name[0] != '/' {
525
+			name = "/" + name
526
+		}
527
+
528
+		if _, err := daemon.containerGraph.Set(name, id); err != nil {
529
+			if !graphdb.IsNonUniqueNameError(err) {
530
+				return "", err
531
+			}
532
+			continue
533
+		}
534
+		return name, nil
535
+	}
536
+
537
+	name = "/" + utils.TruncateID(id)
538
+	if _, err := daemon.containerGraph.Set(name, id); err != nil {
539
+		return "", err
540
+	}
541
+	return name, nil
509 542
 }
510 543
 
511 544
 func (daemon *Daemon) generateHostname(id string, config *runconfig.Config) {
... ...
@@ -2,10 +2,10 @@ package daemon
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"strings"
6
+
5 7
 	"github.com/dotcloud/docker/nat"
6
-	"github.com/dotcloud/docker/pkg/namesgenerator"
7 8
 	"github.com/dotcloud/docker/runconfig"
8
-	"strings"
9 9
 )
10 10
 
11 11
 func migratePortMappings(config *runconfig.Config, hostConfig *runconfig.HostConfig) error {
... ...
@@ -49,16 +49,3 @@ func mergeLxcConfIntoOptions(hostConfig *runconfig.HostConfig, driverConfig map[
49 49
 		driverConfig["lxc"] = lxc
50 50
 	}
51 51
 }
52
-
53
-type checker struct {
54
-	daemon *Daemon
55
-}
56
-
57
-func (c *checker) Exists(name string) bool {
58
-	return c.daemon.containerGraph.Exists("/" + name)
59
-}
60
-
61
-// Generate a random and unique name
62
-func generateRandomName(daemon *Daemon) (string, error) {
63
-	return namesgenerator.GenerateRandomName(&checker{daemon})
64
-}
... ...
@@ -6,10 +6,6 @@ import (
6 6
 	"time"
7 7
 )
8 8
 
9
-type NameChecker interface {
10
-	Exists(name string) bool
11
-}
12
-
13 9
 var (
14 10
 	left = [...]string{"happy", "jolly", "dreamy", "sad", "angry", "pensive", "focused", "sleepy", "grave", "distracted", "determined", "stoic", "stupefied", "sharp", "agitated", "cocky", "tender", "goofy", "furious", "desperate", "hopeful", "compassionate", "silly", "lonely", "condescending", "naughty", "kickass", "drunk", "boring", "nostalgic", "ecstatic", "insane", "cranky", "mad", "jovial", "sick", "hungry", "thirsty", "elegant", "backstabbing", "clever", "trusting", "loving", "suspicious", "berserk", "high", "romantic", "prickly", "evil"}
15 11
 	// Docker 0.7.x generates names from notable scientists and hackers.
... ...
@@ -79,16 +75,17 @@ var (
79 79
 	right = [...]string{"lovelace", "franklin", "tesla", "einstein", "bohr", "davinci", "pasteur", "nobel", "curie", "darwin", "turing", "ritchie", "torvalds", "pike", "thompson", "wozniak", "galileo", "euclid", "newton", "fermat", "archimedes", "poincare", "heisenberg", "feynman", "hawking", "fermi", "pare", "mccarthy", "engelbart", "babbage", "albattani", "ptolemy", "bell", "wright", "lumiere", "morse", "mclean", "brown", "bardeen", "brattain", "shockley", "goldstine", "hoover", "hopper", "bartik", "sammet", "jones", "perlman", "wilson", "kowalevski", "hypatia", "goodall", "mayer", "elion", "blackwell", "lalande", "kirch", "ardinghelli", "colden", "almeida", "leakey", "meitner", "mestorf", "rosalind", "sinoussi", "carson", "mcclintock", "yonath"}
80 80
 )
81 81
 
82
-func GenerateRandomName(checker NameChecker) (string, error) {
83
-	retry := 5
82
+func GetRandomName(retry int) string {
84 83
 	rand.Seed(time.Now().UnixNano())
84
+
85
+begin:
85 86
 	name := fmt.Sprintf("%s_%s", left[rand.Intn(len(left))], right[rand.Intn(len(right))])
86
-	for checker != nil && checker.Exists(name) && retry > 0 || name == "boring_wozniak" /* Steve Wozniak is not boring */ {
87
-		name = fmt.Sprintf("%s%d", name, rand.Intn(10))
88
-		retry = retry - 1
87
+	if name == "boring_wozniak" /* Steve Wozniak is not boring */ {
88
+		goto begin
89 89
 	}
90
-	if retry == 0 {
91
-		return name, fmt.Errorf("Error generating random name")
90
+
91
+	if retry > 0 {
92
+		name = fmt.Sprintf("%s%d", name, rand.Intn(10))
92 93
 	}
93
-	return name, nil
94
+	return name
94 95
 }
... ...
@@ -4,35 +4,9 @@ import (
4 4
 	"testing"
5 5
 )
6 6
 
7
-type FalseChecker struct{}
8
-
9
-func (n *FalseChecker) Exists(name string) bool {
10
-	return false
11
-}
12
-
13
-type TrueChecker struct{}
14
-
15
-func (n *TrueChecker) Exists(name string) bool {
16
-	return true
17
-}
18
-
19
-func TestGenerateRandomName(t *testing.T) {
20
-	if _, err := GenerateRandomName(&FalseChecker{}); err != nil {
21
-		t.Error(err)
22
-	}
23
-
24
-	if _, err := GenerateRandomName(&TrueChecker{}); err == nil {
25
-		t.Error("An error was expected")
26
-	}
27
-
28
-}
29
-
30 7
 // Make sure the generated names are awesome
31 8
 func TestGenerateAwesomeNames(t *testing.T) {
32
-	name, err := GenerateRandomName(&FalseChecker{})
33
-	if err != nil {
34
-		t.Error(err)
35
-	}
9
+	name := GetRandomName(0)
36 10
 	if !isAwesome(name) {
37 11
 		t.Fatalf("Generated name '%s' is not awesome.", name)
38 12
 	}