Merge pull request #9705 from acbodine/9311-truncindex-error-duplicate-id-on-ambiguous-id

Closes #9311 Handles container id/name collisions against daemon functionalities according to #8069

api/client/commands.go

@@ -863,6 +863,12 @@ func (cli *DockerCli) CmdInspect(args ...string) error {
 	for _, name := range cmd.Args() {
 		obj, _, err := readBody(cli.call("GET", "/containers/"+name+"/json", nil, false))
 		if err != nil {
+			if strings.Contains(err.Error(), "Too many") {
+				fmt.Fprintf(cli.err, "Error: %s", err.Error())
+				status = 1
+				continue
+			}
+
 			obj, _, err = readBody(cli.call("GET", "/images/"+name+"/json", nil, false))
 			if err != nil {
 				if strings.Contains(err.Error(), "No such") {

api/server/server.go

@@ -1114,7 +1114,7 @@ func postContainersCopy(eng *engine.Engine, version version.Version, w http.Resp
 	w.Header().Set("Content-Type", "application/x-tar")
 	if err := job.Run(); err != nil {
 		log.Errorf("%s", err.Error())
-		if strings.Contains(strings.ToLower(err.Error()), "no such container") {
+		if strings.Contains(strings.ToLower(err.Error()), "no such id") {
 			w.WriteHeader(http.StatusNotFound)
 		} else if strings.Contains(err.Error(), "no such file or directory") {
 			return fmt.Errorf("Could not find the file %s in container %s", origResource, vars["name"])

builder/internals.go

@@ -87,9 +87,9 @@ func (b *Builder) commit(id string, autoCmd []string, comment string) error {
 		}
 		defer container.Unmount()
 	}
-	container := b.Daemon.Get(id)
-	if container == nil {
-		return fmt.Errorf("An error occured while creating the container")
+	container, err := b.Daemon.Get(id)
+	if err != nil {
+		return err
 	}
 
 	// Note: Actually copy the struct
@@ -710,7 +710,11 @@ func fixPermissions(source, destination string, uid, gid int, destExisted bool)
 
 func (b *Builder) clearTmp() {
 	for c := range b.TmpContainers {
-		tmp := b.Daemon.Get(c)
+		tmp, err := b.Daemon.Get(c)
+		if err != nil {
+			fmt.Fprint(b.OutStream, err.Error())
+		}
+
 		if err := b.Daemon.Destroy(tmp); err != nil {
 			fmt.Fprintf(b.OutStream, "Error removing intermediate container %s: %s\n", utils.TruncateID(c), err.Error())
 			return

daemon/attach.go

@@ -28,9 +28,9 @@ func (daemon *Daemon) ContainerAttach(job *engine.Job) engine.Status {
 		stderr = job.GetenvBool("stderr")
 	)
 
-	container := daemon.Get(name)
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 
 	//logs

daemon/changes.go

@@ -9,24 +9,29 @@ func (daemon *Daemon) ContainerChanges(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s CONTAINER", job.Name)
 	}
 	name := job.Args[0]
-	if container := daemon.Get(name); container != nil {
-		outs := engine.NewTable("", 0)
-		changes, err := container.Changes()
-		if err != nil {
-			return job.Error(err)
-		}
-		for _, change := range changes {
-			out := &engine.Env{}
-			if err := out.Import(change); err != nil {
-				return job.Error(err)
-			}
-			outs.Add(out)
-		}
-		if _, err := outs.WriteListTo(job.Stdout); err != nil {
+
+	container, error := daemon.Get(name)
+	if error != nil {
+		return job.Error(error)
+	}
+
+	outs := engine.NewTable("", 0)
+	changes, err := container.Changes()
+	if err != nil {
+		return job.Error(err)
+	}
+
+	for _, change := range changes {
+		out := &engine.Env{}
+		if err := out.Import(change); err != nil {
 			return job.Error(err)
 		}
-	} else {
-		return job.Errorf("No such container: %s", name)
+		outs.Add(out)
 	}
+
+	if _, err := outs.WriteListTo(job.Stdout); err != nil {
+		return job.Error(err)
+	}
+
 	return engine.StatusOK
 }

daemon/commit.go

@@ -12,9 +12,9 @@ func (daemon *Daemon) ContainerCommit(job *engine.Job) engine.Status {
 	}
 	name := job.Args[0]
 
-	container := daemon.Get(name)
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 
 	var (

daemon/container.go

@@ -1126,7 +1126,12 @@ func (container *Container) updateParentsHosts() error {
 		if ref.ParentID == "0" {
 			continue
 		}
-		c := container.daemon.Get(ref.ParentID)
+
+		c, err := container.daemon.Get(ref.ParentID)
+		if err != nil {
+			log.Error(err)
+		}
+
 		if c != nil && !container.daemon.config.DisableNetwork && container.hostConfig.NetworkMode.IsPrivate() {
 			log.Debugf("Update /etc/hosts of %s for alias %s with ip %s", c.ID, ref.Name, container.NetworkSettings.IPAddress)
 			if err := etchosts.Update(c.HostsPath, container.NetworkSettings.IPAddress, ref.Name); err != nil {
@@ -1395,9 +1400,9 @@ func (container *Container) GetMountLabel() string {
 
 func (container *Container) getIpcContainer() (*Container, error) {
 	containerID := container.hostConfig.IpcMode.Container()
-	c := container.daemon.Get(containerID)
-	if c == nil {
-		return nil, fmt.Errorf("no such container to join IPC: %s", containerID)
+	c, err := container.daemon.Get(containerID)
+	if err != nil {
+		return nil, err
 	}
 	if !c.IsRunning() {
 		return nil, fmt.Errorf("cannot join IPC of a non running container: %s", containerID)
@@ -1412,9 +1417,9 @@ func (container *Container) getNetworkedContainer() (*Container, error) {
 		if len(parts) != 2 {
 			return nil, fmt.Errorf("no container specified to join network")
 		}
-		nc := container.daemon.Get(parts[1])
-		if nc == nil {
-			return nil, fmt.Errorf("no such container to join network: %s", parts[1])
+		nc, err := container.daemon.Get(parts[1])
+		if err != nil {
+			return nil, err
 		}
 		if !nc.IsRunning() {
 			return nil, fmt.Errorf("cannot join network of a non running container: %s", parts[1])

daemon/copy.go

@@ -16,18 +16,19 @@ func (daemon *Daemon) ContainerCopy(job *engine.Job) engine.Status {
 		resource = job.Args[1]
 	)
 
-	if container := daemon.Get(name); container != nil {
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
+	}
 
-		data, err := container.Copy(resource)
-		if err != nil {
-			return job.Error(err)
-		}
-		defer data.Close()
+	data, err := container.Copy(resource)
+	if err != nil {
+		return job.Error(err)
+	}
+	defer data.Close()
 
-		if _, err := io.Copy(job.Stdout, data); err != nil {
-			return job.Error(err)
-		}
-		return engine.StatusOK
+	if _, err := io.Copy(job.Stdout, data); err != nil {
+		return job.Error(err)
 	}
-	return job.Errorf("No such container: %s", name)
+	return engine.StatusOK
 }

daemon/create.go

@@ -132,9 +132,9 @@ func (daemon *Daemon) GenerateSecurityOpt(ipcMode runconfig.IpcMode, pidMode run
 		return label.DisableSecOpt(), nil
 	}
 	if ipcContainer := ipcMode.Container(); ipcContainer != "" {
-		c := daemon.Get(ipcContainer)
-		if c == nil {
-			return nil, fmt.Errorf("no such container to join IPC: %s", ipcContainer)
+		c, err := daemon.Get(ipcContainer)
+		if err != nil {
+			return nil, err
 		}
 		if !c.IsRunning() {
 			return nil, fmt.Errorf("cannot join IPC of a non running container: %s", ipcContainer)

daemon/daemon.go

@@ -155,28 +155,39 @@ func (daemon *Daemon) Install(eng *engine.Engine) error {
 	return nil
 }
 
-// Get looks for a container by the specified ID or name, and returns it.
-// If the container is not found, or if an error occurs, nil is returned.
-func (daemon *Daemon) Get(name string) *Container {
-	id, err := daemon.idIndex.Get(name)
-	if err == nil {
-		return daemon.containers.Get(id)
+// 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 {
+
+		// prefix is an exact match to a full container ID
+		return containerByID, nil
 	}
 
-	if c, _ := daemon.GetByName(name); c != nil {
-		return c
+	// 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)
+
+	if containerByName != nil {
+
+		// prefix is an exact match to a full container Name
+		return containerByName, nil
 	}
 
-	if err == truncindex.ErrDuplicateID {
-		log.Errorf("Short ID %s is ambiguous: please retry with more characters or use the full ID.\n", name)
+	if containerId != "" {
+
+		// prefix is a fuzzy match to a container ID
+		return daemon.containers.Get(containerId), nil
 	}
-	return nil
+
+	return nil, indexError
 }
 
 // Exists returns a true if a container of the specified ID or name exists,
 // false otherwise.
 func (daemon *Daemon) Exists(id string) bool {
-	return daemon.Get(id) != nil
+	c, _ := daemon.Get(id)
+	return c != nil
 }
 
 func (daemon *Daemon) containerRoot(id string) string {
@@ -715,9 +726,9 @@ func (daemon *Daemon) Children(name string) (map[string]*Container, error) {
 	children := make(map[string]*Container)
 
 	err = daemon.containerGraph.Walk(name, func(p string, e *graphdb.Entity) error {
-		c := daemon.Get(e.ID())
-		if c == nil {
-			return fmt.Errorf("Could not get container for name %s and id %s", e.ID(), p)
+		c, err := daemon.Get(e.ID())
+		if err != nil {
+			return err
 		}
 		children[p] = c
 		return nil
@@ -754,7 +765,10 @@ func (daemon *Daemon) RegisterLinks(container *Container, hostConfig *runconfig.
 			if err != nil {
 				return err
 			}
-			child := daemon.Get(parts["name"])
+			child, err := daemon.Get(parts["name"])
+			if err != nil {
+				return err
+			}
 			if child == nil {
 				return fmt.Errorf("Could not get container for %s", parts["name"])
 			}
@@ -1100,18 +1114,18 @@ func (daemon *Daemon) Stats(c *Container) (*execdriver.ResourceStats, error) {
 }
 
 func (daemon *Daemon) SubscribeToContainerStats(name string) (chan interface{}, error) {
-	c := daemon.Get(name)
-	if c == nil {
-		return nil, fmt.Errorf("no such container")
+	c, err := daemon.Get(name)
+	if err != nil {
+		return nil, err
 	}
 	ch := daemon.statsCollector.collect(c)
 	return ch, nil
 }
 
 func (daemon *Daemon) UnsubscribeToContainerStats(name string, ch chan interface{}) error {
-	c := daemon.Get(name)
-	if c == nil {
-		return fmt.Errorf("no such container")
+	c, err := daemon.Get(name)
+	if err != nil {
+		return err
 	}
 	daemon.statsCollector.unsubscribe(c, ch)
 	return nil

daemon/daemon_test.go

@@ -0,0 +1,101 @@
+package daemon
+
+import (
+	"github.com/docker/docker/pkg/graphdb"
+	"github.com/docker/docker/pkg/truncindex"
+	"os"
+	"path"
+	"testing"
+)
+
+//
+// https://github.com/docker/docker/issues/8069
+//
+
+func TestGet(t *testing.T) {
+	c1 := &Container{
+		ID:   "5a4ff6a163ad4533d22d69a2b8960bf7fafdcba06e72d2febdba229008b0bf57",
+		Name: "tender_bardeen",
+	}
+	c2 := &Container{
+		ID:   "3cdbd1aa394fd68559fd1441d6eff2ab7c1e6363582c82febfaa8045df3bd8de",
+		Name: "drunk_hawking",
+	}
+	c3 := &Container{
+		ID:   "3cdbd1aa394fd68559fd1441d6eff2abfafdcba06e72d2febdba229008b0bf57",
+		Name: "3cdbd1aa",
+	}
+	c4 := &Container{
+		ID:   "75fb0b800922abdbef2d27e60abcdfaf7fb0698b2a96d22d3354da361a6ff4a5",
+		Name: "5a4ff6a163ad4533d22d69a2b8960bf7fafdcba06e72d2febdba229008b0bf57",
+	}
+	c5 := &Container{
+		ID:   "d22d69a2b8960bf7fafdcba06e72d2febdba960bf7fafdcba06e72d2f9008b060b",
+		Name: "d22d69a2b896",
+	}
+
+	store := &contStore{
+		s: map[string]*Container{
+			c1.ID: c1,
+			c2.ID: c2,
+			c3.ID: c3,
+			c4.ID: c4,
+			c5.ID: c5,
+		},
+	}
+
+	index := truncindex.NewTruncIndex([]string{})
+	index.Add(c1.ID)
+	index.Add(c2.ID)
+	index.Add(c3.ID)
+	index.Add(c4.ID)
+	index.Add(c5.ID)
+
+	daemonTestDbPath := path.Join(os.TempDir(), "daemon_test.db")
+	graph, err := graphdb.NewSqliteConn(daemonTestDbPath)
+	if err != nil {
+		t.Fatalf("Failed to create daemon test sqlite database at %s", daemonTestDbPath)
+	}
+	graph.Set(c1.Name, c1.ID)
+	graph.Set(c2.Name, c2.ID)
+	graph.Set(c3.Name, c3.ID)
+	graph.Set(c4.Name, c4.ID)
+	graph.Set(c5.Name, c5.ID)
+
+	daemon := &Daemon{
+		containers:     store,
+		idIndex:        index,
+		containerGraph: graph,
+	}
+
+	if container, _ := daemon.Get("3cdbd1aa394fd68559fd1441d6eff2ab7c1e6363582c82febfaa8045df3bd8de"); container != c2 {
+		t.Fatal("Should explicitly match full container IDs")
+	}
+
+	if container, _ := daemon.Get("75fb0b8009"); container != c4 {
+		t.Fatal("Should match a partial ID")
+	}
+
+	if container, _ := daemon.Get("drunk_hawking"); container != c2 {
+		t.Fatal("Should match a full name")
+	}
+
+	// c3.Name is a partial match for both c3.ID and c2.ID
+	if c, _ := daemon.Get("3cdbd1aa"); c != c3 {
+		t.Fatal("Should match a full name even though it collides with another container's ID")
+	}
+
+	if container, _ := daemon.Get("d22d69a2b896"); container != c5 {
+		t.Fatal("Should match a container where the provided prefix is an exact match to the it's name, and is also a prefix for it's ID")
+	}
+
+	if _, err := daemon.Get("3cdbd1"); err == nil {
+		t.Fatal("Should return an error when provided a prefix that partially matches multiple container ID's")
+	}
+
+	if _, err := daemon.Get("nothing"); err == nil {
+		t.Fatal("Should return an error when provided a prefix that is neither a name or a partial match to an ID")
+	}
+
+	os.Remove(daemonTestDbPath)
+}

daemon/delete.go

@@ -17,10 +17,10 @@ func (daemon *Daemon) ContainerRm(job *engine.Job) engine.Status {
 	removeVolume := job.GetenvBool("removeVolume")
 	removeLink := job.GetenvBool("removeLink")
 	forceRemove := job.GetenvBool("forceRemove")
-	container := daemon.Get(name)
 
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 
 	if removeLink {
@@ -36,7 +36,7 @@ func (daemon *Daemon) ContainerRm(job *engine.Job) engine.Status {
 		if pe == nil {
 			return job.Errorf("Cannot get parent %s for name %s", parent, name)
 		}
-		parentContainer := daemon.Get(pe.ID())
+		parentContainer, _ := daemon.Get(pe.ID())
 
 		if parentContainer != nil {
 			parentContainer.DisableLink(n)

daemon/exec.go

@@ -97,10 +97,9 @@ func (d *Daemon) unregisterExecCommand(execConfig *execConfig) {
 }
 
 func (d *Daemon) getActiveContainer(name string) (*Container, error) {
-	container := d.Get(name)
-
-	if container == nil {
-		return nil, fmt.Errorf("No such container: %s", name)
+	container, err := d.Get(name)
+	if err != nil {
+		return nil, err
 	}
 
 	if !container.IsRunning() {

daemon/export.go

@@ -11,20 +11,23 @@ func (daemon *Daemon) ContainerExport(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s container_id", job.Name)
 	}
 	name := job.Args[0]
-	if container := daemon.Get(name); container != nil {
-		data, err := container.Export()
-		if err != nil {
-			return job.Errorf("%s: %s", name, err)
-		}
-		defer data.Close()
 
-		// Stream the entire contents of the container (basically a volatile snapshot)
-		if _, err := io.Copy(job.Stdout, data); err != nil {
-			return job.Errorf("%s: %s", name, err)
-		}
-		// FIXME: factor job-specific LogEvent to engine.Job.Run()
-		container.LogEvent("export")
-		return engine.StatusOK
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
-	return job.Errorf("No such container: %s", name)
+
+	data, err := container.Export()
+	if err != nil {
+		return job.Errorf("%s: %s", name, err)
+	}
+	defer data.Close()
+
+	// Stream the entire contents of the container (basically a volatile snapshot)
+	if _, err := io.Copy(job.Stdout, data); err != nil {
+		return job.Errorf("%s: %s", name, err)
+	}
+	// FIXME: factor job-specific LogEvent to engine.Job.Run()
+	container.LogEvent("export")
+	return engine.StatusOK
 }

daemon/inspect.go

@@ -13,60 +13,62 @@ func (daemon *Daemon) ContainerInspect(job *engine.Job) engine.Status {
 		return job.Errorf("usage: %s NAME", job.Name)
 	}
 	name := job.Args[0]
-	if container := daemon.Get(name); container != nil {
-		container.Lock()
-		defer container.Unlock()
-		if job.GetenvBool("raw") {
-			b, err := json.Marshal(&struct {
-				*Container
-				HostConfig *runconfig.HostConfig
-			}{container, container.hostConfig})
-			if err != nil {
-				return job.Error(err)
-			}
-			job.Stdout.Write(b)
-			return engine.StatusOK
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
+	}
+
+	container.Lock()
+	defer container.Unlock()
+	if job.GetenvBool("raw") {
+		b, err := json.Marshal(&struct {
+			*Container
+			HostConfig *runconfig.HostConfig
+		}{container, container.hostConfig})
+		if err != nil {
+			return job.Error(err)
 		}
+		job.Stdout.Write(b)
+		return engine.StatusOK
+	}
 
-		out := &engine.Env{}
-		out.SetJson("Id", container.ID)
-		out.SetAuto("Created", container.Created)
-		out.SetJson("Path", container.Path)
-		out.SetList("Args", container.Args)
-		out.SetJson("Config", container.Config)
-		out.SetJson("State", container.State)
-		out.Set("Image", container.ImageID)
-		out.SetJson("NetworkSettings", container.NetworkSettings)
-		out.Set("ResolvConfPath", container.ResolvConfPath)
-		out.Set("HostnamePath", container.HostnamePath)
-		out.Set("HostsPath", container.HostsPath)
-		out.SetJson("Name", container.Name)
-		out.SetInt("RestartCount", container.RestartCount)
-		out.Set("Driver", container.Driver)
-		out.Set("ExecDriver", container.ExecDriver)
-		out.Set("MountLabel", container.MountLabel)
-		out.Set("ProcessLabel", container.ProcessLabel)
-		out.SetJson("Volumes", container.Volumes)
-		out.SetJson("VolumesRW", container.VolumesRW)
-		out.SetJson("AppArmorProfile", container.AppArmorProfile)
+	out := &engine.Env{}
+	out.SetJson("Id", container.ID)
+	out.SetAuto("Created", container.Created)
+	out.SetJson("Path", container.Path)
+	out.SetList("Args", container.Args)
+	out.SetJson("Config", container.Config)
+	out.SetJson("State", container.State)
+	out.Set("Image", container.ImageID)
+	out.SetJson("NetworkSettings", container.NetworkSettings)
+	out.Set("ResolvConfPath", container.ResolvConfPath)
+	out.Set("HostnamePath", container.HostnamePath)
+	out.Set("HostsPath", container.HostsPath)
+	out.SetJson("Name", container.Name)
+	out.SetInt("RestartCount", container.RestartCount)
+	out.Set("Driver", container.Driver)
+	out.Set("ExecDriver", container.ExecDriver)
+	out.Set("MountLabel", container.MountLabel)
+	out.Set("ProcessLabel", container.ProcessLabel)
+	out.SetJson("Volumes", container.Volumes)
+	out.SetJson("VolumesRW", container.VolumesRW)
+	out.SetJson("AppArmorProfile", container.AppArmorProfile)
 
-		out.SetList("ExecIDs", container.GetExecIDs())
+	out.SetList("ExecIDs", container.GetExecIDs())
 
-		if children, err := daemon.Children(container.Name); err == nil {
-			for linkAlias, child := range children {
-				container.hostConfig.Links = append(container.hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
-			}
+	if children, err := daemon.Children(container.Name); err == nil {
+		for linkAlias, child := range children {
+			container.hostConfig.Links = append(container.hostConfig.Links, fmt.Sprintf("%s:%s", child.Name, linkAlias))
 		}
+	}
 
-		out.SetJson("HostConfig", container.hostConfig)
+	out.SetJson("HostConfig", container.hostConfig)
 
-		container.hostConfig.Links = nil
-		if _, err := out.WriteTo(job.Stdout); err != nil {
-			return job.Error(err)
-		}
-		return engine.StatusOK
+	container.hostConfig.Links = nil
+	if _, err := out.WriteTo(job.Stdout); err != nil {
+		return job.Error(err)
 	}
-	return job.Errorf("No such container: %s", name)
+	return engine.StatusOK
 }
 
 func (daemon *Daemon) ContainerExecInspect(job *engine.Job) engine.Status {

daemon/kill.go

@@ -38,22 +38,23 @@ func (daemon *Daemon) ContainerKill(job *engine.Job) engine.Status {
 		}
 	}
 
-	if container := daemon.Get(name); container != nil {
-		// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
-		if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
-			if err := container.Kill(); err != nil {
-				return job.Errorf("Cannot kill container %s: %s", name, err)
-			}
-			container.LogEvent("kill")
-		} else {
-			// Otherwise, just send the requested signal
-			if err := container.KillSig(int(sig)); err != nil {
-				return job.Errorf("Cannot kill container %s: %s", name, err)
-			}
-			// FIXME: Add event for signals
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
+	}
+
+	// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
+	if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
+		if err := container.Kill(); err != nil {
+			return job.Errorf("Cannot kill container %s: %s", name, err)
 		}
+		container.LogEvent("kill")
 	} else {
-		return job.Errorf("No such container: %s", name)
+		// Otherwise, just send the requested signal
+		if err := container.KillSig(int(sig)); err != nil {
+			return job.Errorf("Cannot kill container %s: %s", name, err)
+		}
+		// FIXME: Add event for signals
 	}
 	return engine.StatusOK
 }

daemon/list.go

@@ -63,16 +63,16 @@ func (daemon *Daemon) Containers(job *engine.Job) engine.Status {
 
 	var beforeCont, sinceCont *Container
 	if before != "" {
-		beforeCont = daemon.Get(before)
-		if beforeCont == nil {
-			return job.Error(fmt.Errorf("Could not find container with name or id %s", before))
+		beforeCont, err = daemon.Get(before)
+		if err != nil {
+			return job.Error(err)
 		}
 	}
 
 	if since != "" {
-		sinceCont = daemon.Get(since)
-		if sinceCont == nil {
-			return job.Error(fmt.Errorf("Could not find container with name or id %s", since))
+		sinceCont, err = daemon.Get(since)
+		if err != nil {
+			return job.Error(err)
 		}
 	}
 

daemon/logs.go

@@ -40,9 +40,9 @@ func (daemon *Daemon) ContainerLogs(job *engine.Job) engine.Status {
 	if tail == "" {
 		tail = "all"
 	}
-	container := daemon.Get(name)
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 	cLog, err := container.ReadLog("json")
 	if err != nil && os.IsNotExist(err) {

daemon/pause.go

@@ -9,9 +9,9 @@ func (daemon *Daemon) ContainerPause(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s CONTAINER", job.Name)
 	}
 	name := job.Args[0]
-	container := daemon.Get(name)
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 	if err := container.Pause(); err != nil {
 		return job.Errorf("Cannot pause container %s: %s", name, err)
@@ -25,9 +25,9 @@ func (daemon *Daemon) ContainerUnpause(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s CONTAINER", job.Name)
 	}
 	name := job.Args[0]
-	container := daemon.Get(name)
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 	if err := container.Unpause(); err != nil {
 		return job.Errorf("Cannot unpause container %s: %s", name, err)

daemon/rename.go

@@ -11,9 +11,9 @@ func (daemon *Daemon) ContainerRename(job *engine.Job) engine.Status {
 	oldName := job.Args[0]
 	newName := job.Args[1]
 
-	container := daemon.Get(oldName)
-	if container == nil {
-		return job.Errorf("No such container: %s", oldName)
+	container, err := daemon.Get(oldName)
+	if err != nil {
+		return job.Error(err)
 	}
 
 	oldName = container.Name

daemon/resize.go

@@ -19,14 +19,14 @@ func (daemon *Daemon) ContainerResize(job *engine.Job) engine.Status {
 	if err != nil {
 		return job.Error(err)
 	}
-
-	if container := daemon.Get(name); container != nil {
-		if err := container.Resize(height, width); err != nil {
-			return job.Error(err)
-		}
-		return engine.StatusOK
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
-	return job.Errorf("No such container: %s", name)
+	if err := container.Resize(height, width); err != nil {
+		return job.Error(err)
+	}
+	return engine.StatusOK
 }
 
 func (daemon *Daemon) ContainerExecResize(job *engine.Job) engine.Status {

daemon/restart.go

@@ -15,13 +15,13 @@ func (daemon *Daemon) ContainerRestart(job *engine.Job) engine.Status {
 	if job.EnvExists("t") {
 		t = job.GetenvInt("t")
 	}
-	if container := daemon.Get(name); container != nil {
-		if err := container.Restart(int(t)); err != nil {
-			return job.Errorf("Cannot restart container %s: %s\n", name, err)
-		}
-		container.LogEvent("restart")
-	} else {
-		return job.Errorf("No such container: %s\n", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
+	if err := container.Restart(int(t)); err != nil {
+		return job.Errorf("Cannot restart container %s: %s\n", name, err)
+	}
+	container.LogEvent("restart")
 	return engine.StatusOK
 }

daemon/start.go

@@ -14,12 +14,12 @@ func (daemon *Daemon) ContainerStart(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s container_id", job.Name)
 	}
 	var (
-		name      = job.Args[0]
-		container = daemon.Get(name)
+		name = job.Args[0]
 	)
 
-	if container == nil {
-		return job.Errorf("No such container: %s", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
 
 	if container.IsPaused() {

daemon/stop.go

@@ -15,16 +15,16 @@ func (daemon *Daemon) ContainerStop(job *engine.Job) engine.Status {
 	if job.EnvExists("t") {
 		t = job.GetenvInt("t")
 	}
-	if container := daemon.Get(name); container != nil {
-		if !container.IsRunning() {
-			return job.Errorf("Container already stopped")
-		}
-		if err := container.Stop(int(t)); err != nil {
-			return job.Errorf("Cannot stop container %s: %s\n", name, err)
-		}
-		container.LogEvent("stop")
-	} else {
-		return job.Errorf("No such container: %s\n", name)
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
 	}
+	if !container.IsRunning() {
+		return job.Errorf("Container already stopped")
+	}
+	if err := container.Stop(int(t)); err != nil {
+		return job.Errorf("Cannot stop container %s: %s\n", name, err)
+	}
+	container.LogEvent("stop")
 	return engine.StatusOK
 }

daemon/top.go

@@ -21,59 +21,59 @@ func (daemon *Daemon) ContainerTop(job *engine.Job) engine.Status {
 		psArgs = job.Args[1]
 	}
 
-	if container := daemon.Get(name); container != nil {
-		if !container.IsRunning() {
-			return job.Errorf("Container %s is not running", name)
-		}
-		pids, err := daemon.ExecutionDriver().GetPidsForContainer(container.ID)
-		if err != nil {
-			return job.Error(err)
-		}
-		output, err := exec.Command("ps", strings.Split(psArgs, " ")...).Output()
-		if err != nil {
-			return job.Errorf("Error running ps: %s", err)
-		}
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Error(err)
+	}
+	if !container.IsRunning() {
+		return job.Errorf("Container %s is not running", name)
+	}
+	pids, err := daemon.ExecutionDriver().GetPidsForContainer(container.ID)
+	if err != nil {
+		return job.Error(err)
+	}
+	output, err := exec.Command("ps", strings.Split(psArgs, " ")...).Output()
+	if err != nil {
+		return job.Errorf("Error running ps: %s", err)
+	}
 
-		lines := strings.Split(string(output), "\n")
-		header := strings.Fields(lines[0])
-		out := &engine.Env{}
-		out.SetList("Titles", header)
+	lines := strings.Split(string(output), "\n")
+	header := strings.Fields(lines[0])
+	out := &engine.Env{}
+	out.SetList("Titles", header)
 
-		pidIndex := -1
-		for i, name := range header {
-			if name == "PID" {
-				pidIndex = i
-			}
-		}
-		if pidIndex == -1 {
-			return job.Errorf("Couldn't find PID field in ps output")
+	pidIndex := -1
+	for i, name := range header {
+		if name == "PID" {
+			pidIndex = i
 		}
+	}
+	if pidIndex == -1 {
+		return job.Errorf("Couldn't find PID field in ps output")
+	}
 
-		processes := [][]string{}
-		for _, line := range lines[1:] {
-			if len(line) == 0 {
-				continue
-			}
-			fields := strings.Fields(line)
-			p, err := strconv.Atoi(fields[pidIndex])
-			if err != nil {
-				return job.Errorf("Unexpected pid '%s': %s", fields[pidIndex], err)
-			}
+	processes := [][]string{}
+	for _, line := range lines[1:] {
+		if len(line) == 0 {
+			continue
+		}
+		fields := strings.Fields(line)
+		p, err := strconv.Atoi(fields[pidIndex])
+		if err != nil {
+			return job.Errorf("Unexpected pid '%s': %s", fields[pidIndex], err)
+		}
 
-			for _, pid := range pids {
-				if pid == p {
-					// Make sure number of fields equals number of header titles
-					// merging "overhanging" fields
-					process := fields[:len(header)-1]
-					process = append(process, strings.Join(fields[len(header)-1:], " "))
-					processes = append(processes, process)
-				}
+		for _, pid := range pids {
+			if pid == p {
+				// Make sure number of fields equals number of header titles
+				// merging "overhanging" fields
+				process := fields[:len(header)-1]
+				process = append(process, strings.Join(fields[len(header)-1:], " "))
+				processes = append(processes, process)
 			}
 		}
-		out.SetJson("Processes", processes)
-		out.WriteTo(job.Stdout)
-		return engine.StatusOK
-
 	}
-	return job.Errorf("No such container: %s", name)
+	out.SetJson("Processes", processes)
+	out.WriteTo(job.Stdout)
+	return engine.StatusOK
 }

daemon/volumes.go

@@ -266,9 +266,9 @@ func (container *Container) applyVolumesFrom() error {
 			continue
 		}
 
-		c := container.daemon.Get(id)
-		if c == nil {
-			return fmt.Errorf("container %s not found, impossible to mount its volumes", id)
+		c, err := container.daemon.Get(id)
+		if err != nil {
+			return err
 		}
 
 		var (

daemon/wait.go

@@ -11,10 +11,11 @@ func (daemon *Daemon) ContainerWait(job *engine.Job) engine.Status {
 		return job.Errorf("Usage: %s", job.Name)
 	}
 	name := job.Args[0]
-	if container := daemon.Get(name); container != nil {
-		status, _ := container.WaitStop(-1 * time.Second)
-		job.Printf("%d\n", status)
-		return engine.StatusOK
+	container, err := daemon.Get(name)
+	if err != nil {
+		return job.Errorf("%s: %s", job.Name, err.Error())
 	}
-	return job.Errorf("%s: No such container: %s", job.Name, name)
+	status, _ := container.WaitStop(-1 * time.Second)
+	job.Printf("%d\n", status)
+	return engine.StatusOK
 }

integration/api_test.go

@@ -325,7 +325,7 @@ func TestPostCreateNull(t *testing.T) {
 
 	containerAssertExists(eng, containerID, t)
 
-	c := daemon.Get(containerID)
+	c, _ := daemon.Get(containerID)
 	if c.Config.Cpuset != "" {
 		t.Fatalf("Cpuset should have been empty - instead its:" + c.Config.Cpuset)
 	}

integration/runtime_test.go

@@ -282,12 +282,12 @@ func TestDaemonCreate(t *testing.T) {
 	}
 
 	// Make sure we can get the container with Get()
-	if daemon.Get(container.ID) == nil {
+	if _, err := daemon.Get(container.ID); err != nil {
 		t.Errorf("Unable to get newly created container")
 	}
 
 	// Make sure it is the right container
-	if daemon.Get(container.ID) != container {
+	if c, _ := daemon.Get(container.ID); c != container {
 		t.Errorf("Get() returned the wrong container")
 	}
 
@@ -383,8 +383,8 @@ func TestDestroy(t *testing.T) {
 	}
 
 	// Make sure daemon.Get() refuses to return the unexisting container
-	if daemon.Get(container.ID) != nil {
-		t.Errorf("Unable to get newly created container")
+	if c, _ := daemon.Get(container.ID); c != nil {
+		t.Errorf("Got a container that should not exist")
 	}
 
 	// Test double destroy
@@ -407,16 +407,16 @@ func TestGet(t *testing.T) {
 	container3, _, _ := mkContainer(daemon, []string{"_", "ls", "-al"}, t)
 	defer daemon.Destroy(container3)
 
-	if daemon.Get(container1.ID) != container1 {
-		t.Errorf("Get(test1) returned %v while expecting %v", daemon.Get(container1.ID), container1)
+	if c, _ := daemon.Get(container1.ID); c != container1 {
+		t.Errorf("Get(test1) returned %v while expecting %v", c, container1)
 	}
 
-	if daemon.Get(container2.ID) != container2 {
-		t.Errorf("Get(test2) returned %v while expecting %v", daemon.Get(container2.ID), container2)
+	if c, _ := daemon.Get(container2.ID); c != container2 {
+		t.Errorf("Get(test2) returned %v while expecting %v", c, container2)
 	}
 
-	if daemon.Get(container3.ID) != container3 {
-		t.Errorf("Get(test3) returned %v while expecting %v", daemon.Get(container3.ID), container3)
+	if c, _ := daemon.Get(container3.ID); c != container3 {
+		t.Errorf("Get(test3) returned %v while expecting %v", c, container3)
 	}
 
 }
@@ -485,9 +485,9 @@ func startEchoServerContainer(t *testing.T, proto string) (*daemon.Daemon, *daem
 		t.Fatal(err)
 	}
 
-	container := daemon.Get(id)
-	if container == nil {
-		t.Fatalf("Couldn't fetch test container %s", id)
+	container, err := daemon.Get(id)
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	setTimeout(t, "Waiting for the container to be started timed out", 2*time.Second, func() {
@@ -646,8 +646,8 @@ func TestRestore(t *testing.T) {
 	if runningCount != 0 {
 		t.Fatalf("Expected 0 container alive, %d found", runningCount)
 	}
-	container3 := daemon2.Get(container1.ID)
-	if container3 == nil {
+	container3, err := daemon2.Get(container1.ID)
+	if err != nil {
 		t.Fatal("Unable to Get container")
 	}
 	if err := container3.Run(); err != nil {
@@ -666,16 +666,21 @@ func TestDefaultContainerName(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	container := daemon.Get(createNamedTestContainer(eng, config, t, "some_name"))
+	container, err := daemon.Get(createNamedTestContainer(eng, config, t, "some_name"))
+	if err != nil {
+		t.Fatal(err)
+	}
 	containerID := container.ID
 
 	if container.Name != "/some_name" {
 		t.Fatalf("Expect /some_name got %s", container.Name)
 	}
 
-	if c := daemon.Get("/some_name"); c == nil {
+	c, err := daemon.Get("/some_name")
+	if err != nil {
 		t.Fatalf("Couldn't retrieve test container as /some_name")
-	} else if c.ID != containerID {
+	}
+	if c.ID != containerID {
 		t.Fatalf("Container /some_name has ID %s instead of %s", c.ID, containerID)
 	}
 }
@@ -690,14 +695,17 @@ func TestRandomContainerName(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	container := daemon.Get(createTestContainer(eng, config, t))
+	container, err := daemon.Get(createTestContainer(eng, config, t))
+	if err != nil {
+		t.Fatal(err)
+	}
 	containerID := container.ID
 
 	if container.Name == "" {
 		t.Fatalf("Expected not empty container name")
 	}
 
-	if c := daemon.Get(container.Name); c == nil {
+	if c, err := daemon.Get(container.Name); err != nil {
 		log.Fatalf("Could not lookup container %s by its name", container.Name)
 	} else if c.ID != containerID {
 		log.Fatalf("Looking up container name %s returned id %s instead of %s", container.Name, c.ID, containerID)
@@ -737,13 +745,16 @@ func TestContainerNameValidation(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		container := daemon.Get(engine.Tail(outputBuffer, 1))
+		container, err := daemon.Get(engine.Tail(outputBuffer, 1))
+		if err != nil {
+			t.Fatal(err)
+		}
 
 		if container.Name != "/"+test.Name {
 			t.Fatalf("Expect /%s got %s", test.Name, container.Name)
 		}
 
-		if c := daemon.Get("/" + test.Name); c == nil {
+		if c, err := daemon.Get("/" + test.Name); err != nil {
 			t.Fatalf("Couldn't retrieve test container as /%s", test.Name)
 		} else if c.ID != container.ID {
 			t.Fatalf("Container /%s has ID %s instead of %s", test.Name, c.ID, container.ID)
@@ -762,7 +773,10 @@ func TestLinkChildContainer(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	container := daemon.Get(createNamedTestContainer(eng, config, t, "/webapp"))
+	container, err := daemon.Get(createNamedTestContainer(eng, config, t, "/webapp"))
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	webapp, err := daemon.GetByName("/webapp")
 	if err != nil {
@@ -778,7 +792,10 @@ func TestLinkChildContainer(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	childContainer := daemon.Get(createTestContainer(eng, config, t))
+	childContainer, err := daemon.Get(createTestContainer(eng, config, t))
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if err := daemon.RegisterLink(webapp, childContainer, "db"); err != nil {
 		t.Fatal(err)
@@ -804,7 +821,10 @@ func TestGetAllChildren(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	container := daemon.Get(createNamedTestContainer(eng, config, t, "/webapp"))
+	container, err := daemon.Get(createNamedTestContainer(eng, config, t, "/webapp"))
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	webapp, err := daemon.GetByName("/webapp")
 	if err != nil {
@@ -820,7 +840,10 @@ func TestGetAllChildren(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	childContainer := daemon.Get(createTestContainer(eng, config, t))
+	childContainer, err := daemon.Get(createTestContainer(eng, config, t))
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if err := daemon.RegisterLink(webapp, childContainer, "db"); err != nil {
 		t.Fatal(err)

integration/utils_test.go

@@ -117,7 +117,7 @@ func containerAssertExists(eng *engine.Engine, id string, t Fataler) {
 
 func containerAssertNotExists(eng *engine.Engine, id string, t Fataler) {
 	daemon := mkDaemonFromEngine(eng, t)
-	if c := daemon.Get(id); c != nil {
+	if c, _ := daemon.Get(id); c != nil {
 		t.Fatal(fmt.Errorf("Container %s should not exist", id))
 	}
 }
@@ -142,9 +142,9 @@ func assertHttpError(r *httptest.ResponseRecorder, t Fataler) {
 
 func getContainer(eng *engine.Engine, id string, t Fataler) *daemon.Container {
 	daemon := mkDaemonFromEngine(eng, t)
-	c := daemon.Get(id)
-	if c == nil {
-		t.Fatal(fmt.Errorf("No such container: %s", id))
+	c, err := daemon.Get(id)
+	if err != nil {
+		t.Fatal(err)
 	}
 	return c
 }

pkg/truncindex/truncindex.go

@@ -10,10 +10,8 @@ import (
 )
 
 var (
-	// ErrNoID is thrown when attempting to use empty prefixes
-	ErrNoID = errors.New("prefix can't be empty")
-	// ErrDuplicateID is thrown when a duplicated id was found
-	ErrDuplicateID = errors.New("multiple IDs were found")
+	ErrEmptyPrefix     = errors.New("Prefix can't be empty")
+	ErrAmbiguousPrefix = errors.New("Multiple IDs found with provided prefix")
 )
 
 func init() {
@@ -47,7 +45,7 @@ func (idx *TruncIndex) addID(id string) error {
 		return fmt.Errorf("illegal character: ' '")
 	}
 	if id == "" {
-		return ErrNoID
+		return ErrEmptyPrefix
 	}
 	if _, exists := idx.ids[id]; exists {
 		return fmt.Errorf("id already exists: '%s'", id)
@@ -87,26 +85,26 @@ func (idx *TruncIndex) Delete(id string) error {
 // Get retrieves an ID from the TruncIndex. If there are multiple IDs
 // with the given prefix, an error is thrown.
 func (idx *TruncIndex) Get(s string) (string, error) {
-	idx.RLock()
-	defer idx.RUnlock()
+	if s == "" {
+		return "", ErrEmptyPrefix
+	}
 	var (
 		id string
 	)
-	if s == "" {
-		return "", ErrNoID
-	}
 	subTreeVisitFunc := func(prefix patricia.Prefix, item patricia.Item) error {
 		if id != "" {
 			// we haven't found the ID if there are two or more IDs
 			id = ""
-			return ErrDuplicateID
+			return ErrAmbiguousPrefix
 		}
 		id = string(prefix)
 		return nil
 	}
 
+	idx.RLock()
+	defer idx.RUnlock()
 	if err := idx.trie.VisitSubtree(patricia.Prefix(s), subTreeVisitFunc); err != nil {
-		return "", fmt.Errorf("no such id: %s", s)
+		return "", err
 	}
 	if id != "" {
 		return id, nil

pkg/truncindex/truncindex_test.go

@@ -59,6 +59,11 @@ func TestTruncIndex(t *testing.T) {
 	assertIndexGet(t, index, id[:4], "", true)
 	assertIndexGet(t, index, id[:1], "", true)
 
+	// An ambiguous id prefix should return an error
+	if _, err := index.Get(id[:4]); err == nil || err == nil {
+		t.Fatal("An ambiguous id prefix should return an error")
+	}
+
 	// 7 characters should NOT conflict
 	assertIndexGet(t, index, id[:7], id, false)
 	assertIndexGet(t, index, id2[:7], id2, false)