Browse code

Fix the rm error message when a container is restarting/paused

Running the rm command on a paused/restarting container
will give an error message saying the container is running
which is incorrect.

To fix that, the error message will have the correct
container state and a procedure to remove it accordingly.

Notice: docker-py was bumped to:
4a08d04aef0595322e1b5ac7c52f28a931da85a5

Signed-off-by: Boaz Shuster <ripcurld.github@gmail.com>

Boaz Shuster authored on 2017/02/09 19:48:33
Showing 5 changed files
... ...
@@ -191,10 +191,15 @@ RUN set -x \
191 191
 	&& rm -rf "$GOPATH"
192 192
 
193 193
 # Get the "docker-py" source so we can run their integration tests
194
-ENV DOCKER_PY_COMMIT e2655f658408f9ad1f62abdef3eb6ed43c0cf324
194
+ENV DOCKER_PY_COMMIT 4a08d04aef0595322e1b5ac7c52f28a931da85a5
195 195
 RUN git clone https://github.com/docker/docker-py.git /docker-py \
196 196
 	&& cd /docker-py \
197 197
 	&& git checkout -q $DOCKER_PY_COMMIT \
198
+	# To run integration tests docker-pycreds is required.
199
+	# Before running the integration tests conftest.py is
200
+	# loaded which results in loads auth.py that
201
+	# imports the docker-pycreds module.
202
+	&& pip install docker-pycreds==0.2.1 \
198 203
 	&& pip install -r test-requirements.txt
199 204
 
200 205
 # Install yamllint for validating swagger.yaml
... ...
@@ -142,11 +142,15 @@ RUN set -x \
142 142
 	&& rm -rf "$GOPATH"
143 143
 
144 144
 # Get the "docker-py" source so we can run their integration tests
145
-ENV DOCKER_PY_COMMIT e2655f658408f9ad1f62abdef3eb6ed43c0cf324
145
+ENV DOCKER_PY_COMMIT 4a08d04aef0595322e1b5ac7c52f28a931da85a5
146 146
 RUN git clone https://github.com/docker/docker-py.git /docker-py \
147 147
 	&& cd /docker-py \
148 148
 	&& git checkout -q $DOCKER_PY_COMMIT \
149 149
 	&& pip install wheel \
150
+	# Before running the integration tests conftest.py is
151
+	# loaded which results in loads auth.py that
152
+	# imports the docker-pycreds module.
153
+	&& pip install docker-pycreds==0.2.1 \
150 154
 	&& pip install -r test-requirements.txt
151 155
 
152 156
 # Install yamllint for validating swagger.yaml
... ...
@@ -4165,7 +4165,7 @@ paths:
4165 4165
             $ref: "#/definitions/ErrorResponse"
4166 4166
           examples:
4167 4167
             application/json:
4168
-              message: "You cannot remove a running container: c2ada9df5af8. Stop the container before attempting removal or use -f"
4168
+              message: "You cannot remove a running container: c2ada9df5af8. Stop the container before attempting removal or force remove"
4169 4169
         500:
4170 4170
           description: "server error"
4171 4171
           schema:
... ...
@@ -80,7 +80,12 @@ func (daemon *Daemon) rmLink(container *container.Container, name string) error
80 80
 func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemove, removeVolume bool) (err error) {
81 81
 	if container.IsRunning() {
82 82
 		if !forceRemove {
83
-			err := fmt.Errorf("You cannot remove a running container %s. Stop the container before attempting removal or use -f", container.ID)
83
+			state := container.StateString()
84
+			procedure := "Stop the container before attempting removal or force remove"
85
+			if state == "paused" {
86
+				procedure = "Unpause and then " + strings.ToLower(procedure)
87
+			}
88
+			err := fmt.Errorf("You cannot remove a %s container %s. %s", state, container.ID, procedure)
84 89
 			return apierrors.NewRequestConflictError(err)
85 90
 		}
86 91
 		if err := daemon.Kill(container); err != nil {
... ...
@@ -38,8 +38,10 @@ func (s *DockerSuite) TestRmContainerWithVolume(c *check.C) {
38 38
 func (s *DockerSuite) TestRmContainerRunning(c *check.C) {
39 39
 	createRunningContainer(c, "foo")
40 40
 
41
-	_, _, err := dockerCmdWithError("rm", "foo")
41
+	res, _, err := dockerCmdWithError("rm", "foo")
42 42
 	c.Assert(err, checker.NotNil, check.Commentf("Expected error, can't rm a running container"))
43
+	c.Assert(res, checker.Contains, "cannot remove a running container")
44
+	c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
43 45
 }
44 46
 
45 47
 func (s *DockerSuite) TestRmContainerForceRemoveRunning(c *check.C) {
... ...
@@ -83,3 +85,31 @@ func (s *DockerSuite) TestRmInvalidContainer(c *check.C) {
83 83
 func createRunningContainer(c *check.C, name string) {
84 84
 	runSleepingContainer(c, "-dt", "--name", name)
85 85
 }
86
+
87
+// #30842
88
+func (s *DockerSuite) TestRmRestartingContainer(c *check.C) {
89
+	name := "rst"
90
+	dockerCmd(c, "run", "--name", name, "--restart=always", "busybox", "date")
91
+
92
+	res, _, err := dockerCmdWithError("rm", name)
93
+	c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a restarting container, got none"))
94
+	c.Assert(res, checker.Contains, "cannot remove a restarting container")
95
+	c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
96
+	dockerCmd(c, "rm", "-f", name)
97
+}
98
+
99
+// #30842
100
+func (s *DockerSuite) TestRmPausedContainer(c *check.C) {
101
+	testRequires(c, IsPausable)
102
+	name := "psd"
103
+	dockerCmd(c, "run", "--name", name, "-d", "busybox", "sleep", "1m")
104
+	dockerCmd(c, "pause", name)
105
+
106
+	res, _, err := dockerCmdWithError("rm", name)
107
+	c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a paused container, got none"))
108
+	c.Assert(res, checker.Contains, "cannot remove a paused container")
109
+	c.Assert(res, checker.Contains, "Unpause and then stop the container before attempting removal or force remove")
110
+	unpauseContainer(c, name)
111
+	dockerCmd(c, "stop", name)
112
+	dockerCmd(c, "rm", name)
113
+}