Skip to content

Commit 53c7995

Browse files
authored
Merge pull request moby#32312 from thaJeztah/unit-test-remove-errormessage
create unit tests for rm (running, paused, restarting) errormessages
2 parents 21ec12b + 2759194 commit 53c7995

2 files changed

Lines changed: 80 additions & 55 deletions

File tree

daemon/delete_test.go

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,35 +9,102 @@ import (
99
"github.com/docker/docker/api/types"
1010
containertypes "github.com/docker/docker/api/types/container"
1111
"github.com/docker/docker/container"
12+
"github.com/docker/docker/pkg/testutil/assert"
1213
)
1314

14-
func TestContainerDoubleDelete(t *testing.T) {
15+
func newDaemonWithTmpRoot(t *testing.T) (*Daemon, func()) {
1516
tmp, err := ioutil.TempDir("", "docker-daemon-unix-test-")
16-
if err != nil {
17-
t.Fatal(err)
18-
}
19-
defer os.RemoveAll(tmp)
20-
daemon := &Daemon{
17+
assert.NilError(t, err)
18+
d := &Daemon{
2119
repository: tmp,
2220
root: tmp,
2321
}
24-
daemon.containers = container.NewMemoryStore()
22+
d.containers = container.NewMemoryStore()
23+
return d, func() { os.RemoveAll(tmp) }
24+
}
25+
26+
// TestContainerDeletePaused tests that a useful error message and instructions is given when attempting
27+
// to remove a paused container (#30842)
28+
func TestContainerDeletePaused(t *testing.T) {
29+
c := &container.Container{
30+
CommonContainer: container.CommonContainer{
31+
ID: "test",
32+
State: &container.State{Paused: true, Running: true},
33+
Config: &containertypes.Config{},
34+
},
35+
}
36+
37+
d, cleanup := newDaemonWithTmpRoot(t)
38+
defer cleanup()
39+
d.containers.Add(c.ID, c)
40+
41+
err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
42+
43+
assert.Error(t, err, "cannot remove a paused container")
44+
assert.Error(t, err, "Unpause and then stop the container before attempting removal or force remove")
45+
}
46+
47+
// TestContainerDeleteRestarting tests that a useful error message and instructions is given when attempting
48+
// to remove a container that is restarting (#30842)
49+
func TestContainerDeleteRestarting(t *testing.T) {
50+
c := &container.Container{
51+
CommonContainer: container.CommonContainer{
52+
ID: "test",
53+
State: container.NewState(),
54+
Config: &containertypes.Config{},
55+
},
56+
}
57+
58+
c.SetRunning(0, true)
59+
c.SetRestarting(&container.ExitStatus{})
60+
61+
d, cleanup := newDaemonWithTmpRoot(t)
62+
defer cleanup()
63+
d.containers.Add(c.ID, c)
64+
65+
err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
66+
assert.Error(t, err, "cannot remove a restarting container")
67+
assert.Error(t, err, "Stop the container before attempting removal or force remove")
68+
}
69+
70+
// TestContainerDeleteRunning tests that a useful error message and instructions is given when attempting
71+
// to remove a running container (#30842)
72+
func TestContainerDeleteRunning(t *testing.T) {
73+
c := &container.Container{
74+
CommonContainer: container.CommonContainer{
75+
ID: "test",
76+
State: &container.State{Running: true},
77+
Config: &containertypes.Config{},
78+
},
79+
}
2580

26-
container := &container.Container{
81+
d, cleanup := newDaemonWithTmpRoot(t)
82+
defer cleanup()
83+
d.containers.Add(c.ID, c)
84+
85+
err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
86+
assert.Error(t, err, "cannot remove a running container")
87+
assert.Error(t, err, "Stop the container before attempting removal or force remove")
88+
}
89+
90+
func TestContainerDoubleDelete(t *testing.T) {
91+
c := &container.Container{
2792
CommonContainer: container.CommonContainer{
2893
ID: "test",
2994
State: container.NewState(),
3095
Config: &containertypes.Config{},
3196
},
3297
}
33-
daemon.containers.Add(container.ID, container)
3498

3599
// Mark the container as having a delete in progress
36-
container.SetRemovalInProgress()
100+
c.SetRemovalInProgress()
101+
102+
d, cleanup := newDaemonWithTmpRoot(t)
103+
defer cleanup()
104+
d.containers.Add(c.ID, c)
37105

38106
// Try to remove the container when its state is removalInProgress.
39107
// It should return an error indicating it is under removal progress.
40-
if err := daemon.ContainerRm(container.ID, &types.ContainerRmConfig{ForceRemove: true}); err == nil {
41-
t.Fatalf("expected err: %v, got nil", fmt.Sprintf("removal of container %s is already in progress", container.ID))
42-
}
108+
err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: true})
109+
assert.Error(t, err, fmt.Sprintf("removal of container %s is already in progress", c.ID))
43110
}

integration-cli/docker_cli_rm_test.go

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package main
33
import (
44
"io/ioutil"
55
"os"
6-
"strings"
7-
"time"
86

97
"github.com/docker/docker/integration-cli/checker"
108
"github.com/docker/docker/integration-cli/cli/build"
@@ -44,7 +42,6 @@ func (s *DockerSuite) TestRmContainerRunning(c *check.C) {
4442
res, _, err := dockerCmdWithError("rm", "foo")
4543
c.Assert(err, checker.NotNil, check.Commentf("Expected error, can't rm a running container"))
4644
c.Assert(res, checker.Contains, "cannot remove a running container")
47-
c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
4845
}
4946

5047
func (s *DockerSuite) TestRmContainerForceRemoveRunning(c *check.C) {
@@ -88,42 +85,3 @@ func (s *DockerSuite) TestRmInvalidContainer(c *check.C) {
8885
func createRunningContainer(c *check.C, name string) {
8986
runSleepingContainer(c, "-dt", "--name", name)
9087
}
91-
92-
// #30842
93-
func (s *DockerSuite) TestRmRestartingContainer(c *check.C) {
94-
name := "rst"
95-
dockerCmd(c, "run", "--name", name, "--restart=always", "busybox", "date")
96-
97-
res, _, err := dockerCmdWithError("rm", name)
98-
99-
for strings.Contains(res, "cannot remove a running container") {
100-
// The `date` command hasn't exited yet. It should end
101-
// up in a "restarting" state if we wait a bit, though
102-
// it is possible that it might be running again by
103-
// that time. The wait period between each restart
104-
// increases though so we just loop in this condition.
105-
time.Sleep(100 * time.Millisecond)
106-
res, _, err = dockerCmdWithError("rm", name)
107-
}
108-
109-
c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a restarting container, got none"))
110-
c.Assert(res, checker.Contains, "cannot remove a restarting container")
111-
c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
112-
dockerCmd(c, "rm", "-f", name)
113-
}
114-
115-
// #30842
116-
func (s *DockerSuite) TestRmPausedContainer(c *check.C) {
117-
testRequires(c, IsPausable)
118-
name := "psd"
119-
dockerCmd(c, "run", "--name", name, "-d", "busybox", "sleep", "1m")
120-
dockerCmd(c, "pause", name)
121-
122-
res, _, err := dockerCmdWithError("rm", name)
123-
c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a paused container, got none"))
124-
c.Assert(res, checker.Contains, "cannot remove a paused container")
125-
c.Assert(res, checker.Contains, "Unpause and then stop the container before attempting removal or force remove")
126-
unpauseContainer(c, name)
127-
dockerCmd(c, "stop", name)
128-
dockerCmd(c, "rm", name)
129-
}

0 commit comments

Comments
 (0)