Merge branch 'fix_exec_ict' into 'IT-1.11.2'

Fix docker leaks ExecIds on failed exec -i

upstream issue: moby#30311
Cherry-pick from moby#30340
fix moby#161

Signed-off-by: Dmitry Shyshkin <dmitry@shyshkin.org.ua>
Signed-off-by: Lei Jitang <leijitang@huawei.com>


See merge request docker/docker!275

api/server/httputils/errors.go

@@ -47,6 +47,7 @@ func WriteError(w http.ResponseWriter, err error) {
 		errStr := strings.ToLower(errMsg)
 		for keyword, status := range map[string]int{
 			"not found":             http.StatusNotFound,
+			"cannot find":           http.StatusNotFound,
 			"no such":               http.StatusNotFound,
 			"bad parameter":         http.StatusBadRequest,
 			"conflict":              http.StatusConflict,

daemon/exec.go

@@ -152,18 +152,25 @@ func (d *Daemon) ContainerExecStart(name string, stdin io.ReadCloser, stdout io.
 		return fmt.Errorf("Error: Exec command %s is already running", ec.ID)
 	}
 	ec.Running = true
+	ec.Unlock()
+
+	c := d.containers.Get(ec.ContainerID)
+	logrus.Debugf("starting exec command %s in container %s", ec.ID, c.ID)
+	d.LogContainerEvent(c, "exec_start: "+ec.Entrypoint+" "+strings.Join(ec.Args, " "))
+
 	defer func() {
 		if err != nil {
+			ec.Lock()
 			ec.Running = false
 			exitCode := 126
 			ec.ExitCode = &exitCode
+			if err := ec.CloseStreams(); err != nil {
+				logrus.Errorf("failed to cleanup exec %s streams: %s", c.ID, err)
+			}
+			ec.Unlock()
+			c.ExecCommands.Delete(ec.ID)
 		}
 	}()
-	ec.Unlock()
-
-	c := d.containers.Get(ec.ContainerID)
-	logrus.Debugf("starting exec command %s in container %s", ec.ID, c.ID)
-	d.LogContainerEvent(c, "exec_start: "+ec.Entrypoint+" "+strings.Join(ec.Args, " "))
 
 	if ec.OpenStdin && stdin != nil {
 		r, w := io.Pipe()

daemon/monitor.go

@@ -80,7 +80,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error {
 			execConfig.Running = false
 			execConfig.Wait()
 			if err := execConfig.CloseStreams(); err != nil {
-				logrus.Errorf("%s: %s", c.ID, err)
+				logrus.Errorf("failed to cleanup exec %s streams: %s", c.ID, err)
 			}
 
 			// remove the exec command from the container's store only and not the

integration-cli/docker_api_exec_test.go

@@ -103,21 +103,7 @@ func (s *DockerSuite) TestExecApiStartMultipleTimesError(c *check.C) {
 	runSleepingContainer(c, "-d", "--name", "test")
 	execID := createExec(c, "test")
 	startExec(c, execID, http.StatusOK)
-
-	timeout := time.After(60 * time.Second)
-	var execJSON struct{ Running bool }
-	for {
-		select {
-		case <-timeout:
-			c.Fatal("timeout waiting for exec to start")
-		default:
-		}
-
-		inspectExec(c, execID, &execJSON)
-		if !execJSON.Running {
-			break
-		}
-	}
+	waitForExec(c, execID)
 
 	startExec(c, execID, http.StatusConflict)
 }
@@ -152,8 +138,43 @@ func (s *DockerSuite) TestExecApiStartWithDetach(c *check.C) {
 	}
 }
 
+// #30311
+func (s *DockerSuite) TestExecAPIStartValidCommand(c *check.C) {
+	name := "exec_test"
+	dockerCmd(c, "run", "-d", "-t", "--name", name, "busybox", "/bin/sh")
+
+	id := createExecCmd(c, name, "true")
+	startExec(c, id, http.StatusOK)
+
+	waitForExec(c, id)
+
+	var inspectJSON struct{ ExecIDs []string }
+	inspectContainer(c, name, &inspectJSON)
+
+	c.Assert(inspectJSON.ExecIDs, checker.IsNil)
+}
+
+// #30311
+func (s *DockerSuite) TestExecAPIStartInvalidCommand(c *check.C) {
+	name := "exec_test"
+	dockerCmd(c, "run", "-d", "-t", "--name", name, "busybox", "/bin/sh")
+
+	id := createExecCmd(c, name, "invalid")
+	startExec(c, id, http.StatusNotFound)
+	waitForExec(c, id)
+
+	var inspectJSON struct{ ExecIDs []string }
+	inspectContainer(c, name, &inspectJSON)
+
+	c.Assert(inspectJSON.ExecIDs, checker.IsNil)
+}
+
 func createExec(c *check.C, name string) string {
-	_, b, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": []string{"true"}})
+	return createExecCmd(c, name, "true")
+}
+
+func createExecCmd(c *check.C, name string, cmd string) string {
+	_, b, err := sockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": []string{cmd}})
 	c.Assert(err, checker.IsNil, check.Commentf(string(b)))
 
 	createResp := struct {
@@ -181,3 +202,29 @@ func inspectExec(c *check.C, id string, out interface{}) {
 	err = json.NewDecoder(body).Decode(out)
 	c.Assert(err, checker.IsNil)
 }
+
+func waitForExec(c *check.C, id string) {
+	timeout := time.After(60 * time.Second)
+	var execJSON struct{ Running bool }
+	for {
+		select {
+		case <-timeout:
+			c.Fatal("timeout waiting for exec to start")
+		default:
+		}
+
+		inspectExec(c, id, &execJSON)
+		if !execJSON.Running {
+			break
+		}
+	}
+}
+
+func inspectContainer(c *check.C, id string, out interface{}) {
+	resp, body, err := sockRequestRaw("GET", fmt.Sprintf("/containers/%s/json", id), nil, "")
+	c.Assert(err, checker.IsNil)
+	defer body.Close()
+	c.Assert(resp.StatusCode, checker.Equals, http.StatusOK)
+	err = json.NewDecoder(body).Decode(out)
+	c.Assert(err, checker.IsNil)
+}