From 6217f2944f7e9aea4a81615102b7c68740db9350 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Fri, 13 Feb 2015 15:14:38 -0500 Subject: [PATCH] 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 (github: estesp) --- daemon/daemon.go | 27 ++++++++++++------------ integration-cli/docker_cli_links_test.go | 16 ++++++++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 54a686a9e9..e23193e2b8 100644 --- a/daemon/daemon.go +++ b/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() { diff --git a/integration-cli/docker_cli_links_test.go b/integration-cli/docker_cli_links_test.go index 542adc379b..87e89be227 100644 --- a/integration-cli/docker_cli_links_test.go +++ b/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")