don't leave empty cidFile behind

This makes `--cidfile` clean up empty container ID files. These are left behind when creating the container fails. Docker-DCO-1.1-Signed-off-by: Cristian Staretu <cristian.staretu@gmail.com> (github: unclejack)
rjnagal · Mar 12, 2014 · a56d1b9 · a56d1b9
1 parent 2ad16c8
commit a56d1b9
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 2 deletions.
diff --git a/api/client.go b/api/client.go
@@ -1763,7 +1763,21 @@ func (cli *DockerCli) CmdRun(args ...string) error {
 		if containerIDFile, err = os.Create(hostConfig.ContainerIDFile); err != nil {
 			return fmt.Errorf("Failed to create the container ID file: %s", err)
 		}
-		defer containerIDFile.Close()
+		defer func() {
+			containerIDFile.Close()
+			var (
+				cidFileInfo os.FileInfo
+				err         error
+			)
+			if cidFileInfo, err = os.Stat(hostConfig.ContainerIDFile); err != nil {
+				return
+			}
+			if cidFileInfo.Size() == 0 {
+				if err := os.Remove(hostConfig.ContainerIDFile); err != nil {
+					fmt.Printf("failed to remove CID file '%s': %s \n", hostConfig.ContainerIDFile, err)
+				}
+			}
+		}()
 	}
 
 	containerValues := url.Values{}

diff --git a/integration/commands_test.go b/integration/commands_test.go
@@ -931,7 +931,7 @@ run    [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ]
 // #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) {
+func TestRunCidFileCheckIDLength(t *testing.T) {
 	stdout, stdoutPipe := io.Pipe()
 
 	tmpDir, err := ioutil.TempDir("", "TestRunCidFile")
@@ -980,6 +980,35 @@ func TestRunCidFile(t *testing.T) {
 
 }
 
+// Ensure that CIDFile gets deleted if it's empty
+// Perform this test by making `docker run` fail
+func TestRunCidFileCleanupIfEmpty(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "TestRunCidFile")
+	if err != nil {
+		t.Fatal(err)
+	}
+	tmpCidFile := path.Join(tmpDir, "cid")
+
+	cli := api.NewDockerCli(nil, ioutil.Discard, ioutil.Discard, testDaemonProto, testDaemonAddr)
+	defer cleanup(globalEngine, t)
+
+	c := make(chan struct{})
+	go func() {
+		defer close(c)
+		if err := cli.CmdRun("--cidfile", tmpCidFile, unitTestImageID); err == nil {
+			t.Fatal("running without a command should haveve failed")
+		}
+		if _, err := os.Stat(tmpCidFile); err == nil {
+			t.Fatalf("empty CIDFile '%s' should've been deleted", tmpCidFile)
+		}
+	}()
+	defer os.RemoveAll(tmpDir)
+
+	setTimeout(t, "CmdRun timed out", 5*time.Second, func() {
+		<-c
+	})
+}
+
 func TestContainerOrphaning(t *testing.T) {
 
 	// setup a temporary directory