Browse code
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 <dug@us.ibm.com>
Doug Davis authored on 2015/07/10 11:39:57Showing 2 changed files
| ... | ... |
@@ -81,7 +81,16 @@ func (d *Daemon) registerExecCommand(execConfig *execConfig) {
|
| 81 | 81 |
} |
| 82 | 82 |
|
| 83 | 83 |
func (d *Daemon) getExecConfig(name string) (*execConfig, error) {
|
| 84 |
- if execConfig := d.execCommands.Get(name); execConfig != nil {
|
|
| 84 |
+ execConfig := d.execCommands.Get(name) |
|
| 85 |
+ |
|
| 86 |
+ // If the exec is found but its container is not in the daemon's list of |
|
| 87 |
+ // containers then it must have been delete, in which case instead of |
|
| 88 |
+ // saying the container isn't running, we should return a 404 so that |
|
| 89 |
+ // the user sees the same error now that they will after the |
|
| 90 |
+ // 5 minute clean-up loop is run which erases old/dead execs. |
|
| 91 |
+ |
|
| 92 |
+ if execConfig != nil && d.containers.Get(execConfig.Container.ID) != nil {
|
|
| 93 |
+ |
|
| 85 | 94 |
if !execConfig.Container.IsRunning() {
|
| 86 | 95 |
return nil, fmt.Errorf("Container %s is not running", execConfig.Container.ID)
|
| 87 | 96 |
} |
| ... | ... |
@@ -481,6 +481,17 @@ func (s *DockerSuite) TestInspectExecID(c *check.C) {
|
| 481 | 481 |
if sc != http.StatusOK {
|
| 482 | 482 |
c.Fatalf("received status != 200 OK: %s\n%s", sc, body)
|
| 483 | 483 |
} |
| 484 |
+ |
|
| 485 |
+ // Now delete the container and then an 'inspect' on the exec should |
|
| 486 |
+ // result in a 404 (not 'container not running') |
|
| 487 |
+ out, ec := dockerCmd(c, "rm", "-f", id) |
|
| 488 |
+ if ec != 0 {
|
|
| 489 |
+ c.Fatalf("error removing container: %s", out)
|
|
| 490 |
+ } |
|
| 491 |
+ sc, body, err = sockRequest("GET", "/exec/"+execID+"/json", nil)
|
|
| 492 |
+ if sc != http.StatusNotFound {
|
|
| 493 |
+ c.Fatalf("received status != 404: %s\n%s", sc, body)
|
|
| 494 |
+ } |
|
| 484 | 495 |
} |
| 485 | 496 |
|
| 486 | 497 |
func (s *DockerSuite) TestLinksPingLinkedContainersOnRename(c *check.C) {
|