diff --git a/api/client/commands.go b/api/client/commands.go index e42c6f26ed..f3775c95f0 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -862,6 +862,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") { diff --git a/api/server/server.go b/api/server/server.go index d5cdbd00cc..0184f5cf87 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -1132,7 +1132,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"]) diff --git a/builder/internals.go b/builder/internals.go index 830da72725..99484600db 100644 --- a/builder/internals.go +++ b/builder/internals.go @@ -86,9 +86,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 @@ -709,7 +709,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 diff --git a/daemon/attach.go b/daemon/attach.go index 881b021e17..89af4360f4 100644 --- a/daemon/attach.go +++ b/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 diff --git a/daemon/changes.go b/daemon/changes.go index 1e5726eda8..faa4323145 100644 --- a/daemon/changes.go +++ b/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 { - return job.Error(err) - } - } else { - return job.Errorf("No such container: %s", name) + + 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) + } + outs.Add(out) + } + + if _, err := outs.WriteListTo(job.Stdout); err != nil { + return job.Error(err) + } + return engine.StatusOK } diff --git a/daemon/commit.go b/daemon/commit.go index 06d0465adc..7c83c60cc4 100644 --- a/daemon/commit.go +++ b/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 ( diff --git a/daemon/container.go b/daemon/container.go index c1c215ffee..45b07abb48 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -1113,7 +1113,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 { @@ -1382,9 +1387,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) @@ -1399,9 +1404,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]) diff --git a/daemon/copy.go b/daemon/copy.go index 9d18b010c0..d42f450fdb 100644 --- a/daemon/copy.go +++ b/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 { - - 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 + container, err := daemon.Get(name) + if err != nil { + return job.Error(err) } - return job.Errorf("No such container: %s", name) + + 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 } diff --git a/daemon/create.go b/daemon/create.go index 785b0cc345..ebe4ccf02e 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -129,9 +129,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) diff --git a/daemon/daemon.go b/daemon/daemon.go index c03e9d7aa8..dd36c2889a 100644 --- a/daemon/daemon.go +++ b/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 diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go new file mode 100644 index 0000000000..43030b6f9b --- /dev/null +++ b/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) +} diff --git a/daemon/delete.go b/daemon/delete.go index 59c7651785..78daa2aab0 100644 --- a/daemon/delete.go +++ b/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) diff --git a/daemon/exec.go b/daemon/exec.go index 8bb4e72d13..9881ed0896 100644 --- a/daemon/exec.go +++ b/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() { diff --git a/daemon/export.go b/daemon/export.go index bc0f14a3bb..859c80f9bb 100644 --- a/daemon/export.go +++ b/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 } diff --git a/daemon/inspect.go b/daemon/inspect.go index 37d00573bc..96095c9fa0 100644 --- a/daemon/inspect.go +++ b/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) + } - 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()) - - 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) - - container.hostConfig.Links = nil - if _, err := out.WriteTo(job.Stdout); err != 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 } - return job.Errorf("No such container: %s", name) + + 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()) + + 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) + + container.hostConfig.Links = nil + if _, err := out.WriteTo(job.Stdout); err != nil { + return job.Error(err) + } + return engine.StatusOK } func (daemon *Daemon) ContainerExecInspect(job *engine.Job) engine.Status { diff --git a/daemon/kill.go b/daemon/kill.go index f5f5897c88..84094f8fbf 100644 --- a/daemon/kill.go +++ b/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 } diff --git a/daemon/list.go b/daemon/list.go index 5197d9986c..676ff53f27 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -61,16 +61,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) } } diff --git a/daemon/logs.go b/daemon/logs.go index 6c9373f737..28ff9be4ee 100644 --- a/daemon/logs.go +++ b/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) { diff --git a/daemon/pause.go b/daemon/pause.go index 0e4323d9a8..af943de103 100644 --- a/daemon/pause.go +++ b/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) diff --git a/daemon/rename.go b/daemon/rename.go index 1dedc7d3a7..3d315252ae 100644 --- a/daemon/rename.go +++ b/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 diff --git a/daemon/resize.go b/daemon/resize.go index 68c070370a..860f79eba4 100644 --- a/daemon/resize.go +++ b/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 { diff --git a/daemon/restart.go b/daemon/restart.go index bcc057156d..bcde628d38 100644 --- a/daemon/restart.go +++ b/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 } diff --git a/daemon/start.go b/daemon/start.go index d6655189d7..4a35555dca 100644 --- a/daemon/start.go +++ b/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() { diff --git a/daemon/stop.go b/daemon/stop.go index 1a098a1ad3..e2f1d284a8 100644 --- a/daemon/stop.go +++ b/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 } diff --git a/daemon/top.go b/daemon/top.go index 4d916ee5dc..782cc83dcf 100644 --- a/daemon/top.go +++ b/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) - } - - 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") - } - - 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) - } - } - } - out.SetJson("Processes", processes) - out.WriteTo(job.Stdout) - return engine.StatusOK - + container, err := daemon.Get(name) + if err != nil { + return job.Error(err) } - return job.Errorf("No such container: %s", name) + 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) + + 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) + } + + 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 } diff --git a/daemon/volumes.go b/daemon/volumes.go index 7b49733839..c6f1b9930a 100644 --- a/daemon/volumes.go +++ b/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 ( diff --git a/daemon/wait.go b/daemon/wait.go index a1f657c353..e2747a3e42 100644 --- a/daemon/wait.go +++ b/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 } diff --git a/integration/api_test.go b/integration/api_test.go index ab2c3070b4..8daa4d46f3 100644 --- a/integration/api_test.go +++ b/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) } diff --git a/integration/runtime_test.go b/integration/runtime_test.go index a436995fd3..73ed793043 100644 --- a/integration/runtime_test.go +++ b/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) diff --git a/integration/utils_test.go b/integration/utils_test.go index 32ca8e0d62..61a156e476 100644 --- a/integration/utils_test.go +++ b/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 } diff --git a/pkg/truncindex/truncindex.go b/pkg/truncindex/truncindex.go index eec5597306..73c7e24fb4 100644 --- a/pkg/truncindex/truncindex.go +++ b/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 diff --git a/pkg/truncindex/truncindex_test.go b/pkg/truncindex/truncindex_test.go index 32c41c7d76..8ad1634fd4 100644 --- a/pkg/truncindex/truncindex_test.go +++ b/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)