From 621e3d8587bbee86b4e36d0b7822662bfbedd76c Mon Sep 17 00:00:00 2001
From: David Calavera <david.calavera@gmail.com>
Date: Tue, 28 Jul 2015 11:36:38 -0700
Subject: [PATCH] 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 <david.calavera@gmail.com>
---
 api/server/server.go                    |  8 +++++++-
 daemon/container.go                     | 18 +++++++++++++-----
 daemon/kill.go                          |  9 +++------
 integration-cli/docker_cli_kill_test.go | 11 +++++++++++
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/api/server/server.go b/api/server/server.go
index 162f60d4b6..2544d435b5 100644
--- a/api/server/server.go
+++ b/api/server/server.go
@@ -298,7 +298,13 @@ func (s *Server) postContainersKill(version version.Version, w http.ResponseWrit
 	}
 
 	if err := s.daemon.ContainerKill(name, sig); err != nil {
-		return err
+		_, isStopped := err.(daemon.ErrContainerNotRunning)
+		// Return error that's not caused because the container is stopped.
+		// Return error if the container is not running and the api is >= 1.20
+		// to keep backwards compatibility.
+		if version.GreaterThanOrEqualTo("1.20") || !isStopped {
+			return fmt.Errorf("Cannot kill container %s: %v", name, err)
+		}
 	}
 
 	w.WriteHeader(http.StatusNoContent)
diff --git a/daemon/container.go b/daemon/container.go
index 855900f061..dd8c753bdb 100644
--- a/daemon/container.go
+++ b/daemon/container.go
@@ -41,6 +41,14 @@ var (
 	ErrContainerRootfsReadonly = errors.New("container rootfs is marked read-only")
 )
 
+type ErrContainerNotRunning struct {
+	id string
+}
+
+func (e ErrContainerNotRunning) Error() string {
+	return fmt.Sprintf("Container %s is not running", e.id)
+}
+
 type StreamConfig struct {
 	stdout    *broadcastwriter.BroadcastWriter
 	stderr    *broadcastwriter.BroadcastWriter
@@ -361,7 +369,7 @@ func (container *Container) KillSig(sig int) error {
 	}
 
 	if !container.Running {
-		return fmt.Errorf("Container %s is not running", container.ID)
+		return ErrContainerNotRunning{container.ID}
 	}
 
 	// signal to the monitor that it should not restart the container
@@ -398,7 +406,7 @@ func (container *Container) Pause() error {
 
 	// We cannot Pause the container which is not running
 	if !container.Running {
-		return fmt.Errorf("Container %s is not running, cannot pause a non-running container", container.ID)
+		return ErrContainerNotRunning{container.ID}
 	}
 
 	// We cannot Pause the container which is already paused
@@ -420,7 +428,7 @@ func (container *Container) Unpause() error {
 
 	// We cannot unpause the container which is not running
 	if !container.Running {
-		return fmt.Errorf("Container %s is not running, cannot unpause a non-running container", container.ID)
+		return ErrContainerNotRunning{container.ID}
 	}
 
 	// We cannot unpause the container which is not paused
@@ -438,7 +446,7 @@ func (container *Container) Unpause() error {
 
 func (container *Container) Kill() error {
 	if !container.IsRunning() {
-		return fmt.Errorf("Container %s is not running", container.ID)
+		return ErrContainerNotRunning{container.ID}
 	}
 
 	// 1. Send SIGKILL
@@ -520,7 +528,7 @@ func (container *Container) Restart(seconds int) error {
 
 func (container *Container) Resize(h, w int) error {
 	if !container.IsRunning() {
-		return fmt.Errorf("Cannot resize container %s, container is not running", container.ID)
+		return ErrContainerNotRunning{container.ID}
 	}
 	if err := container.command.ProcessConfig.Terminal.Resize(h, w); err != nil {
 		return err
diff --git a/daemon/kill.go b/daemon/kill.go
index 3f7bb9bcfe..7a4d9ce8ac 100644
--- a/daemon/kill.go
+++ b/daemon/kill.go
@@ -1,9 +1,6 @@
 package daemon
 
-import (
-	"fmt"
-	"syscall"
-)
+import "syscall"
 
 // ContainerKill send signal to the container
 // 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 {
 	// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
 	if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
 		if err := container.Kill(); err != nil {
-			return fmt.Errorf("Cannot kill container %s: %s", name, err)
+			return err
 		}
 	} else {
 		// Otherwise, just send the requested signal
 		if err := container.KillSig(int(sig)); err != nil {
-			return fmt.Errorf("Cannot kill container %s: %s", name, err)
+			return err
 		}
 	}
 	return nil
diff --git a/integration-cli/docker_cli_kill_test.go b/integration-cli/docker_cli_kill_test.go
index 6f96db00b6..505d4a4f5f 100644
--- a/integration-cli/docker_cli_kill_test.go
+++ b/integration-cli/docker_cli_kill_test.go
@@ -1,6 +1,8 @@
 package main
 
 import (
+	"fmt"
+	"net/http"
 	"strings"
 
 	"github.com/go-check/check"
@@ -87,3 +89,12 @@ func (s *DockerSuite) TestKillWithInvalidSignal(c *check.C) {
 		c.Fatal("Container should be in running state after an invalid signal")
 	}
 }
+
+func (s *DockerSuite) TestKillofStoppedContainerAPIPre120(c *check.C) {
+	dockerCmd(c, "run", "--name", "docker-kill-test-api", "-d", "busybox", "top")
+	dockerCmd(c, "stop", "docker-kill-test-api")
+
+	status, _, err := sockRequest("POST", fmt.Sprintf("/v1.19/containers/%s/kill", "docker-kill-test-api"), nil)
+	c.Assert(err, check.IsNil)
+	c.Assert(status, check.Equals, http.StatusNoContent)
+}