diff --git a/api/server/httputils/errors.go b/api/server/httputils/errors.go index c8f24cad28..8c605f3362 100644 --- a/api/server/httputils/errors.go +++ b/api/server/httputils/errors.go @@ -54,6 +54,7 @@ func GetHTTPErrorStatusCode(err error) int { code int }{ {"not found", http.StatusNotFound}, + {"cannot find", http.StatusNotFound}, {"no such", http.StatusNotFound}, {"bad parameter", http.StatusBadRequest}, {"no command", http.StatusBadRequest}, diff --git a/daemon/exec.go b/daemon/exec.go index 001ffbd7fd..1622a9cd3c 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -165,19 +165,26 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R return fmt.Errorf("Error: Exec command %s is already running", ec.ID) } ec.Running = true - defer func() { - if err != nil { - ec.Running = false - exitCode := 126 - ec.ExitCode = &exitCode - } - }() 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) + } + }() + if ec.OpenStdin && stdin != nil { r, w := io.Pipe() go func() { diff --git a/daemon/monitor.go b/daemon/monitor.go index ee0d1fcce0..b286e073f6 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -90,7 +90,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { execConfig.Running = false execConfig.StreamConfig.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 diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go index 2819445425..2d61ee13d6 100644 --- a/integration-cli/docker_api_exec_test.go +++ b/integration-cli/docker_api_exec_test.go @@ -120,21 +120,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) } @@ -169,8 +155,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 := request.SockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": []string{"true"}}, daemonHost()) + return createExecCmd(c, name, "true") +} + +func createExecCmd(c *check.C, name string, cmd string) string { + _, b, err := request.SockRequest("POST", fmt.Sprintf("/containers/%s/exec", name), map[string]interface{}{"Cmd": []string{cmd}}, daemonHost()) c.Assert(err, checker.IsNil, check.Commentf(string(b))) createResp := struct { @@ -198,3 +219,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 := request.SockRequestRaw("GET", fmt.Sprintf("/containers/%s/json", id), nil, "", daemonHost()) + 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) +}