Browse code

Revert `rm -f` deprecation use SIGKILL instead

`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)

Brian Goff authored on 2014/08/08 03:50:59
Showing 9 changed files
... ...
@@ -1016,8 +1016,7 @@ func (cli *DockerCli) CmdRm(args ...string) error {
1016 1016
 	cmd := cli.Subcmd("rm", "[OPTIONS] CONTAINER [CONTAINER...]", "Remove one or more containers")
1017 1017
 	v := cmd.Bool([]string{"v", "-volumes"}, false, "Remove the volumes associated with the container")
1018 1018
 	link := cmd.Bool([]string{"l", "#link", "-link"}, false, "Remove the specified link and not the underlying container")
1019
-	stop := cmd.Bool([]string{"#f", "s", "#-force", "-stop"}, false, "Stop and remove a running container")
1020
-	kill := cmd.Bool([]string{"k", "-kill"}, false, "Kill and remove a running container")
1019
+	force := cmd.Bool([]string{"f", "-force"}, false, "Force the removal of a running container (uses SIGKILL)")
1021 1020
 
1022 1021
 	if err := cmd.Parse(args); err != nil {
1023 1022
 		return nil
... ...
@@ -1026,9 +1025,7 @@ func (cli *DockerCli) CmdRm(args ...string) error {
1026 1026
 		cmd.Usage()
1027 1027
 		return nil
1028 1028
 	}
1029
-	if *stop && *kill {
1030
-		return fmt.Errorf("Conflicting options: -s/--stop and -k/--kill")
1031
-	}
1029
+
1032 1030
 	val := url.Values{}
1033 1031
 	if *v {
1034 1032
 		val.Set("v", "1")
... ...
@@ -1036,11 +1033,9 @@ func (cli *DockerCli) CmdRm(args ...string) error {
1036 1036
 	if *link {
1037 1037
 		val.Set("link", "1")
1038 1038
 	}
1039
-	if *stop {
1040
-		val.Set("stop", "1")
1041
-	}
1042
-	if *kill {
1043
-		val.Set("kill", "1")
1039
+
1040
+	if *force {
1041
+		val.Set("force", "1")
1044 1042
 	}
1045 1043
 
1046 1044
 	var encounteredError error
... ...
@@ -680,16 +680,8 @@ func deleteContainers(eng *engine.Engine, version version.Version, w http.Respon
680 680
 	}
681 681
 	job := eng.Job("delete", vars["name"])
682 682
 
683
-	if version.GreaterThanOrEqualTo("1.14") {
684
-		job.Setenv("stop", r.Form.Get("stop"))
685
-		job.Setenv("kill", r.Form.Get("kill"))
683
+	job.Setenv("forceRemove", r.Form.Get("force"))
686 684
 
687
-		if job.GetenvBool("stop") && job.GetenvBool("kill") {
688
-			return fmt.Errorf("Bad parameters: can't use stop and kill simultaneously")
689
-		}
690
-	} else {
691
-		job.Setenv("stop", r.Form.Get("force"))
692
-	}
693 685
 	job.Setenv("removeVolume", r.Form.Get("v"))
694 686
 	job.Setenv("removeLink", r.Form.Get("link"))
695 687
 	if err := job.Run(); err != nil {
... ...
@@ -474,30 +474,6 @@ func TestDeleteContainers(t *testing.T) {
474 474
 	}
475 475
 }
476 476
 
477
-func TestDeleteContainersWithStopAndKill(t *testing.T) {
478
-	if api.APIVERSION.LessThan("1.14") {
479
-		return
480
-	}
481
-	eng := engine.New()
482
-	var called bool
483
-	eng.Register("delete", func(job *engine.Job) engine.Status {
484
-		called = true
485
-		return engine.StatusOK
486
-	})
487
-	r := serveRequest("DELETE", "/containers/foo?stop=1&kill=1", nil, eng, t)
488
-	if r.Code != http.StatusBadRequest {
489
-		t.Fatalf("Got status %d, expected %d", r.Code, http.StatusBadRequest)
490
-	}
491
-	if called {
492
-		t.Fatalf("container_delete jobs was called, but it shouldn't")
493
-	}
494
-	res := strings.TrimSpace(r.Body.String())
495
-	expected := "Bad parameters: can't use stop and kill simultaneously"
496
-	if !strings.Contains(res, expected) {
497
-		t.Fatalf("Output %s, expected %s in it", res, expected)
498
-	}
499
-}
500
-
501 477
 func serveRequest(method, target string, body io.Reader, eng *engine.Engine, t *testing.T) *httptest.ResponseRecorder {
502 478
 	return serveRequestUsingVersion(method, target, api.APIVERSION, body, eng, t)
503 479
 }
... ...
@@ -20,9 +20,7 @@ func (daemon *Daemon) ContainerDestroy(job *engine.Job) engine.Status {
20 20
 	name := job.Args[0]
21 21
 	removeVolume := job.GetenvBool("removeVolume")
22 22
 	removeLink := job.GetenvBool("removeLink")
23
-	stop := job.GetenvBool("stop")
24
-	kill := job.GetenvBool("kill")
25
-
23
+	forceRemove := job.GetenvBool("forceRemove")
26 24
 	container := daemon.Get(name)
27 25
 
28 26
 	if removeLink {
... ...
@@ -55,11 +53,7 @@ func (daemon *Daemon) ContainerDestroy(job *engine.Job) engine.Status {
55 55
 
56 56
 	if container != nil {
57 57
 		if container.State.IsRunning() {
58
-			if stop {
59
-				if err := container.Stop(5); err != nil {
60
-					return job.Errorf("Could not stop running container, cannot remove - %v", err)
61
-				}
62
-			} else if kill {
58
+			if forceRemove {
63 59
 				if err := container.Kill(); err != nil {
64 60
 					return job.Errorf("Could not kill running container, cannot remove - %v", err)
65 61
 				}
... ...
@@ -6,9 +6,8 @@ docker-rm - Remove one or more containers
6 6
 
7 7
 # SYNOPSIS
8 8
 **docker rm**
9
-[**-k**|**--kill**[=*false*]]
10 9
 [**-l**|**--link**[=*false*]]
11
-[**-s**|**--stop**[=*false*]]
10
+[**-f**|**--force**[=*false*]]
12 11
 [**-v**|**--volumes**[=*false*]]
13 12
  CONTAINER [CONTAINER...]
14 13
 
... ...
@@ -20,15 +19,11 @@ remove a running container unless you use the \fB-f\fR option. To see all
20 20
 containers on a host use the **docker ps -a** command.
21 21
 
22 22
 # OPTIONS
23
-**-k**, **--kill**=*true*|*false*
24
-   Kill and remove a running container. The default is *false*.
25
-
26 23
 **-l**, **--link**=*true*|*false*
27 24
    Remove the specified link and not the underlying container. The default is *false*.
28 25
 
29
-**-s**, **--stop**=*true*|*false*
30
-   Stop and remove a running container. The default is *false*.
31
-
26
+**-f**, **--force**=*true*|*false*
27
+  Allows removing of a running container by first killing it with SIGKILL.
32 28
 **-v**, **--volumes**=*true*|*false*
33 29
    Remove the volumes associated with the container. The default is *false*.
34 30
 
... ...
@@ -37,11 +37,7 @@ You can still call an old version of the API using
37 37
 `DELETE /containers/(id)`
38 38
 
39 39
 **New!**
40
-You can now use the `stop` parameter to stop running containers before removal
41
-(replace `force`).
42
-
43
-**New!**
44
-You can now use the `kill` parameter to kill running containers before removal.
40
+When using `force`, the container will be immediately killed with SIGKILL.
45 41
 
46 42
 `POST /containers/(id)/start`
47 43
 
... ...
@@ -679,9 +679,7 @@ Remove the container `id` from the filesystem
679 679
 
680 680
     -   **v** – 1/True/true or 0/False/false, Remove the volumes
681 681
         associated to the container. Default false
682
-    -   **stop** – 1/True/true or 0/False/false, Stop then remove the container.
683
-        Default false
684
-    -   **kill** - 1/True/true or 0/False/false, Kill then remove the container.
682
+    -   **force** - 1/True/true or 0/False/false, Kill then remove the container.
685 683
         Default false
686 684
 
687 685
     Status Codes:
... ...
@@ -864,8 +864,7 @@ registry or to a self-hosted one.
864 864
 
865 865
     Remove one or more containers
866 866
 
867
-      -s, --stop=false       Stop and remove a running container
868
-      -k, --kill=false       Kill and remove a running container
867
+      -f, --force=false      Force removal of a running container. Uses SIGKILL to stop the container.
869 868
       -l, --link=false       Remove the specified link and not the underlying container
870 869
       -v, --volumes=false    Remove the volumes associated with the container
871 870
 
... ...
@@ -890,21 +889,12 @@ This will remove the underlying link between `/webapp`
890 890
 and the `/redis` containers removing all
891 891
 network communication.
892 892
 
893
-    $ sudo docker rm --stop redis
894
-    redis
895
- 
896
-The main process inside the container referenced under the link `/redis` will receive
897
-SIGTERM, and after a grace period, SIGKILL, then the container will be removed.
898
-
899
-    $ sudo docker rm --kill redis
893
+    $ sudo docker rm --force redis
900 894
     redis
901 895
 
902 896
 The main process inside the container referenced under the link `/redis` will receive
903 897
 SIGKILL, then the container will be removed.
904 898
 
905
-NOTE: If you try to use `stop` and `kill` simultaneously, Docker will return an error.
906
-
907
-    $ sudo docker rm $(docker ps -a -q)
908 899
 
909 900
 This command will delete all stopped containers. The command
910 901
 `docker ps -a -q` will return all existing container
... ...
@@ -57,40 +57,18 @@ func TestRemoveRunningContainer(t *testing.T) {
57 57
 	logDone("rm - running container")
58 58
 }
59 59
 
60
-func TestStopAndRemoveRunningContainer(t *testing.T) {
60
+func TestForceRemoveRunningContainer(t *testing.T) {
61 61
 	createRunningContainer(t, "foo")
62 62
 
63 63
 	// Stop then remove with -s
64
-	cmd := exec.Command(dockerBinary, "rm", "-s", "foo")
64
+	cmd := exec.Command(dockerBinary, "rm", "-f", "foo")
65 65
 	if _, err := runCommand(cmd); err != nil {
66 66
 		t.Fatal(err)
67 67
 	}
68 68
 
69 69
 	deleteAllContainers()
70 70
 
71
-	logDone("rm - running container with --stop=true")
72
-}
73
-
74
-func TestKillAndRemoveRunningContainer(t *testing.T) {
75
-	createRunningContainer(t, "foo")
76
-
77
-	// Kill then remove with -k
78
-	cmd := exec.Command(dockerBinary, "rm", "-k", "foo")
79
-	if _, err := runCommand(cmd); err != nil {
80
-		t.Fatal(err)
81
-	}
82
-
83
-	deleteAllContainers()
84
-
85
-	logDone("rm - running container with --kill=true")
86
-}
87
-
88
-func TestRemoveContainerWithStopAndKill(t *testing.T) {
89
-	cmd := exec.Command(dockerBinary, "rm", "-sk", "foo")
90
-	if _, err := runCommand(cmd); err == nil {
91
-		t.Fatalf("Expected error: can't use stop and kill simulteanously")
92
-	}
93
-	logDone("rm - with --stop=true and --kill=true")
71
+	logDone("rm - running container with --force=true")
94 72
 }
95 73
 
96 74
 func TestContainerOrphaning(t *testing.T) {