From 2759194d44634b88d7a39134a24a61ca8d60c0ab Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Apr 2017 12:10:39 +0200 Subject: [PATCH] create unit tests for rm (running, paused, restarting) errormessages These integration tests were basically testing if a decent error message was printed when attempting to remove a running, paused, or restarting container. Moving these tests to a unit-test to make the tests not flaky (especially on the "restarting" container test). Signed-off-by: Sebastiaan van Stijn --- daemon/delete_test.go | 93 +++++++++++++++++++++++---- integration-cli/docker_cli_rm_test.go | 42 ------------ 2 files changed, 80 insertions(+), 55 deletions(-) diff --git a/daemon/delete_test.go b/daemon/delete_test.go index 1fd27e1ffa..21b4696a6d 100644 --- a/daemon/delete_test.go +++ b/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)) } diff --git a/integration-cli/docker_cli_rm_test.go b/integration-cli/docker_cli_rm_test.go index 6ade39bd84..d281704a7b 100644 --- a/integration-cli/docker_cli_rm_test.go +++ b/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) -}