Merge pull request #2140 from SvenDowideit/2098-run-cidfile-should-return-longid

Server: always return long id's

AUTHORS

@@ -165,6 +165,7 @@ Sridatta Thatipamala <sthatipamala@gmail.com>
 Sridhar Ratnakumar <sridharr@activestate.com>
 Steeve Morin <steeve.morin@gmail.com>
 Stefan Praszalowicz <stefan@greplin.com>
+Sven Dowideit <SvenDowideit@home.org.au>
 Thatcher Peskens <thatcher@dotcloud.com>
 Thermionix <bond711@gmail.com>
 Thijs Terlouw <thijsterlouw@gmail.com>

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
+	})
+
+}

container.go

@@ -1247,7 +1247,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
@@ -1314,7 +1314,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
 	}
 
@@ -1334,9 +1334,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
 		}
@@ -1472,14 +1472,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))
 }

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",

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.

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.
+

docs/sources/use/basics.rst

@@ -22,22 +22,37 @@ specify the path to it and manually start it.
     # Run docker in daemon mode
     sudo <path to>/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 
+<searching_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``?
 -------------

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")

runtime.go

@@ -182,7 +182,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
 
@@ -289,7 +289,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 {

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)
 	}