Skip to content

Commit 621e3d8

Browse files
calaveracalavera
committed
Keep backwards compatibility in kill api.
Return an error when the container is stopped only in api versions equal or greater than 1.20 (docker 1.8). Signed-off-by: David Calavera <[email protected]>
1 parent c473ae1 commit 621e3d8

4 files changed

Lines changed: 34 additions & 12 deletions

File tree

‎api/server/server.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,13 @@ func (s *Server) postContainersKill(version version.Version, w http.ResponseWrit
298298
}
299299

300300
if err := s.daemon.ContainerKill(name, sig); err != nil {
301-
return err
301+
_, isStopped := err.(daemon.ErrContainerNotRunning)
302+
// Return error that's not caused because the container is stopped.
303+
// Return error if the container is not running and the api is >= 1.20
304+
// to keep backwards compatibility.
305+
if version.GreaterThanOrEqualTo("1.20") || !isStopped {
306+
return fmt.Errorf("Cannot kill container %s: %v", name, err)
307+
}
302308
}
303309

304310
w.WriteHeader(http.StatusNoContent)

‎daemon/container.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ var (
4141
ErrContainerRootfsReadonly = errors.New("container rootfs is marked read-only")
4242
)
4343

44+
type ErrContainerNotRunning struct {
45+
id string
46+
}
47+
48+
func (e ErrContainerNotRunning) Error() string {
49+
return fmt.Sprintf("Container %s is not running", e.id)
50+
}
51+
4452
type StreamConfig struct {
4553
stdout *broadcastwriter.BroadcastWriter
4654
stderr *broadcastwriter.BroadcastWriter
@@ -361,7 +369,7 @@ func (container *Container) KillSig(sig int) error {
361369
}
362370

363371
if !container.Running {
364-
return fmt.Errorf("Container %s is not running", container.ID)
372+
return ErrContainerNotRunning{container.ID}
365373
}
366374

367375
// signal to the monitor that it should not restart the container
@@ -398,7 +406,7 @@ func (container *Container) Pause() error {
398406

399407
// We cannot Pause the container which is not running
400408
if !container.Running {
401-
return fmt.Errorf("Container %s is not running, cannot pause a non-running container", container.ID)
409+
return ErrContainerNotRunning{container.ID}
402410
}
403411

404412
// We cannot Pause the container which is already paused
@@ -420,7 +428,7 @@ func (container *Container) Unpause() error {
420428

421429
// We cannot unpause the container which is not running
422430
if !container.Running {
423-
return fmt.Errorf("Container %s is not running, cannot unpause a non-running container", container.ID)
431+
return ErrContainerNotRunning{container.ID}
424432
}
425433

426434
// We cannot unpause the container which is not paused
@@ -438,7 +446,7 @@ func (container *Container) Unpause() error {
438446

439447
func (container *Container) Kill() error {
440448
if !container.IsRunning() {
441-
return fmt.Errorf("Container %s is not running", container.ID)
449+
return ErrContainerNotRunning{container.ID}
442450
}
443451

444452
// 1. Send SIGKILL
@@ -520,7 +528,7 @@ func (container *Container) Restart(seconds int) error {
520528

521529
func (container *Container) Resize(h, w int) error {
522530
if !container.IsRunning() {
523-
return fmt.Errorf("Cannot resize container %s, container is not running", container.ID)
531+
return ErrContainerNotRunning{container.ID}
524532
}
525533
if err := container.command.ProcessConfig.Terminal.Resize(h, w); err != nil {
526534
return err

‎daemon/kill.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
package daemon
22

3-
import (
4-
"fmt"
5-
"syscall"
6-
)
3+
import "syscall"
74

85
// ContainerKill send signal to the container
96
// If no signal is given (sig 0), then Kill with SIGKILL and wait
@@ -18,12 +15,12 @@ func (daemon *Daemon) ContainerKill(name string, sig uint64) error {
1815
// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
1916
if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
2017
if err := container.Kill(); err != nil {
21-
return fmt.Errorf("Cannot kill container %s: %s", name, err)
18+
return err
2219
}
2320
} else {
2421
// Otherwise, just send the requested signal
2522
if err := container.KillSig(int(sig)); err != nil {
26-
return fmt.Errorf("Cannot kill container %s: %s", name, err)
23+
return err
2724
}
2825
}
2926
return nil

‎integration-cli/docker_cli_kill_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package main
22

33
import (
4+
"fmt"
5+
"net/http"
46
"strings"
57

68
"github.com/go-check/check"
@@ -87,3 +89,12 @@ func (s *DockerSuite) TestKillWithInvalidSignal(c *check.C) {
8789
c.Fatal("Container should be in running state after an invalid signal")
8890
}
8991
}
92+
93+
func (s *DockerSuite) TestKillofStoppedContainerAPIPre120(c *check.C) {
94+
dockerCmd(c, "run", "--name", "docker-kill-test-api", "-d", "busybox", "top")
95+
dockerCmd(c, "stop", "docker-kill-test-api")
96+
97+
status, _, err := sockRequest("POST", fmt.Sprintf("/v1.19/containers/%s/kill", "docker-kill-test-api"), nil)
98+
c.Assert(err, check.IsNil)
99+
c.Assert(status, check.Equals, http.StatusNoContent)
100+
}

0 commit comments

Comments
 (0)