From 5e7eade1f74ca8305dc3760687a44aa4b2373a3b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 8 Sep 2023 20:45:45 +0200 Subject: [PATCH] integration: don't poll for containers to be running container.Run() should be a synchronous operation in normal circumstances; the container is created and started, so polling after that for the container to be in the "running" state should not be needed. This should also prevent issues when a container (for whatever reason) exited immediately after starting; in that case we would continue polling for it to be running (which likely would never happen). Let's skip the polling; if the container is not in the expected state (i.e. exited), tests should fail as well. Signed-off-by: Sebastiaan van Stijn --- integration/container/checkpoint_test.go | 4 ---- integration/container/devices_windows_test.go | 5 ----- integration/container/kill_test.go | 1 - integration/container/nat_test.go | 9 +++------ integration/container/pause_test.go | 5 ----- integration/container/pidmode_linux_test.go | 4 ---- integration/container/rename_test.go | 12 +----------- integration/container/run_cgroupns_linux_test.go | 4 ---- integration/container/run_linux_test.go | 10 ---------- integration/container/stats_test.go | 5 ----- integration/container/stop_linux_test.go | 1 - integration/container/update_linux_test.go | 3 --- integration/container/wait_test.go | 4 ---- integration/plugin/authz/authz_plugin_test.go | 2 -- 14 files changed, 4 insertions(+), 65 deletions(-) diff --git a/integration/container/checkpoint_test.go b/integration/container/checkpoint_test.go index 03d2872b43..63795ce596 100644 --- a/integration/container/checkpoint_test.go +++ b/integration/container/checkpoint_test.go @@ -47,10 +47,6 @@ func TestCheckpoint(t *testing.T) { Type: mounttypes.TypeTmpfs, Target: "/tmp", })) - poll.WaitOn(t, - container.IsInState(ctx, apiClient, cID, "running"), - poll.WithDelay(100*time.Millisecond), - ) // FIXME: ipv6 iptables modules are not uploaded in the test environment stdoutStderr, err = exec.Command("bash", "-c", "set -x; "+ diff --git a/integration/container/devices_windows_test.go b/integration/container/devices_windows_test.go index 4fe1e8793c..7c3f7f959a 100644 --- a/integration/container/devices_windows_test.go +++ b/integration/container/devices_windows_test.go @@ -3,14 +3,12 @@ package container // import "github.com/docker/docker/integration/container" import ( "strings" "testing" - "time" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration/internal/container" "github.com/docker/docker/testutil" "gotest.tools/v3/assert" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -107,11 +105,8 @@ func TestWindowsDevices(t *testing.T) { assert.ErrorContains(t, err, d.expectedStartFailureMessage) return } - assert.NilError(t, err) - poll.WaitOn(t, container.IsInState(ctx, apiClient, id, "running"), poll.WithDelay(100*time.Millisecond)) - // /Windows/System32/HostDriverStore is mounted from the host when class GUID 5B45201D-F2F2-4F3B-85BB-30FF1F953599 // is mounted. See `C:\windows\System32\containers\devices.def` on a Windows host for (slightly more) details. res, err := container.Exec(ctx, apiClient, id, []string{ diff --git a/integration/container/kill_test.go b/integration/container/kill_test.go index 5951b74d0b..72b29cb407 100644 --- a/integration/container/kill_test.go +++ b/integration/container/kill_test.go @@ -152,7 +152,6 @@ func TestKillDifferentUserContainer(t *testing.T) { id := container.Run(ctx, t, apiClient, func(c *container.TestContainerConfig) { c.Config.User = "daemon" }) - poll.WaitOn(t, container.IsInState(ctx, apiClient, id, "running"), poll.WithDelay(100*time.Millisecond)) err := apiClient.ContainerKill(ctx, id, "SIGKILL") assert.NilError(t, err) diff --git a/integration/container/nat_test.go b/integration/container/nat_test.go index 6836c6aa70..492fe75ead 100644 --- a/integration/container/nat_test.go +++ b/integration/container/nat_test.go @@ -94,7 +94,7 @@ func startServerContainer(ctx context.Context, t *testing.T, msg string, port in t.Helper() apiClient := testEnv.APIClient() - cID := container.Run(ctx, t, apiClient, + return container.Run(ctx, t, apiClient, container.WithName("server-"+t.Name()), container.WithCmd("sh", "-c", fmt.Sprintf("echo %q | nc -lp %d", msg, port)), container.WithExposedPorts(fmt.Sprintf("%d/tcp", port)), @@ -106,11 +106,8 @@ func startServerContainer(ctx context.Context, t *testing.T, msg string, port in }, }, } - }) - - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - - return cID + }, + ) } // getExternalAddress() returns the external IP-address from eth0. If eth0 has diff --git a/integration/container/pause_test.go b/integration/container/pause_test.go index 61119c4fe3..ef45cc4d8f 100644 --- a/integration/container/pause_test.go +++ b/integration/container/pause_test.go @@ -27,7 +27,6 @@ func TestPause(t *testing.T) { apiClient := testEnv.APIClient() cID := container.Run(ctx, t, apiClient) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) since := request.DaemonUnixTime(ctx, t, apiClient, testEnv) @@ -58,8 +57,6 @@ func TestPauseFailsOnWindowsServerContainers(t *testing.T) { apiClient := testEnv.APIClient() cID := container.Run(ctx, t, apiClient) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - err := apiClient.ContainerPause(ctx, cID) assert.Check(t, is.ErrorContains(err, cerrdefs.ErrNotImplemented.Error())) } @@ -72,8 +69,6 @@ func TestPauseStopPausedContainer(t *testing.T) { apiClient := testEnv.APIClient() cID := container.Run(ctx, t, apiClient) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - err := apiClient.ContainerPause(ctx, cID) assert.NilError(t, err) diff --git a/integration/container/pidmode_linux_test.go b/integration/container/pidmode_linux_test.go index f0713a2c07..728ac1969d 100644 --- a/integration/container/pidmode_linux_test.go +++ b/integration/container/pidmode_linux_test.go @@ -3,14 +3,12 @@ package container // import "github.com/docker/docker/integration/container" import ( "os" "testing" - "time" "github.com/docker/docker/api/types" "github.com/docker/docker/errdefs" "github.com/docker/docker/integration/internal/container" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -25,12 +23,10 @@ func TestPIDModeHost(t *testing.T) { apiClient := testEnv.APIClient() cID := container.Run(ctx, t, apiClient, container.WithPIDMode("host")) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) cPid := container.GetContainerNS(ctx, t, apiClient, cID, "pid") assert.Assert(t, hostPid == cPid) cID = container.Run(ctx, t, apiClient) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) cPid = container.GetContainerNS(ctx, t, apiClient, cID, "pid") assert.Assert(t, hostPid != cPid) } diff --git a/integration/container/rename_test.go b/integration/container/rename_test.go index 5383699d2e..5345dc6d8e 100644 --- a/integration/container/rename_test.go +++ b/integration/container/rename_test.go @@ -52,7 +52,6 @@ func TestRenameStoppedContainer(t *testing.T) { oldName := "first_name" + t.Name() cID := container.Run(ctx, t, apiClient, container.WithName(oldName), container.WithCmd("sh")) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "exited"), poll.WithDelay(100*time.Millisecond)) inspect, err := apiClient.ContainerInspect(ctx, cID) assert.NilError(t, err) @@ -73,7 +72,6 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { oldName := "first_name" + t.Name() cID := container.Run(ctx, t, apiClient, container.WithName(oldName)) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) newName := "new_name" + stringid.GenerateRandomID() err := apiClient.ContainerRename(ctx, oldName, newName) @@ -87,7 +85,6 @@ func TestRenameRunningContainerAndReuse(t *testing.T) { assert.Check(t, is.ErrorContains(err, "No such container: "+oldName)) cID = container.Run(ctx, t, apiClient, container.WithName(oldName)) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) inspect, err = apiClient.ContainerInspect(ctx, cID) assert.NilError(t, err) @@ -100,7 +97,6 @@ func TestRenameInvalidName(t *testing.T) { oldName := "first_name" + t.Name() cID := container.Run(ctx, t, apiClient, container.WithName(oldName)) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := apiClient.ContainerRename(ctx, oldName, "new:invalid") assert.Check(t, is.ErrorContains(err, "Invalid container name")) @@ -142,8 +138,6 @@ func TestRenameAnonymousContainer(t *testing.T) { err = apiClient.ContainerStart(ctx, container1Name, types.ContainerStartOptions{}) assert.NilError(t, err) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - count := "-c" if testEnv.DaemonInfo.OSType == "windows" { count = "-n" @@ -168,8 +162,6 @@ func TestRenameContainerWithSameName(t *testing.T) { oldName := "old" + t.Name() cID := container.Run(ctx, t, apiClient, container.WithName(oldName)) - - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) err := apiClient.ContainerRename(ctx, oldName, oldName) assert.Check(t, is.ErrorContains(err, "Renaming a container with the same name")) err = apiClient.ContainerRename(ctx, cID, oldName) @@ -190,12 +182,10 @@ func TestRenameContainerWithLinkedContainer(t *testing.T) { db1Name := "db1" + t.Name() db1ID := container.Run(ctx, t, apiClient, container.WithName(db1Name)) - poll.WaitOn(t, container.IsInState(ctx, apiClient, db1ID, "running"), poll.WithDelay(100*time.Millisecond)) app1Name := "app1" + t.Name() app2Name := "app2" + t.Name() - app1ID := container.Run(ctx, t, apiClient, container.WithName(app1Name), container.WithLinks(db1Name+":/mysql")) - poll.WaitOn(t, container.IsInState(ctx, apiClient, app1ID, "running"), poll.WithDelay(100*time.Millisecond)) + container.Run(ctx, t, apiClient, container.WithName(app1Name), container.WithLinks(db1Name+":/mysql")) err := apiClient.ContainerRename(ctx, app1Name, app2Name) assert.NilError(t, err) diff --git a/integration/container/run_cgroupns_linux_test.go b/integration/container/run_cgroupns_linux_test.go index e6a8dcfc26..f8ba5fcd75 100644 --- a/integration/container/run_cgroupns_linux_test.go +++ b/integration/container/run_cgroupns_linux_test.go @@ -3,7 +3,6 @@ package container // import "github.com/docker/docker/integration/container" import ( "context" "testing" - "time" "github.com/docker/docker/client" "github.com/docker/docker/integration/internal/container" @@ -11,7 +10,6 @@ import ( "github.com/docker/docker/testutil" "github.com/docker/docker/testutil/daemon" "gotest.tools/v3/assert" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -24,7 +22,6 @@ func testRunWithCgroupNs(ctx context.Context, t *testing.T, daemonNsMode string, defer d.Stop(t) cID := container.Run(ctx, t, apiClient, containerOpts...) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) daemonCgroup := d.CgroupNamespace(t) containerCgroup := container.GetContainerNS(ctx, t, apiClient, cID, "cgroup") @@ -148,7 +145,6 @@ func TestCgroupNamespacesRunOlderClient(t *testing.T) { defer d.Stop(t) cID := container.Run(ctx, t, apiClient) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) daemonCgroup := d.CgroupNamespace(t) containerCgroup := container.GetContainerNS(ctx, t, apiClient, cID, "cgroup") diff --git a/integration/container/run_linux_test.go b/integration/container/run_linux_test.go index cd34077aa2..f63824ba3b 100644 --- a/integration/container/run_linux_test.go +++ b/integration/container/run_linux_test.go @@ -49,9 +49,6 @@ func TestNISDomainname(t *testing.T) { c.Config.Hostname = hostname c.Config.Domainname = domainname }) - - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - inspect, err := apiClient.ContainerInspect(ctx, cID) assert.NilError(t, err) assert.Check(t, is.Equal(hostname, inspect.Config.Hostname)) @@ -92,9 +89,6 @@ func TestHostnameDnsResolution(t *testing.T) { c.Config.Hostname = hostname c.HostConfig.NetworkMode = containertypes.NetworkMode(netName) }) - - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - inspect, err := apiClient.ContainerInspect(ctx, cID) assert.NilError(t, err) assert.Check(t, is.Equal(hostname, inspect.Config.Hostname)) @@ -118,8 +112,6 @@ func TestUnprivilegedPortsAndPing(t *testing.T) { c.Config.User = "1000:1000" }) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - // Check net.ipv4.ping_group_range. res, err := container.Exec(ctx, apiClient, cID, []string{"cat", "/proc/sys/net/ipv4/ping_group_range"}) assert.NilError(t, err) @@ -165,8 +157,6 @@ func TestPrivilegedHostDevices(t *testing.T) { cID := container.Run(ctx, t, apiClient, container.WithPrivileged(true)) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - // Check test device. res, err := container.Exec(ctx, apiClient, cID, []string{"ls", devTest}) assert.NilError(t, err) diff --git a/integration/container/stats_test.go b/integration/container/stats_test.go index 5d78c23dae..de9adf5261 100644 --- a/integration/container/stats_test.go +++ b/integration/container/stats_test.go @@ -5,13 +5,11 @@ import ( "io" "reflect" "testing" - "time" "github.com/docker/docker/api/types" "github.com/docker/docker/integration/internal/container" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -26,9 +24,6 @@ func TestStats(t *testing.T) { assert.NilError(t, err) cID := container.Run(ctx, t, apiClient) - - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - resp, err := apiClient.ContainerStats(ctx, cID, false) assert.NilError(t, err) defer resp.Body.Close() diff --git a/integration/container/stop_linux_test.go b/integration/container/stop_linux_test.go index bb65d370c9..e5a0a7320f 100644 --- a/integration/container/stop_linux_test.go +++ b/integration/container/stop_linux_test.go @@ -88,7 +88,6 @@ func TestStopContainerWithTimeoutCancel(t *testing.T) { id := container.Run(ctx, t, apiClient, container.WithCmd("sh", "-c", "trap 'echo received TERM' TERM; while true; do usleep 10; done"), ) - poll.WaitOn(t, container.IsInState(ctx, apiClient, id, "running")) ctxCancel, cancel := context.WithCancel(ctx) t.Cleanup(cancel) diff --git a/integration/container/update_linux_test.go b/integration/container/update_linux_test.go index 3ecc806ef5..a661a7e83c 100644 --- a/integration/container/update_linux_test.go +++ b/integration/container/update_linux_test.go @@ -14,7 +14,6 @@ import ( "github.com/docker/docker/testutil/request" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -33,8 +32,6 @@ func TestUpdateMemory(t *testing.T) { } }) - poll.WaitOn(t, container.IsInState(ctx, apiClient, cID, "running"), poll.WithDelay(100*time.Millisecond)) - const ( setMemory int64 = 314572800 setMemorySwap int64 = 524288000 diff --git a/integration/container/wait_test.go b/integration/container/wait_test.go index 28a45c94ca..faf7ea51bb 100644 --- a/integration/container/wait_test.go +++ b/integration/container/wait_test.go @@ -86,8 +86,6 @@ func TestWaitBlocked(t *testing.T) { t.Parallel() ctx := testutil.StartSpan(ctx, t) containerID := container.Run(ctx, t, cli, container.WithCmd("sh", "-c", tc.cmd)) - poll.WaitOn(t, container.IsInState(ctx, cli, containerID, "running"), poll.WithTimeout(30*time.Second), poll.WithDelay(100*time.Millisecond)) - waitResC, errC := cli.ContainerWait(ctx, containerID, "") err := cli.ContainerStop(ctx, containerID, containertypes.StopOptions{}) @@ -214,8 +212,6 @@ func TestWaitRestartedContainer(t *testing.T) { ) defer cli.ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{Force: true}) - poll.WaitOn(t, container.IsInState(ctx, cli, containerID, "running"), poll.WithTimeout(30*time.Second), poll.WithDelay(100*time.Millisecond)) - // Container is running now, wait for exit waitResC, errC := cli.ContainerWait(ctx, containerID, tc.waitCond) diff --git a/integration/plugin/authz/authz_plugin_test.go b/integration/plugin/authz/authz_plugin_test.go index 658e57e97c..6bf5135199 100644 --- a/integration/plugin/authz/authz_plugin_test.go +++ b/integration/plugin/authz/authz_plugin_test.go @@ -25,7 +25,6 @@ import ( "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/testutil/environment" "gotest.tools/v3/assert" - "gotest.tools/v3/poll" "gotest.tools/v3/skip" ) @@ -227,7 +226,6 @@ func TestAuthZPluginAllowEventStream(t *testing.T) { // Create a container and wait for the creation events cID := container.Run(ctx, t, c) - poll.WaitOn(t, container.IsInState(ctx, c, cID, "running")) created := false started := false