diff --git a/client/allocrunner/consul_http_sock_hook_test.go b/client/allocrunner/consul_http_sock_hook_test.go index 05bc31908..3d5a97ec5 100644 --- a/client/allocrunner/consul_http_sock_hook_test.go +++ b/client/allocrunner/consul_http_sock_hook_test.go @@ -93,14 +93,14 @@ func TestConsulHTTPSocketHook_Prerun_Error(t *testing.T) { logger := testlog.HCLogger(t) - allocDir, cleanupDir := allocdir.TestAllocDir(t, logger, "ConnectNativeTask", alloc.ID) - defer cleanupDir() - consulConfig := new(config.ConsulConfig) alloc := mock.Alloc() connectNativeAlloc := mock.ConnectNativeAlloc("bridge") + allocDir, cleanupDir := allocdir.TestAllocDir(t, logger, "ConnectNativeTask", alloc.ID) + defer cleanupDir() + { // an alloc without a connect native task should not return an error h := newConsulHTTPSocketHook(logger, alloc, allocDir, consulConfig) diff --git a/client/allocrunner/taskrunner/connect_native_hook_test.go b/client/allocrunner/taskrunner/connect_native_hook_test.go index 2ce016d9b..a9c43d210 100644 --- a/client/allocrunner/taskrunner/connect_native_hook_test.go +++ b/client/allocrunner/taskrunner/connect_native_hook_test.go @@ -469,7 +469,7 @@ func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) { logger := testlog.HCLogger(t) - allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative", alloc.ID) defer cleanup() // register group services diff --git a/client/allocrunner/taskrunner/dispatch_hook_test.go b/client/allocrunner/taskrunner/dispatch_hook_test.go index 9ac683a14..9f56fe0fd 100644 --- a/client/allocrunner/taskrunner/dispatch_hook_test.go +++ b/client/allocrunner/taskrunner/dispatch_hook_test.go @@ -26,12 +26,13 @@ func TestTaskRunner_DispatchHook_NoPayload(t *testing.T) { require := require.New(t) ctx := context.Background() logger := testlog.HCLogger(t) - allocDir := allocdir.NewAllocDir(logger, "nomadtest_nopayload") - defer allocDir.Destroy() // Default mock alloc/job is not a dispatch job alloc := mock.BatchAlloc() task := alloc.Job.TaskGroups[0].Tasks[0] + + allocDir := allocdir.NewAllocDir(logger, "nomadtest_nopayload", alloc.ID) + defer allocDir.Destroy() taskDir := allocDir.NewTaskDir(task.Name) require.NoError(taskDir.Build(false, nil)) @@ -61,8 +62,6 @@ func TestTaskRunner_DispatchHook_Ok(t *testing.T) { require := require.New(t) ctx := context.Background() logger := testlog.HCLogger(t) - allocDir := allocdir.NewAllocDir(logger, "nomadtest_dispatchok") - defer allocDir.Destroy() // Default mock alloc/job is not a dispatch job; update it alloc := mock.BatchAlloc() @@ -77,6 +76,9 @@ func TestTaskRunner_DispatchHook_Ok(t *testing.T) { task.DispatchPayload = &structs.DispatchPayloadConfig{ File: "out", } + + allocDir := allocdir.NewAllocDir(logger, "nomadtest_dispatchok", alloc.ID) + defer allocDir.Destroy() taskDir := allocDir.NewTaskDir(task.Name) require.NoError(taskDir.Build(false, nil)) @@ -104,8 +106,6 @@ func TestTaskRunner_DispatchHook_Error(t *testing.T) { require := require.New(t) ctx := context.Background() logger := testlog.HCLogger(t) - allocDir := allocdir.NewAllocDir(logger, "nomadtest_dispatcherr") - defer allocDir.Destroy() // Default mock alloc/job is not a dispatch job; update it alloc := mock.BatchAlloc() @@ -121,6 +121,9 @@ func TestTaskRunner_DispatchHook_Error(t *testing.T) { task.DispatchPayload = &structs.DispatchPayloadConfig{ File: "out", } + + allocDir := allocdir.NewAllocDir(logger, "nomadtest_dispatcherr", alloc.ID) + defer allocDir.Destroy() taskDir := allocDir.NewTaskDir(task.Name) require.NoError(taskDir.Build(false, nil)) diff --git a/client/allocrunner/taskrunner/envoy_version_hook_test.go b/client/allocrunner/taskrunner/envoy_version_hook_test.go index 299a9961f..225c8d6e6 100644 --- a/client/allocrunner/taskrunner/envoy_version_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_version_hook_test.go @@ -228,7 +228,7 @@ func TestTaskRunner_EnvoyVersionHook_Prestart_standard(t *testing.T) { // Setup an Allocation alloc := mock.ConnectAlloc() alloc.Job.TaskGroups[0].Tasks[0] = mock.ConnectSidecarTask() - allocDir, cleanupDir := allocdir.TestAllocDir(t, logger, "EnvoyVersionHook") + allocDir, cleanupDir := allocdir.TestAllocDir(t, logger, "EnvoyVersionHook", alloc.ID) defer cleanupDir() // Setup a mock for Consul API diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 9e81abd31..f3ef206cf 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -80,8 +80,7 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri } // Create the alloc dir + task dir - allocPath := filepath.Join(clientConf.AllocDir, alloc.ID) - allocDir := allocdir.NewAllocDir(logger, allocPath) + allocDir := allocdir.NewAllocDir(logger, clientConf.AllocDir, alloc.ID) if err := allocDir.Build(); err != nil { cleanup() t.Fatalf("error building alloc dir: %v", err) diff --git a/client/fs_endpoint_test.go b/client/fs_endpoint_test.go index a8c71265c..fa650df27 100644 --- a/client/fs_endpoint_test.go +++ b/client/fs_endpoint_test.go @@ -31,19 +31,14 @@ import ( "github.com/stretchr/testify/require" ) -// tempAllocDir returns a new alloc dir that is rooted in a temp dir. The caller -// should destroy the temp dir. +// tempAllocDir returns a new alloc dir that is rooted in a temp dir. Caller +// should cleanup with AllocDir.Destroy() func tempAllocDir(t testing.TB) *allocdir.AllocDir { - dir, err := ioutil.TempDir("", "nomadtest") - if err != nil { - t.Fatalf("TempDir() failed: %v", err) - } + dir := t.TempDir() - if err := os.Chmod(dir, 0777); err != nil { - t.Fatalf("failed to chmod dir: %v", err) - } + require.NoError(t, os.Chmod(dir, 0o777)) - return allocdir.NewAllocDir(testlog.HCLogger(t), dir, "test") + return allocdir.NewAllocDir(testlog.HCLogger(t), dir, "test_allocid") } type nopWriteCloser struct { @@ -1561,12 +1556,11 @@ func TestFS_findClosest(t *testing.T) { func TestFS_streamFile_NoFile(t *testing.T) { t.Parallel() - require := require.New(t) c, cleanup := TestClient(t, nil) defer cleanup() ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + defer ad.Destroy() frames := make(chan *sframer.StreamFrame, 32) framer := sframer.NewStreamFramer(frames, streamHeartbeatRate, streamBatchWindow, streamFrameSize) @@ -1575,11 +1569,11 @@ func TestFS_streamFile_NoFile(t *testing.T) { err := c.endpoints.FileSystem.streamFile( context.Background(), 0, "foo", 0, ad, framer, nil) - require.NotNil(err) + require.Error(t, err) if runtime.GOOS == "windows" { - require.Contains(err.Error(), "cannot find the file") + require.Contains(t, err.Error(), "cannot find the file") } else { - require.Contains(err.Error(), "no such file") + require.Contains(t, err.Error(), "no such file") } } @@ -1591,7 +1585,8 @@ func TestFS_streamFile_Modify(t *testing.T) { // Get a temp alloc dir ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + require.NoError(t, ad.Build()) + defer ad.Destroy() // Create a file in the temp dir streamFile := "stream_file" @@ -1660,16 +1655,15 @@ func TestFS_streamFile_Truncate(t *testing.T) { // Get a temp alloc dir ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + require.NoError(t, ad.Build()) + defer ad.Destroy() // Create a file in the temp dir data := []byte("helloworld") streamFile := "stream_file" streamFilePath := filepath.Join(ad.AllocDir, streamFile) f, err := os.Create(streamFilePath) - if err != nil { - t.Fatalf("Failed to create file: %v", err) - } + require.NoError(t, err) defer f.Close() // Start the reader @@ -1768,7 +1762,8 @@ func TestFS_streamImpl_Delete(t *testing.T) { // Get a temp alloc dir ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + require.NoError(t, ad.Build()) + defer ad.Destroy() // Create a file in the temp dir data := []byte("helloworld") @@ -1840,7 +1835,8 @@ func TestFS_logsImpl_NoFollow(t *testing.T) { // Get a temp alloc dir and create the log dir ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + require.NoError(t, ad.Build()) + defer ad.Destroy() logDir := filepath.Join(ad.SharedDir, allocdir.LogDirName) if err := os.MkdirAll(logDir, 0777); err != nil { @@ -1908,7 +1904,8 @@ func TestFS_logsImpl_Follow(t *testing.T) { // Get a temp alloc dir and create the log dir ad := tempAllocDir(t) - defer os.RemoveAll(ad.AllocDir) + require.NoError(t, ad.Build()) + defer ad.Destroy() logDir := filepath.Join(ad.SharedDir, allocdir.LogDirName) if err := os.MkdirAll(logDir, 0777); err != nil { diff --git a/drivers/docker/driver_unix_test.go b/drivers/docker/driver_unix_test.go index 019b42764..701ed8985 100644 --- a/drivers/docker/driver_unix_test.go +++ b/drivers/docker/driver_unix_test.go @@ -765,15 +765,15 @@ func copyImage(t *testing.T, taskDir *allocdir.TaskDir, image string) { // copyFile moves an existing file to the destination func copyFile(src, dst string, t *testing.T) { + t.Helper() in, err := os.Open(src) if err != nil { t.Fatalf("copying %v -> %v failed: %v", src, dst, err) } defer in.Close() out, err := os.Create(dst) - if err != nil { - t.Fatalf("copying %v -> %v failed: %v", src, dst, err) - } + require.NoError(t, err, "copying %v -> %v failed: %v", src, dst, err) + defer func() { if err := out.Close(); err != nil { t.Fatalf("copying %v -> %v failed: %v", src, dst, err) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 16d5fdc3f..687c64635 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -64,7 +64,7 @@ func testExecutorCommandWithChroot(t *testing.T) *testExecCmd { task := alloc.Job.TaskGroups[0].Tasks[0] taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, "global").Build() - allocDir := allocdir.NewAllocDir(testlog.HCLogger(t), t.TempDir(), alloc.ID) + allocDir := allocdir.NewAllocDir(testlog.HCLogger(t), os.TempDir(), alloc.ID) if err := allocDir.Build(); err != nil { t.Fatalf("AllocDir.Build() failed: %v", err) } diff --git a/plugins/drivers/testutils/testing.go b/plugins/drivers/testutils/testing.go index b26b7e1f3..32cc24477 100644 --- a/plugins/drivers/testutils/testing.go +++ b/plugins/drivers/testutils/testing.go @@ -83,10 +83,12 @@ func (h *DriverHarness) Kill() { func (h *DriverHarness) MkAllocDir(t *drivers.TaskConfig, enableLogs bool) func() { dir, err := ioutil.TempDir("", "nomad_driver_harness-") require.NoError(h.t, err) - t.AllocDir = dir allocDir := allocdir.NewAllocDir(h.logger, dir, t.AllocID) require.NoError(h.t, allocDir.Build()) + + t.AllocDir = allocDir.AllocDir + taskDir := allocDir.NewTaskDir(t.Name) caps, err := h.Capabilities() diff --git a/testutil/wait.go b/testutil/wait.go index 4f9965a6a..0f4dfffed 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -201,7 +201,9 @@ func WaitForRunningWithToken(t testing.TB, rpc rpcFn, job *structs.Job, token st var resp structs.JobAllocationsResponse - WaitForResult(func() (bool, error) { + // This can be quite slow if the job has expensive setup such as + // downloading large artifacts or creating a chroot. + WaitForResultRetries(2000*TestMultiplier(), func() (bool, error) { args := &structs.JobSpecificRequest{} args.JobID = job.ID args.QueryOptions.Region = job.Region