From b3974abe4f01d408850e245c9b52c77f3571e0b2 Mon Sep 17 00:00:00 2001 From: Sven Dowideit Date: Fri, 25 Oct 2013 11:59:59 +1000 Subject: [PATCH] make all image ID and container ID API responses use the Long ID (Closes #2098) --- AUTHORS | 1 + commands_test.go | 128 +++++++++++++++++++++++++++++-- container.go | 16 +--- container_test.go | 3 +- docs/sources/terms/container.rst | 7 ++ docs/sources/terms/image.rst | 8 ++ docs/sources/use/basics.rst | 23 +++++- image.go | 4 - runtime.go | 4 +- server.go | 34 ++++---- 10 files changed, 182 insertions(+), 46 deletions(-) diff --git a/AUTHORS b/AUTHORS index 64f2ce21aa..711c7718ff 100644 --- a/AUTHORS +++ b/AUTHORS @@ -165,6 +165,7 @@ Sridatta Thatipamala Sridhar Ratnakumar Steeve Morin Stefan Praszalowicz +Sven Dowideit Thatcher Peskens Thermionix Thijs Terlouw diff --git a/commands_test.go b/commands_test.go index 6c6a8e975b..1778f1b89f 100644 --- a/commands_test.go +++ b/commands_test.go @@ -6,6 +6,8 @@ import ( "github.com/dotcloud/docker/utils" "io" "io/ioutil" + "os" + "path" "regexp" "strings" "testing" @@ -381,8 +383,8 @@ func TestRunAttachStdin(t *testing.T) { if err != nil { t.Fatal(err) } - if cmdOutput != container.ShortID()+"\n" { - t.Fatalf("Wrong output: should be '%s', not '%s'\n", container.ShortID()+"\n", cmdOutput) + if cmdOutput != container.ID+"\n" { + t.Fatalf("Wrong output: should be '%s', not '%s'\n", container.ID+"\n", cmdOutput) } }) @@ -459,7 +461,7 @@ func TestRunDetach(t *testing.T) { }) } -// TestAttachDetach checks that attach in tty mode can be detached +// TestAttachDetach checks that attach in tty mode can be detached using the long container ID func TestAttachDetach(t *testing.T) { stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() @@ -486,8 +488,8 @@ func TestAttachDetach(t *testing.T) { container = globalRuntime.List()[0] - if strings.Trim(string(buf[:n]), " \r\n") != container.ShortID() { - t.Fatalf("Wrong ID received. Expect %s, received %s", container.ShortID(), buf[:n]) + if strings.Trim(string(buf[:n]), " \r\n") != container.ID { + t.Fatalf("Wrong ID received. Expect %s, received %s", container.ID, buf[:n]) } }) setTimeout(t, "Starting container timed out", 10*time.Second, func() { @@ -501,7 +503,69 @@ func TestAttachDetach(t *testing.T) { ch = make(chan struct{}) go func() { defer close(ch) - if err := cli.CmdAttach(container.ShortID()); err != nil { + if err := cli.CmdAttach(container.ID); err != nil { + if err != io.ErrClosedPipe { + t.Fatal(err) + } + } + }() + + setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err != io.ErrClosedPipe { + t.Fatal(err) + } + } + }) + + setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { + stdinPipe.Write([]byte{16, 17}) + if err := stdinPipe.Close(); err != nil { + t.Fatal(err) + } + }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + + // wait for CmdRun to return + setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() { + <-ch + }) + + time.Sleep(500 * time.Millisecond) + if !container.State.Running { + t.Fatal("The detached container should be still running") + } + + setTimeout(t, "Waiting for container to die timedout", 5*time.Second, func() { + container.Kill() + }) +} + +// TestAttachDetachTruncatedID checks that attach in tty mode can be detached +func TestAttachDetachTruncatedID(t *testing.T) { + stdin, stdinPipe := io.Pipe() + stdout, stdoutPipe := io.Pipe() + + cli := NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + + go stdout.Read(make([]byte, 1024)) + setTimeout(t, "Starting container timed out", 2*time.Second, func() { + if err := cli.CmdRun("-i", "-t", "-d", unitTestImageID, "cat"); err != nil { + t.Fatal(err) + } + }) + + container := globalRuntime.List()[0] + + stdin, stdinPipe = io.Pipe() + stdout, stdoutPipe = io.Pipe() + cli = NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) + + ch := make(chan struct{}) + go func() { + defer close(ch) + if err := cli.CmdAttach(utils.TruncateID(container.ID)); err != nil { if err != io.ErrClosedPipe { t.Fatal(err) } @@ -825,3 +889,55 @@ run [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ] return image } + +// #2098 - Docker cidFiles only contain short version of the containerId +//sudo docker run -cidfile /tmp/docker_test.cid ubuntu echo "test" +// TestRunCidFile tests that run -cidfile returns the longid +func TestRunCidFile(t *testing.T) { + stdout, stdoutPipe := io.Pipe() + + tmpDir, err := ioutil.TempDir("", "TestRunCidFile") + if err != nil { + t.Fatal(err) + } + tmpCidFile := path.Join(tmpDir, "cid") + + cli := NewDockerCli(nil, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + + c := make(chan struct{}) + go func() { + defer close(c) + if err := cli.CmdRun("-cidfile", tmpCidFile, unitTestImageID, "ls"); err != nil { + t.Fatal(err) + } + }() + + defer os.RemoveAll(tmpDir) + setTimeout(t, "Reading command output time out", 2*time.Second, func() { + cmdOutput, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil { + t.Fatal(err) + } + if len(cmdOutput) < 1 { + t.Fatalf("'ls' should return something , not '%s'", cmdOutput) + } + //read the tmpCidFile + buffer, err := ioutil.ReadFile(tmpCidFile) + if err != nil { + t.Fatal(err) + } + id := string(buffer) + + if len(id) != len("2bf44ea18873287bd9ace8a4cb536a7cbe134bed67e805fdf2f58a57f69b320c") { + t.Fatalf("-cidfile should be a long id, not '%s'", id) + } + //test that its a valid cid? (though the container is gone..) + //remove the file and dir. + }) + + setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + <-c + }) + +} diff --git a/container.go b/container.go index 419ba84d29..330ddb0d2d 100644 --- a/container.go +++ b/container.go @@ -1235,7 +1235,7 @@ func (container *Container) monitor() { container.State.setStopped(exitCode) if container.runtime != nil && container.runtime.srv != nil { - container.runtime.srv.LogEvent("die", container.ShortID(), container.runtime.repositories.ImageName(container.Image)) + container.runtime.srv.LogEvent("die", container.ID, container.runtime.repositories.ImageName(container.Image)) } // Cleanup @@ -1302,7 +1302,7 @@ func (container *Container) kill(sig int) error { } if output, err := exec.Command("lxc-kill", "-n", container.ID, strconv.Itoa(sig)).CombinedOutput(); err != nil { - log.Printf("error killing container %s (%s, %s)", container.ShortID(), output, err) + log.Printf("error killing container %s (%s, %s)", utils.TruncateID(container.ID), output, err) return err } @@ -1322,9 +1322,9 @@ func (container *Container) Kill() error { // 2. Wait for the process to die, in last resort, try to kill the process directly if err := container.WaitTimeout(10 * time.Second); err != nil { if container.cmd == nil { - return fmt.Errorf("lxc-kill failed, impossible to kill the container %s", container.ShortID()) + return fmt.Errorf("lxc-kill failed, impossible to kill the container %s", utils.TruncateID(container.ID)) } - log.Printf("Container %s failed to exit within 10 seconds of lxc-kill %s - trying direct SIGKILL", "SIGKILL", container.ShortID()) + log.Printf("Container %s failed to exit within 10 seconds of lxc-kill %s - trying direct SIGKILL", "SIGKILL", utils.TruncateID(container.ID)) if err := container.cmd.Process.Kill(); err != nil { return err } @@ -1460,14 +1460,6 @@ func (container *Container) Unmount() error { return Unmount(container.RootfsPath()) } -// ShortID returns a shorthand version of the container's id for convenience. -// A collision with other container shorthands is very unlikely, but possible. -// In case of a collision a lookup with Runtime.Get() will fail, and the caller -// will need to use a langer prefix, or the full-length container Id. -func (container *Container) ShortID() string { - return utils.TruncateID(container.ID) -} - func (container *Container) logPath(name string) string { return path.Join(container.root, fmt.Sprintf("%s-%s.log", container.ID, name)) } diff --git a/container_test.go b/container_test.go index d51946ece3..26007a732d 100644 --- a/container_test.go +++ b/container_test.go @@ -3,6 +3,7 @@ package docker import ( "bufio" "fmt" + "github.com/dotcloud/docker/utils" "io" "io/ioutil" "math/rand" @@ -1005,7 +1006,7 @@ func TestEnv(t *testing.T) { "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "HOME=/", "container=lxc", - "HOSTNAME=" + container.ShortID(), + "HOSTNAME=" + utils.TruncateID(container.ID), "FALSE=true", "TRUE=false", "TRICKY=tri", diff --git a/docs/sources/terms/container.rst b/docs/sources/terms/container.rst index aeb7b1c3a9..206664bd82 100644 --- a/docs/sources/terms/container.rst +++ b/docs/sources/terms/container.rst @@ -38,3 +38,10 @@ was when the container was stopped. You can promote a container to an :ref:`image_def` with ``docker commit``. Once a container is an image, you can use it as a parent for new containers. + +Container IDs +............. +All containers are identified by a 64 hexadecimal digit string (internally a 256bit +value). To simplify their use, a short ID of the first 12 characters can be used +on the commandline. There is a small possibility of short id collisions, so the +docker server will always return the long ID. diff --git a/docs/sources/terms/image.rst b/docs/sources/terms/image.rst index dafda1f3fc..6d5c8b2e7c 100644 --- a/docs/sources/terms/image.rst +++ b/docs/sources/terms/image.rst @@ -36,3 +36,11 @@ Base Image .......... An image that has no parent is a **base image**. + +Image IDs +......... +All images are identified by a 64 hexadecimal digit string (internally a 256bit +value). To simplify their use, a short ID of the first 12 characters can be used +on the command line. There is a small possibility of short id collisions, so the +docker server will always return the long ID. + diff --git a/docs/sources/use/basics.rst b/docs/sources/use/basics.rst index 0097b6836d..d1ad081f99 100644 --- a/docs/sources/use/basics.rst +++ b/docs/sources/use/basics.rst @@ -22,22 +22,37 @@ specify the path to it and manually start it. # Run docker in daemon mode sudo /docker -d & - -Running an interactive shell ----------------------------- +Download a pre-built image +-------------------------- .. code-block:: bash # Download an ubuntu image sudo docker pull ubuntu +This will find the ``ubuntu`` image by name in the :ref:`Central Index +` and download it from the top-level Central +Repository to a local image cache. + +.. NOTE:: When the image has successfully downloaded, you will see a 12 +character hash ``539c0211cd76: Download complete`` which is the short +form of the image ID. These short image IDs are the first 12 characters +of the full image ID - which can be found using ``docker inspect`` or +``docker images -notrunc=true`` + +.. _dockergroup: + +Running an interactive shell +---------------------------- + +.. code-block:: bash + # Run an interactive shell in the ubuntu image, # allocate a tty, attach stdin and stdout # To detach the tty without exiting the shell, # use the escape sequence Ctrl-p + Ctrl-q sudo docker run -i -t ubuntu /bin/bash -.. _dockergroup: Why ``sudo``? ------------- diff --git a/image.go b/image.go index 94cccaac67..c600273c1a 100644 --- a/image.go +++ b/image.go @@ -202,10 +202,6 @@ func (image *Image) Changes(rw string) ([]Change, error) { return Changes(layers, rw) } -func (image *Image) ShortID() string { - return utils.TruncateID(image.ID) -} - func ValidateID(id string) error { if id == "" { return fmt.Errorf("Image id can't be empty") diff --git a/runtime.go b/runtime.go index 671694fce5..6a3b76a595 100644 --- a/runtime.go +++ b/runtime.go @@ -181,7 +181,7 @@ func (runtime *Runtime) ensureName(container *Container) error { if container.Name == "" { name, err := generateRandomName(runtime) if err != nil { - name = container.ShortID() + name = utils.TruncateID(container.ID) } container.Name = name @@ -288,7 +288,7 @@ func (runtime *Runtime) restore() error { // Try to set the default name for a container if it exists prior to links container.Name, err = generateRandomName(runtime) if err != nil { - container.Name = container.ShortID() + container.Name = utils.TruncateID(container.ID) } if _, err := runtime.containerGraph.Set(container.Name, container.ID); err != nil { diff --git a/server.go b/server.go index 3441029027..2e23612621 100644 --- a/server.go +++ b/server.go @@ -154,7 +154,7 @@ func (srv *Server) ContainerKill(name string, sig int) error { if err := container.Kill(); err != nil { return fmt.Errorf("Cannot kill container %s: %s", name, err) } - srv.LogEvent("kill", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) + srv.LogEvent("kill", container.ID, srv.runtime.repositories.ImageName(container.Image)) } else { // Otherwise, just send the requested signal if err := container.kill(sig); err != nil { @@ -180,7 +180,7 @@ func (srv *Server) ContainerExport(name string, out io.Writer) error { if _, err := io.Copy(out, data); err != nil { return err } - srv.LogEvent("export", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) + srv.LogEvent("export", container.ID, srv.runtime.repositories.ImageName(container.Image)) return nil } return fmt.Errorf("No such container: %s", name) @@ -230,7 +230,7 @@ func (srv *Server) ImageInsert(name, url, path string, out io.Writer, sf *utils. return "", err } out.Write(sf.FormatStatus("", img.ID)) - return img.ShortID(), nil + return img.ID, nil } func (srv *Server) ImagesViz(out io.Writer) error { @@ -250,9 +250,9 @@ func (srv *Server) ImagesViz(out io.Writer) error { return fmt.Errorf("Error while getting parent image: %v", err) } if parentImage != nil { - out.Write([]byte(" \"" + parentImage.ShortID() + "\" -> \"" + image.ShortID() + "\"\n")) + out.Write([]byte(" \"" + parentImage.ID + "\" -> \"" + image.ID + "\"\n")) } else { - out.Write([]byte(" base -> \"" + image.ShortID() + "\" [style=invis]\n")) + out.Write([]byte(" base -> \"" + image.ID + "\" [style=invis]\n")) } } @@ -465,7 +465,7 @@ func (srv *Server) Containers(all, size bool, n int, since, before string) []API continue } if before != "" { - if container.ShortID() == before { + if container.ID == before || utils.TruncateID(container.ID) == before { foundBefore = true continue } @@ -476,7 +476,7 @@ func (srv *Server) Containers(all, size bool, n int, since, before string) []API if displayed == n { break } - if container.ShortID() == since { + if container.ID == since || utils.TruncateID(container.ID) == since { break } displayed++ @@ -518,7 +518,7 @@ func (srv *Server) ContainerCommit(name, repo, tag, author, comment string, conf if err != nil { return "", err } - return img.ShortID(), err + return img.ID, err } func (srv *Server) ContainerTag(name, repo, tag string, force bool) error { @@ -1017,7 +1017,7 @@ func (srv *Server) ImageImport(src, repo, tag string, in io.Reader, out io.Write return err } } - out.Write(sf.FormatStatus("", img.ShortID())) + out.Write(sf.FormatStatus("", img.ID)) return nil } @@ -1046,8 +1046,8 @@ func (srv *Server) ContainerCreate(config *Config, name string) (string, []strin } return "", nil, err } - srv.LogEvent("create", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) - return container.ShortID(), buildWarnings, nil + srv.LogEvent("create", container.ID, srv.runtime.repositories.ImageName(container.Image)) + return container.ID, buildWarnings, nil } func (srv *Server) ContainerRestart(name string, t int) error { @@ -1055,7 +1055,7 @@ func (srv *Server) ContainerRestart(name string, t int) error { if err := container.Restart(t); err != nil { return fmt.Errorf("Cannot restart container %s: %s", name, err) } - srv.LogEvent("restart", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) + srv.LogEvent("restart", container.ID, srv.runtime.repositories.ImageName(container.Image)) } else { return fmt.Errorf("No such container: %s", name) } @@ -1111,7 +1111,7 @@ func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) if err := srv.runtime.Destroy(container); err != nil { return fmt.Errorf("Cannot destroy container %s: %s", name, err) } - srv.LogEvent("destroy", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) + srv.LogEvent("destroy", container.ID, srv.runtime.repositories.ImageName(container.Image)) if removeVolume { // Retrieve all volumes from all remaining containers @@ -1228,8 +1228,8 @@ func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, erro return nil, err } if tagDeleted { - imgs = append(imgs, APIRmi{Untagged: img.ShortID()}) - srv.LogEvent("untag", img.ShortID(), "") + imgs = append(imgs, APIRmi{Untagged: img.ID}) + srv.LogEvent("untag", img.ID, "") } } if len(srv.runtime.repositories.ByID()[img.ID]) == 0 { @@ -1364,7 +1364,7 @@ func (srv *Server) ContainerStart(name string, hostConfig *HostConfig) error { if err := container.Start(); err != nil { return fmt.Errorf("Cannot start container %s: %s", name, err) } - srv.LogEvent("start", container.ShortID(), runtime.repositories.ImageName(container.Image)) + srv.LogEvent("start", container.ID, runtime.repositories.ImageName(container.Image)) return nil } @@ -1374,7 +1374,7 @@ func (srv *Server) ContainerStop(name string, t int) error { if err := container.Stop(t); err != nil { return fmt.Errorf("Cannot stop container %s: %s", name, err) } - srv.LogEvent("stop", container.ShortID(), srv.runtime.repositories.ImageName(container.Image)) + srv.LogEvent("stop", container.ID, srv.runtime.repositories.ImageName(container.Image)) } else { return fmt.Errorf("No such container: %s", name) }