test: update tests to properly use AllocDir

Also use t.TempDir when possible.

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)

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

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))
 

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

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)

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 {

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)

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)
 	}

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()

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