Merge pull request #6987 from vieux/fix_6863

Stop & Kill options for containers removal

api/client/commands.go

@@ -983,7 +983,8 @@ func (cli *DockerCli) CmdRm(args ...string) error {
 	cmd := cli.Subcmd("rm", "[OPTIONS] CONTAINER [CONTAINER...]", "Remove one or more containers")
 	v := cmd.Bool([]string{"v", "-volumes"}, false, "Remove the volumes associated with the container")
 	link := cmd.Bool([]string{"l", "#link", "-link"}, false, "Remove the specified link and not the underlying container")
-	force := cmd.Bool([]string{"f", "-force"}, false, "Force removal of running container")
+	stop := cmd.Bool([]string{"#f", "s", "#-force", "-stop"}, false, "Stop and remove a running container")
+	kill := cmd.Bool([]string{"k", "-kill"}, false, "Kill and remove a running container")
 
 	if err := cmd.Parse(args); err != nil {
 		return nil
@@ -992,6 +993,9 @@ func (cli *DockerCli) CmdRm(args ...string) error {
 		cmd.Usage()
 		return nil
 	}
+	if *stop && *kill {
+		return fmt.Errorf("Conflicting options: -s/--stop and -k/--kill")
+	}
 	val := url.Values{}
 	if *v {
 		val.Set("v", "1")
@@ -999,8 +1003,11 @@ func (cli *DockerCli) CmdRm(args ...string) error {
 	if *link {
 		val.Set("link", "1")
 	}
-	if *force {
-		val.Set("force", "1")
+	if *stop {
+		val.Set("stop", "1")
+	}
+	if *kill {
+		val.Set("kill", "1")
 	}
 
 	var encounteredError error

api/server/server.go

@@ -678,9 +678,19 @@ func deleteContainers(eng *engine.Engine, version version.Version, w http.Respon
 		return fmt.Errorf("Missing parameter")
 	}
 	job := eng.Job("container_delete", vars["name"])
+
+	if version.GreaterThanOrEqualTo("1.14") {
+		job.Setenv("stop", r.Form.Get("stop"))
+		job.Setenv("kill", r.Form.Get("kill"))
+
+		if job.GetenvBool("stop") && job.GetenvBool("kill") {
+			return fmt.Errorf("Bad parameters: can't use stop and kill simultaneously")
+		}
+	} else {
+		job.Setenv("stop", r.Form.Get("force"))
+	}
 	job.Setenv("removeVolume", r.Form.Get("v"))
 	job.Setenv("removeLink", r.Form.Get("link"))
-	job.Setenv("forceRemove", r.Form.Get("force"))
 	if err := job.Run(); err != nil {
 		return err
 	}

api/server/server_unit_test.go

@@ -451,6 +451,53 @@ func TestGetImagesByName(t *testing.T) {
 	}
 }
 
+func TestDeleteContainers(t *testing.T) {
+	eng := engine.New()
+	name := "foo"
+	var called bool
+	eng.Register("container_delete", func(job *engine.Job) engine.Status {
+		called = true
+		if len(job.Args) == 0 {
+			t.Fatalf("Job arguments is empty")
+		}
+		if job.Args[0] != name {
+			t.Fatalf("name != '%s': %#v", name, job.Args[0])
+		}
+		return engine.StatusOK
+	})
+	r := serveRequest("DELETE", "/containers/"+name, nil, eng, t)
+	if !called {
+		t.Fatalf("handler was not called")
+	}
+	if r.Code != http.StatusNoContent {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusNoContent)
+	}
+}
+
+func TestDeleteContainersWithStopAndKill(t *testing.T) {
+	if api.APIVERSION.LessThan("1.14") {
+		return
+	}
+	eng := engine.New()
+	var called bool
+	eng.Register("container_delete", func(job *engine.Job) engine.Status {
+		called = true
+		return engine.StatusOK
+	})
+	r := serveRequest("DELETE", "/containers/foo?stop=1&kill=1", nil, eng, t)
+	if r.Code != http.StatusBadRequest {
+		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusBadRequest)
+	}
+	if called {
+		t.Fatalf("container_delete jobs was called, but it shouldn't")
+	}
+	res := strings.TrimSpace(r.Body.String())
+	expected := "Bad parameters: can't use stop and kill simultaneously"
+	if !strings.Contains(res, expected) {
+		t.Fatalf("Output %s, expected %s in it", res, expected)
+	}
+}
+
 func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t *testing.T) *httptest.ResponseRecorder {
 	return serveRequestUsingVersion(method, target, api.APIVERSION, body, eng, t)
 }

docs/man/docker-rm.1.md

@@ -6,7 +6,8 @@ docker-rm - Remove one or more containers
 
 # SYNOPSIS
 **docker rm**
-[**-f**|**--force**[=*false*]]
+[**-s**|**--stop**[=*false*]]
+[**-k**|**--kill**[=*false*]]
 [**-l**|**--link**[=*false*]]
 [**-v**|**--volumes**[=*false*]]
  CONTAINER [CONTAINER...]
@@ -19,8 +20,11 @@ remove a running container unless you use the \fB-f\fR option. To see all
 containers on a host use the **docker ps -a** command.
 
 # OPTIONS
-**-f**, **--force**=*true*|*false*
-   Force removal of running container. The default is *false*.
+**-s**, **--stop**=*true*|*false*
+   Stop then remove a running container. The default is *false*.
+
+**-k**, **--kill**=*true*|*false*
+   Kill then remove a running container. The default is *false*.
 
 **-l**, **--link**=*true*|*false*
    Remove the specified link and not the underlying container. The default is *false*.

docs/sources/reference/api/docker_remote_api.md

@@ -34,6 +34,15 @@ You can still call an old version of the API using
 
 ### What's new
 
+`DELETE /containers/(id)`
+
+**New!**
+You can now use the `stop` parameter to stop running containers before removal
+(replace `force`).
+
+**New!**
+You can now use the `kill` parameter to kill running containers before removal.
+
 ## v1.13
 
 ### Full Documentation

docs/sources/reference/commandline/cli.md

@@ -838,7 +838,8 @@ registry or to a self-hosted one.
 
     Remove one or more containers
 
-      -f, --force=false      Force removal of running container
+      -s, --stop=false       Stop and remove a running container
+      -k, --kill=false       Kill and remove a running container
       -l, --link=false       Remove the specified link and not the underlying container
       -v, --volumes=false    Remove the volumes associated with the container
 
@@ -863,6 +864,20 @@ This will remove the underlying link between `/webapp`
 and the `/redis` containers removing all
 network communication.
 
+    $ sudo docker rm --stop redis
+    redis
+ 
+The main process inside the container referenced under the link `/redis` will receive
+SIGTERM, and after a grace period, SIGKILL, then the container will be removed.
+
+    $ sudo docker rm --kill redis
+    redis
+
+The main process inside the container referenced under the link `/redis` will receive
+SIGKILL, then the container will be removed.
+
+NOTE: If you try to use `stop` and `kill` simultaneously, Docker will return an error.
+
     $ sudo docker rm $(docker ps -a -q)
 
 This command will delete all stopped containers. The command

integration-cli/docker_cli_rm_test.go

@@ -42,25 +42,59 @@ func TestRemoveContainerWithVolume(t *testing.T) {
 	logDone("rm - volume")
 }
 
-func TestRemoveContainerRunning(t *testing.T) {
-	cmd := exec.Command(dockerBinary, "run", "-dt", "--name", "foo", "busybox", "top")
-	if _, err := runCommand(cmd); err != nil {
-		t.Fatal(err)
-	}
+func TestRemoveRunningContainer(t *testing.T) {
+	createRunningContainer(t, "foo")
 
 	// Test cannot remove running container
-	cmd = exec.Command(dockerBinary, "rm", "foo")
+	cmd := exec.Command(dockerBinary, "rm", "foo")
 	if _, err := runCommand(cmd); err == nil {
 		t.Fatalf("Expected error, can't rm a running container")
 	}
 
-	// Remove with -f
-	cmd = exec.Command(dockerBinary, "rm", "-f", "foo")
-	if _, err := runCommand(cmd); err != nil {
-		t.Fatal(err)
-	}
-
 	deleteAllContainers()
 
 	logDone("rm - running container")
 }
+
+func TestStopAndRemoveRunningContainer(t *testing.T) {
+	createRunningContainer(t, "foo")
+
+	// Stop then remove with -s
+	cmd := exec.Command(dockerBinary, "rm", "-s", "foo")
+	if _, err := runCommand(cmd); err != nil {
+		t.Fatal(err)
+	}
+
+	deleteAllContainers()
+
+	logDone("rm - running container with --stop=true")
+}
+
+func TestKillAndRemoveRunningContainer(t *testing.T) {
+	createRunningContainer(t, "foo")
+
+	// Kill then remove with -k
+	cmd := exec.Command(dockerBinary, "rm", "-k", "foo")
+	if _, err := runCommand(cmd); err != nil {
+		t.Fatal(err)
+	}
+
+	deleteAllContainers()
+
+	logDone("rm - running container with --kill=true")
+}
+
+func TestRemoveContainerWithStopAndKill(t *testing.T) {
+	cmd := exec.Command(dockerBinary, "rm", "-sk", "foo")
+	if _, err := runCommand(cmd); err == nil {
+		t.Fatalf("Expected error: can't use stop and kill simulteanously")
+	}
+	logDone("rm - with --stop=true and --kill=true")
+}
+
+func createRunningContainer(t *testing.T, name string) {
+	cmd := exec.Command(dockerBinary, "run", "-dt", "--name", name, "busybox", "top")
+	if _, err := runCommand(cmd); err != nil {
+		t.Fatal(err)
+	}
+}

integration/api_test.go

@@ -768,35 +768,6 @@ func TestPostContainersAttachStderr(t *testing.T) {
 	containerWait(eng, containerID, t)
 }
 
-// FIXME: Test deleting running container
-// FIXME: Test deleting container with volume
-// FIXME: Test deleting volume in use by other container
-func TestDeleteContainers(t *testing.T) {
-	eng := NewTestEngine(t)
-	defer mkDaemonFromEngine(eng, t).Nuke()
-
-	containerID := createTestContainer(eng,
-		&runconfig.Config{
-			Image: unitTestImageID,
-			Cmd:   []string{"touch", "/test"},
-		},
-		t,
-	)
-	req, err := http.NewRequest("DELETE", "/containers/"+containerID, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
-	assertHttpNotError(r, t)
-	if r.Code != http.StatusNoContent {
-		t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code)
-	}
-	containerAssertNotExists(eng, containerID, t)
-}
-
 func TestOptionsRoute(t *testing.T) {
 	eng := NewTestEngine(t)
 	defer mkDaemonFromEngine(eng, t).Nuke()

server/server.go

@@ -1787,7 +1787,8 @@ func (srv *Server) ContainerDestroy(job *engine.Job) engine.Status {
 	name := job.Args[0]
 	removeVolume := job.GetenvBool("removeVolume")
 	removeLink := job.GetenvBool("removeLink")
-	forceRemove := job.GetenvBool("forceRemove")
+	stop := job.GetenvBool("stop")
+	kill := job.GetenvBool("kill")
 
 	container := srv.daemon.Get(name)
 
@@ -1821,12 +1822,16 @@ func (srv *Server) ContainerDestroy(job *engine.Job) engine.Status {
 
 	if container != nil {
 		if container.State.IsRunning() {
-			if forceRemove {
+			if stop {
 				if err := container.Stop(5); err != nil {
 					return job.Errorf("Could not stop running container, cannot remove - %v", err)
 				}
+			} else if kill {
+				if err := container.Kill(); err != nil {
+					return job.Errorf("Could not kill running container, cannot remove - %v", err)
+				}
 			} else {
-				return job.Errorf("Impossible to remove a running container, please stop it first or use -f")
+				return job.Errorf("You cannot remove a running container. Stop the container before attempting removal or use -s or -k")
 			}
 		}
 		if err := srv.daemon.Destroy(container); err != nil {