From d841b779fda882c3fd505f626c01cc9d60768aa6 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Thu, 9 Jul 2015 19:39:57 -0700 Subject: [PATCH] Return 404 on exec-inspect when container is dead but exec is still around When a container is removed but it had an exec, that still hasn't been GC'd per PR #14476, and someone tries to inspect the exec we should return a 404, not a 500+container not running. Returning "..not running" is not only misleading because it could lead people to think the container is actually still around, but after 5 minutes the error will change to a 404 after the GC. This means that we're externalizing our internall soft-deletion/GC logic which shouldn't be any of the end user's concern. They should get the same results immediate or after 5 minutes. Signed-off-by: Doug Davis --- daemon/exec.go | 11 ++++++++++- integration-cli/docker_cli_exec_test.go | 11 +++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/daemon/exec.go b/daemon/exec.go index 82e267bc22..0a7afe254f 100644 --- a/daemon/exec.go +++ b/daemon/exec.go @@ -81,7 +81,16 @@ func (d *Daemon) registerExecCommand(execConfig *execConfig) { } func (d *Daemon) getExecConfig(name string) (*execConfig, error) { - if execConfig := d.execCommands.Get(name); execConfig != nil { + execConfig := d.execCommands.Get(name) + + // If the exec is found but its container is not in the daemon's list of + // containers then it must have been delete, in which case instead of + // saying the container isn't running, we should return a 404 so that + // the user sees the same error now that they will after the + // 5 minute clean-up loop is run which erases old/dead execs. + + if execConfig != nil && d.containers.Get(execConfig.Container.ID) != nil { + if !execConfig.Container.IsRunning() { return nil, fmt.Errorf("Container %s is not running", execConfig.Container.ID) } diff --git a/integration-cli/docker_cli_exec_test.go b/integration-cli/docker_cli_exec_test.go index 3c59552c7b..e39c0aaf9b 100644 --- a/integration-cli/docker_cli_exec_test.go +++ b/integration-cli/docker_cli_exec_test.go @@ -481,6 +481,17 @@ func (s *DockerSuite) TestInspectExecID(c *check.C) { if sc != http.StatusOK { c.Fatalf("received status != 200 OK: %s\n%s", sc, body) } + + // Now delete the container and then an 'inspect' on the exec should + // result in a 404 (not 'container not running') + out, ec := dockerCmd(c, "rm", "-f", id) + if ec != 0 { + c.Fatalf("error removing container: %s", out) + } + sc, body, err = sockRequest("GET", "/exec/"+execID+"/json", nil) + if sc != http.StatusNotFound { + c.Fatalf("received status != 404: %s\n%s", sc, body) + } } func (s *DockerSuite) TestLinksPingLinkedContainersOnRename(c *check.C) {