Browse Source

Fix daemon.Get error handling with --link setup. Add test.

Fixes newly introduced error flow from daemon.Get() in the link setup
code.  Also added more detail to daemon.Get() comments for clarity.
Added test to test the bogus container link codepath.

Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
Phil Estes 10 years ago
parent
commit
6217f2944f
2 changed files with 29 additions and 14 deletions
  1. 13 14
      daemon/daemon.go
  2. 16 0
      integration-cli/docker_cli_links_test.go

+ 13 - 14
daemon/daemon.go

@@ -155,31 +155,32 @@ func (daemon *Daemon) Install(eng *engine.Engine) error {
 	return nil
 }
 
-// Get looks for a container with the provided prefix
-func (daemon *Daemon) Get(prefix string) (*Container, error) {
-	if containerByID := daemon.containers.Get(prefix); containerByID != nil {
-
+// Get looks for a container using the provided information, which could be
+// one of the following inputs from the caller:
+//  - A full container ID, which will exact match a container in daemon's list
+//  - A container name, which will only exact match via the GetByName() function
+//  - A partial container ID prefix (e.g. short ID) of any length that is
+//    unique enough to only return a single container object
+//  If none of these searches succeed, an error is returned
+func (daemon *Daemon) Get(prefixOrName string) (*Container, error) {
+	if containerByID := daemon.containers.Get(prefixOrName); containerByID != nil {
 		// prefix is an exact match to a full container ID
 		return containerByID, nil
 	}
 
-	// Either GetByName finds an entity matching prefix exactly, or it doesn't.
-	// Check value of containerByName and ignore any errors
-	containerByName, _ := daemon.GetByName(prefix)
-	containerId, indexError := daemon.idIndex.Get(prefix)
+	// GetByName will match only an exact name provided; we ignore errors
+	containerByName, _ := daemon.GetByName(prefixOrName)
+	containerId, indexError := daemon.idIndex.Get(prefixOrName)
 
 	if containerByName != nil {
-
 		// prefix is an exact match to a full container Name
 		return containerByName, nil
 	}
 
 	if containerId != "" {
-
 		// prefix is a fuzzy match to a container ID
 		return daemon.containers.Get(containerId), nil
 	}
-
 	return nil, indexError
 }
 
@@ -767,9 +768,7 @@ func (daemon *Daemon) RegisterLinks(container *Container, hostConfig *runconfig.
 			}
 			child, err := daemon.Get(parts["name"])
 			if err != nil {
-				return err
-			}
-			if child == nil {
+				//An error from daemon.Get() means this name could not be found
 				return fmt.Errorf("Could not get container for %s", parts["name"])
 			}
 			if child.hostConfig.NetworkMode.IsHost() {

+ 16 - 0
integration-cli/docker_cli_links_test.go

@@ -64,6 +64,22 @@ func TestLinksPingUnlinkedContainers(t *testing.T) {
 	logDone("links - ping unlinked container")
 }
 
+// Test for appropriate error when calling --link with an invalid target container
+func TestLinksInvalidContainerTarget(t *testing.T) {
+	runCmd := exec.Command(dockerBinary, "run", "--link", "bogus:alias", "busybox", "true")
+	out, _, err := runCommandWithOutput(runCmd)
+
+	if err == nil {
+		t.Fatal("an invalid container target should produce an error")
+	}
+	if !strings.Contains(out, "Could not get container") {
+		t.Fatal("error output expected 'Could not get container', but got %q instead; err: %v", out, err)
+	}
+	deleteAllContainers()
+
+	logDone("links - linking to non-existent container should not work")
+}
+
 func TestLinksPingLinkedContainers(t *testing.T) {
 	var out string
 	out, _, _ = dockerCmd(t, "run", "-d", "--name", "container1", "busybox", "sleep", "10")