Merge pull request #32312 from thaJeztah/unit-test-remove-errormessage

create unit tests for rm (running, paused, restarting) errormessages

daemon/delete_test.go

@@ -9,35 +9,102 @@ import (
 	"github.com/docker/docker/api/types"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
+	"github.com/docker/docker/pkg/testutil/assert"
 )
 
-func TestContainerDoubleDelete(t *testing.T) {
+func newDaemonWithTmpRoot(t *testing.T) (*Daemon, func()) {
 	tmp, err := ioutil.TempDir("", "docker-daemon-unix-test-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(tmp)
-	daemon := &Daemon{
+	assert.NilError(t, err)
+	d := &Daemon{
 		repository: tmp,
 		root:       tmp,
 	}
-	daemon.containers = container.NewMemoryStore()
+	d.containers = container.NewMemoryStore()
+	return d, func() { os.RemoveAll(tmp) }
+}
 
-	container := &container.Container{
+// TestContainerDeletePaused tests that a useful error message and instructions is given when attempting
+// to remove a paused container (#30842)
+func TestContainerDeletePaused(t *testing.T) {
+	c := &container.Container{
+		CommonContainer: container.CommonContainer{
+			ID:     "test",
+			State:  &container.State{Paused: true, Running: true},
+			Config: &containertypes.Config{},
+		},
+	}
+
+	d, cleanup := newDaemonWithTmpRoot(t)
+	defer cleanup()
+	d.containers.Add(c.ID, c)
+
+	err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
+
+	assert.Error(t, err, "cannot remove a paused container")
+	assert.Error(t, err, "Unpause and then stop the container before attempting removal or force remove")
+}
+
+// TestContainerDeleteRestarting tests that a useful error message and instructions is given when attempting
+// to remove a container that is restarting (#30842)
+func TestContainerDeleteRestarting(t *testing.T) {
+	c := &container.Container{
+		CommonContainer: container.CommonContainer{
+			ID:     "test",
+			State:  container.NewState(),
+			Config: &containertypes.Config{},
+		},
+	}
+
+	c.SetRunning(0, true)
+	c.SetRestarting(&container.ExitStatus{})
+
+	d, cleanup := newDaemonWithTmpRoot(t)
+	defer cleanup()
+	d.containers.Add(c.ID, c)
+
+	err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
+	assert.Error(t, err, "cannot remove a restarting container")
+	assert.Error(t, err, "Stop the container before attempting removal or force remove")
+}
+
+// TestContainerDeleteRunning tests that a useful error message and instructions is given when attempting
+// to remove a running container (#30842)
+func TestContainerDeleteRunning(t *testing.T) {
+	c := &container.Container{
+		CommonContainer: container.CommonContainer{
+			ID:     "test",
+			State:  &container.State{Running: true},
+			Config: &containertypes.Config{},
+		},
+	}
+
+	d, cleanup := newDaemonWithTmpRoot(t)
+	defer cleanup()
+	d.containers.Add(c.ID, c)
+
+	err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: false})
+	assert.Error(t, err, "cannot remove a running container")
+	assert.Error(t, err, "Stop the container before attempting removal or force remove")
+}
+
+func TestContainerDoubleDelete(t *testing.T) {
+	c := &container.Container{
 		CommonContainer: container.CommonContainer{
 			ID:     "test",
 			State:  container.NewState(),
 			Config: &containertypes.Config{},
 		},
 	}
-	daemon.containers.Add(container.ID, container)
 
 	// Mark the container as having a delete in progress
-	container.SetRemovalInProgress()
+	c.SetRemovalInProgress()
+
+	d, cleanup := newDaemonWithTmpRoot(t)
+	defer cleanup()
+	d.containers.Add(c.ID, c)
 
 	// Try to remove the container when its state is removalInProgress.
 	// It should return an error indicating it is under removal progress.
-	if err := daemon.ContainerRm(container.ID, &types.ContainerRmConfig{ForceRemove: true}); err == nil {
-		t.Fatalf("expected err: %v, got nil", fmt.Sprintf("removal of container %s is already in progress", container.ID))
-	}
+	err := d.ContainerRm(c.ID, &types.ContainerRmConfig{ForceRemove: true})
+	assert.Error(t, err, fmt.Sprintf("removal of container %s is already in progress", c.ID))
 }

integration-cli/docker_cli_rm_test.go

@@ -3,8 +3,6 @@ package main
 import (
 	"io/ioutil"
 	"os"
-	"strings"
-	"time"
 
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli/build"
@@ -44,7 +42,6 @@ func (s *DockerSuite) TestRmContainerRunning(c *check.C) {
 	res, _, err := dockerCmdWithError("rm", "foo")
 	c.Assert(err, checker.NotNil, check.Commentf("Expected error, can't rm a running container"))
 	c.Assert(res, checker.Contains, "cannot remove a running container")
-	c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
 }
 
 func (s *DockerSuite) TestRmContainerForceRemoveRunning(c *check.C) {
@@ -88,42 +85,3 @@ func (s *DockerSuite) TestRmInvalidContainer(c *check.C) {
 func createRunningContainer(c *check.C, name string) {
 	runSleepingContainer(c, "-dt", "--name", name)
 }
-
-// #30842
-func (s *DockerSuite) TestRmRestartingContainer(c *check.C) {
-	name := "rst"
-	dockerCmd(c, "run", "--name", name, "--restart=always", "busybox", "date")
-
-	res, _, err := dockerCmdWithError("rm", name)
-
-	for strings.Contains(res, "cannot remove a running container") {
-		// The `date` command hasn't exited yet. It should end
-		// up in a "restarting" state if we wait a bit, though
-		// it is possible that it might be running again by
-		// that time. The wait period between each restart
-		// increases though so we just loop in this condition.
-		time.Sleep(100 * time.Millisecond)
-		res, _, err = dockerCmdWithError("rm", name)
-	}
-
-	c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a restarting container, got none"))
-	c.Assert(res, checker.Contains, "cannot remove a restarting container")
-	c.Assert(res, checker.Contains, "Stop the container before attempting removal or force remove")
-	dockerCmd(c, "rm", "-f", name)
-}
-
-// #30842
-func (s *DockerSuite) TestRmPausedContainer(c *check.C) {
-	testRequires(c, IsPausable)
-	name := "psd"
-	dockerCmd(c, "run", "--name", name, "-d", "busybox", "sleep", "1m")
-	dockerCmd(c, "pause", name)
-
-	res, _, err := dockerCmdWithError("rm", name)
-	c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm a paused container, got none"))
-	c.Assert(res, checker.Contains, "cannot remove a paused container")
-	c.Assert(res, checker.Contains, "Unpause and then stop the container before attempting removal or force remove")
-	unpauseContainer(c, name)
-	dockerCmd(c, "stop", name)
-	dockerCmd(c, "rm", name)
-}