Skip to content

Commit

Permalink
Revert rm -f deprecation use SIGKILL instead
Browse files Browse the repository at this point in the history
`rm -f` was originally deprecated in favor of `rm --stop/--kill` since `rm
-f` was sending SIGTERM and potentially very slow.
Instead this will bring back `rm -f` but use SIGKILL isntead

Docker-DCO-1.1-Signed-off-by: Brian Goff <cpuguy83@gmail.com> (github: cpuguy83)
  • Loading branch information
cpuguy83 committed Aug 7, 2014
1 parent 140e6ab commit 95f86da
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 104 deletions.
15 changes: 5 additions & 10 deletions api/client/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,8 +1016,7 @@ 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")
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")
force := cmd.Bool([]string{"f", "-force"}, false, "Force the removal of a running container (uses SIGKILL)")

if err := cmd.Parse(args); err != nil {
return nil
Expand All @@ -1026,21 +1025,17 @@ 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")
}
if *link {
val.Set("link", "1")
}
if *stop {
val.Set("stop", "1")
}
if *kill {
val.Set("kill", "1")

if *force {
val.Set("force", "1")
}

var encounteredError error
Expand Down
10 changes: 1 addition & 9 deletions api/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,16 +680,8 @@ func deleteContainers(eng *engine.Engine, version version.Version, w http.Respon
}
job := eng.Job("delete", vars["name"])

if version.GreaterThanOrEqualTo("1.14") {
job.Setenv("stop", r.Form.Get("stop"))
job.Setenv("kill", r.Form.Get("kill"))
job.Setenv("forceRemove", r.Form.Get("force"))

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"))
if err := job.Run(); err != nil {
Expand Down
24 changes: 0 additions & 24 deletions api/server/server_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,30 +474,6 @@ func TestDeleteContainers(t *testing.T) {
}
}

func TestDeleteContainersWithStopAndKill(t *testing.T) {
if api.APIVERSION.LessThan("1.14") {
return
}
eng := engine.New()
var called bool
eng.Register("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)
}
Expand Down
10 changes: 2 additions & 8 deletions daemon/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ func (daemon *Daemon) ContainerDestroy(job *engine.Job) engine.Status {
name := job.Args[0]
removeVolume := job.GetenvBool("removeVolume")
removeLink := job.GetenvBool("removeLink")
stop := job.GetenvBool("stop")
kill := job.GetenvBool("kill")

forceRemove := job.GetenvBool("forceRemove")
container := daemon.Get(name)

if removeLink {
Expand Down Expand Up @@ -55,11 +53,7 @@ func (daemon *Daemon) ContainerDestroy(job *engine.Job) engine.Status {

if container != nil {
if container.State.IsRunning() {
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 forceRemove {
if err := container.Kill(); err != nil {
return job.Errorf("Could not kill running container, cannot remove - %v", err)
}
Expand Down
11 changes: 3 additions & 8 deletions docs/man/docker-rm.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ docker-rm - Remove one or more containers

# SYNOPSIS
**docker rm**
[**-k**|**--kill**[=*false*]]
[**-l**|**--link**[=*false*]]
[**-s**|**--stop**[=*false*]]
[**-f**|**--force**[=*false*]]
[**-v**|**--volumes**[=*false*]]
CONTAINER [CONTAINER...]

Expand All @@ -20,15 +19,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
**-k**, **--kill**=*true*|*false*
Kill and 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*.

**-s**, **--stop**=*true*|*false*
Stop and remove a running container. The default is *false*.

**-f**, **--force**=*true*|*false*
Allows removing of a running container by first killing it with SIGKILL.
**-v**, **--volumes**=*true*|*false*
Remove the volumes associated with the container. The default is *false*.

Expand Down
6 changes: 1 addition & 5 deletions docs/sources/reference/api/docker_remote_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ You can still call an old version of the API using
`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.
When using `force`, the container will be immediately killed with SIGKILL.

`POST /containers/(id)/start`

Expand Down
4 changes: 1 addition & 3 deletions docs/sources/reference/api/docker_remote_api_v1.14.md
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,7 @@ Remove the container `id` from the filesystem

- **v** – 1/True/true or 0/False/false, Remove the volumes
associated to the container. Default false
- **stop** – 1/True/true or 0/False/false, Stop then remove the container.
Default false
- **kill** - 1/True/true or 0/False/false, Kill then remove the container.
- **force** - 1/True/true or 0/False/false, Kill then remove the container.
Default false

Status Codes:
Expand Down
14 changes: 2 additions & 12 deletions docs/sources/reference/commandline/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -864,8 +864,7 @@ registry or to a self-hosted one.

Remove one or more containers

-s, --stop=false Stop and remove a running container
-k, --kill=false Kill and remove a running container
-f, --force=false Force removal of a running container. Uses SIGKILL to stop the container.
-l, --link=false Remove the specified link and not the underlying container
-v, --volumes=false Remove the volumes associated with the container

Expand All @@ -890,21 +889,12 @@ 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
$ sudo docker rm --force 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
`docker ps -a -q` will return all existing container
Expand Down
28 changes: 3 additions & 25 deletions integration-cli/docker_cli_rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,40 +57,18 @@ func TestRemoveRunningContainer(t *testing.T) {
logDone("rm - running container")
}

func TestStopAndRemoveRunningContainer(t *testing.T) {
func TestForceRemoveRunningContainer(t *testing.T) {
createRunningContainer(t, "foo")

// Stop then remove with -s
cmd := exec.Command(dockerBinary, "rm", "-s", "foo")
cmd := exec.Command(dockerBinary, "rm", "-f", "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")
logDone("rm - running container with --force=true")
}

func TestContainerOrphaning(t *testing.T) {
Expand Down

0 comments on commit 95f86da

Please sign in to comment.