diff --git a/runner/internal/shim/docker_test.go b/runner/internal/shim/docker_test.go index f1f51eda5..b629a0d18 100644 --- a/runner/internal/shim/docker_test.go +++ b/runner/internal/shim/docker_test.go @@ -17,13 +17,17 @@ import ( "github.com/stretchr/testify/require" ) +/* +Docker tests are not parallel: they share the Docker daemon, and a DockerRunner adopts +every task container on the host, including the containers of the other tests. +*/ + // TestDocker_SSHServer pulls ubuntu image (without sshd), installs openssh-server and exits // Basically, it indirectly tests a shell script generated by getSSHShellCommands func TestDocker_SSHServer(t *testing.T) { if testing.Short() { t.Skip() } - t.Parallel() params := &dockerParametersMock{ commands: []string{"/usr/sbin/sshd -V 2>&1 | grep OpenSSH"}, @@ -39,7 +43,7 @@ func TestDocker_SSHServer(t *testing.T) { require.NoError(t, err) taskConfig := createTaskConfig(t) - defer dockerRunner.Remove(t.Context(), taskConfig.ID) + defer cleanupTask(t, dockerRunner, taskConfig.ID) assert.NoError(t, dockerRunner.Submit(ctx, taskConfig)) assert.NoError(t, dockerRunner.Start(ctx, taskConfig.ID)) @@ -50,7 +54,6 @@ func TestDocker_ShmNoexecByDefault(t *testing.T) { if testing.Short() { t.Skip() } - t.Parallel() params := &dockerParametersMock{ commands: []string{"mount | grep '/dev/shm .*size=65536k' | grep noexec"}, @@ -65,7 +68,7 @@ func TestDocker_ShmNoexecByDefault(t *testing.T) { require.NoError(t, err) taskConfig := createTaskConfig(t) - defer dockerRunner.Remove(t.Context(), taskConfig.ID) + defer cleanupTask(t, dockerRunner, taskConfig.ID) assert.NoError(t, dockerRunner.Submit(ctx, taskConfig)) assert.NoError(t, dockerRunner.Start(ctx, taskConfig.ID)) @@ -76,7 +79,6 @@ func TestDocker_ShmExecIfSizeSpecified(t *testing.T) { if testing.Short() { t.Skip() } - t.Parallel() params := &dockerParametersMock{ commands: []string{"mount | grep '/dev/shm .*size=1024k' | grep -v noexec"}, @@ -92,7 +94,7 @@ func TestDocker_ShmExecIfSizeSpecified(t *testing.T) { taskConfig := createTaskConfig(t) taskConfig.ShmSize = 1024 * 1024 - defer dockerRunner.Remove(t.Context(), taskConfig.ID) + defer cleanupTask(t, dockerRunner, taskConfig.ID) assert.NoError(t, dockerRunner.Submit(ctx, taskConfig)) assert.NoError(t, dockerRunner.Start(ctx, taskConfig.ID)) @@ -103,7 +105,6 @@ func TestDocker_ContainerExitedWithError(t *testing.T) { if testing.Short() { t.Skip() } - t.Parallel() params := &dockerParametersMock{ commands: []string{"echo failed for a reason", "exit 3"}, @@ -118,7 +119,7 @@ func TestDocker_ContainerExitedWithError(t *testing.T) { require.NoError(t, err) taskConfig := createTaskConfig(t) - defer dockerRunner.Remove(t.Context(), taskConfig.ID) + defer cleanupTask(t, dockerRunner, taskConfig.ID) require.NoError(t, dockerRunner.Submit(ctx, taskConfig)) require.NoError(t, dockerRunner.Start(ctx, taskConfig.ID)) @@ -135,7 +136,6 @@ func TestDocker_RestoredTaskIsTerminated(t *testing.T) { if testing.Short() { t.Skip() } - t.Parallel() params := &dockerParametersMock{ commands: []string{"sleep 3", "exit 7"}, @@ -156,7 +156,7 @@ func TestDocker_RestoredTaskIsTerminated(t *testing.T) { // The restarted shim restores the task from its container restartedRunner, err := NewDockerRunner(ctx, params) require.NoError(t, err) - defer restartedRunner.Remove(t.Context(), taskConfig.ID) + defer cleanupTask(t, restartedRunner, taskConfig.ID) require.Equal(t, taskConfig.ID, restartedRunner.TaskInfo(taskConfig.ID).ID) taskInfo := waitTaskTerminated(t, restartedRunner, taskConfig.ID) @@ -271,16 +271,32 @@ func assertTaskDone(t *testing.T, runner *DockerRunner, taskID string) { } // waitTaskTerminated processes tasks until the task is terminated, as Start() only -// starts the container and does not wait for it to exit +// starts the container and does not wait for it to exit. +// The timeout covers the whole container lifetime, which may be long, e.g., if the +// container installs packages func waitTaskTerminated(t *testing.T, runner *DockerRunner, taskID string) TaskInfo { t.Helper() - var taskInfo TaskInfo require.Eventually(t, func() bool { runner.ProcessTasks(t.Context()) - taskInfo = runner.TaskInfo(taskID) - return taskInfo.Status == TaskStatusTerminated - }, 60*time.Second, 200*time.Millisecond) - return taskInfo + return runner.TaskInfo(taskID).Status == TaskStatusTerminated + }, 150*time.Second, 200*time.Millisecond) + // Read outside of the condition, which Eventually runs in a separate goroutine + // that may outlive the call + return runner.TaskInfo(taskID) +} + +// cleanupTask terminates and removes the task, so that its container does not outlive +// the test even if the test fails. A container left behind is adopted by the runners of +// the following test runs, breaking them in confusing ways +func cleanupTask(t *testing.T, runner *DockerRunner, taskID string) { + t.Helper() + reason := string(types.TerminationReasonTerminatedByUser) + if err := runner.Terminate(t.Context(), taskID, 0, reason, "test cleanup"); err != nil { + t.Logf("failed to terminate task %s: %s", taskID, err) + } + if err := runner.Remove(t.Context(), taskID); err != nil { + t.Logf("failed to remove task %s: %s", taskID, err) + } } func createTaskConfig(t *testing.T) TaskConfig {