Merge pull request #8060 from hashicorp/tests-deflake-20200526

Deflake some tests - 2020-05-27 edition

drivers/docker/config.go

@@ -684,6 +684,7 @@ func (d *Driver) SetConfig(c *base.Config) error {
 		return fmt.Errorf("failed to get docker client: %v", err)
 	}
 	coordinatorConfig := &dockerCoordinatorConfig{
+		ctx:         d.ctx,
 		client:      dockerClient,
 		cleanup:     d.config.GC.Image,
 		logger:      d.logger,

drivers/docker/coordinator.go

@@ -70,6 +70,8 @@ func noopLogEventFn(string, map[string]string) {}
 
 // dockerCoordinatorConfig is used to configure the Docker coordinator.
 type dockerCoordinatorConfig struct {
+	ctx context.Context
+
 	// logger is the logger the coordinator should use
 	logger hclog.Logger
 
@@ -283,7 +285,7 @@ func (d *dockerCoordinator) RemoveImage(imageID, callerID string) {
 	}
 
 	// Setup a future to delete the image
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(d.ctx)
 	d.deleteFuture[imageID] = cancel
 	go d.removeImageImpl(imageID, ctx)
 

drivers/docker/coordinator_test.go

@@ -1,6 +1,7 @@
 package docker
 
 import (
+	"context"
 	"fmt"
 	"sync"
 	"testing"
@@ -10,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/testutil"
+	"github.com/stretchr/testify/require"
 )
 
 type mockImageClient struct {
@@ -61,6 +63,7 @@ func TestDockerCoordinator_ConcurrentPulls(t *testing.T) {
 	// Add a delay so we can get multiple queued up
 	mock := newMockImageClient(mapping, 10*time.Millisecond)
 	config := &dockerCoordinatorConfig{
+		ctx:         context.Background(),
 		logger:      testlog.HCLogger(t),
 		cleanup:     true,
 		client:      mock,
@@ -70,10 +73,10 @@ func TestDockerCoordinator_ConcurrentPulls(t *testing.T) {
 	// Create a coordinator
 	coordinator := newDockerCoordinator(config)
 
-	id, _ := coordinator.PullImage(image, nil, uuid.Generate(), nil, 2 * time.Minute)
+	id, _ := coordinator.PullImage(image, nil, uuid.Generate(), nil, 2*time.Minute)
 	for i := 0; i < 9; i++ {
 		go func() {
-			coordinator.PullImage(image, nil, uuid.Generate(), nil, 2 * time.Minute)
+			coordinator.PullImage(image, nil, uuid.Generate(), nil, 2*time.Minute)
 		}()
 	}
 
@@ -112,6 +115,7 @@ func TestDockerCoordinator_Pull_Remove(t *testing.T) {
 	// Add a delay so we can get multiple queued up
 	mock := newMockImageClient(mapping, 10*time.Millisecond)
 	config := &dockerCoordinatorConfig{
+		ctx:         context.Background(),
 		logger:      testlog.HCLogger(t),
 		cleanup:     true,
 		client:      mock,
@@ -125,7 +129,7 @@ func TestDockerCoordinator_Pull_Remove(t *testing.T) {
 	callerIDs := make([]string, 10, 10)
 	for i := 0; i < 10; i++ {
 		callerIDs[i] = uuid.Generate()
-		id, _ = coordinator.PullImage(image, nil, callerIDs[i], nil, 2 * time.Minute)
+		id, _ = coordinator.PullImage(image, nil, callerIDs[i], nil, 2*time.Minute)
 	}
 
 	// Check the reference count
@@ -179,6 +183,7 @@ func TestDockerCoordinator_Remove_Cancel(t *testing.T) {
 
 	mock := newMockImageClient(mapping, 1*time.Millisecond)
 	config := &dockerCoordinatorConfig{
+		ctx:         context.Background(),
 		logger:      testlog.HCLogger(t),
 		cleanup:     true,
 		client:      mock,
@@ -190,7 +195,7 @@ func TestDockerCoordinator_Remove_Cancel(t *testing.T) {
 	callerID := uuid.Generate()
 
 	// Pull image
-	id, _ := coordinator.PullImage(image, nil, callerID, nil, 2 * time.Minute)
+	id, _ := coordinator.PullImage(image, nil, callerID, nil, 2*time.Minute)
 
 	// Check the reference count
 	if references := coordinator.imageRefCount[id]; len(references) != 1 {
@@ -206,7 +211,7 @@ func TestDockerCoordinator_Remove_Cancel(t *testing.T) {
 	}
 
 	// Pull image again within delay
-	id, _ = coordinator.PullImage(image, nil, callerID, nil, 2 * time.Minute)
+	id, _ = coordinator.PullImage(image, nil, callerID, nil, 2*time.Minute)
 
 	// Check the reference count
 	if references := coordinator.imageRefCount[id]; len(references) != 1 {
@@ -227,6 +232,7 @@ func TestDockerCoordinator_No_Cleanup(t *testing.T) {
 
 	mock := newMockImageClient(mapping, 1*time.Millisecond)
 	config := &dockerCoordinatorConfig{
+		ctx:         context.Background(),
 		logger:      testlog.HCLogger(t),
 		cleanup:     false,
 		client:      mock,
@@ -238,7 +244,7 @@ func TestDockerCoordinator_No_Cleanup(t *testing.T) {
 	callerID := uuid.Generate()
 
 	// Pull image
-	id, _ := coordinator.PullImage(image, nil, callerID, nil, 2 * time.Minute)
+	id, _ := coordinator.PullImage(image, nil, callerID, nil, 2*time.Minute)
 
 	// Check the reference count
 	if references := coordinator.imageRefCount[id]; len(references) != 0 {
@@ -253,3 +259,55 @@ func TestDockerCoordinator_No_Cleanup(t *testing.T) {
 		t.Fatalf("Image deleted when it shouldn't have")
 	}
 }
+
+func TestDockerCoordinator_Cleanup_HonorsCtx(t *testing.T) {
+	image1ID := uuid.Generate()
+	image2ID := uuid.Generate()
+
+	mapping := map[string]string{image1ID: "foo", image2ID: "bar"}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	mock := newMockImageClient(mapping, 1*time.Millisecond)
+	config := &dockerCoordinatorConfig{
+		ctx:         ctx,
+		logger:      testlog.HCLogger(t),
+		cleanup:     true,
+		client:      mock,
+		removeDelay: 1 * time.Millisecond,
+	}
+
+	// Create a coordinator
+	coordinator := newDockerCoordinator(config)
+	callerID := uuid.Generate()
+
+	// Pull image
+	id1, _ := coordinator.PullImage(image1ID, nil, callerID, nil, 2*time.Minute)
+	require.Len(t, coordinator.imageRefCount[id1], 1, "image reference count")
+
+	id2, _ := coordinator.PullImage(image2ID, nil, callerID, nil, 2*time.Minute)
+	require.Len(t, coordinator.imageRefCount[id2], 1, "image reference count")
+
+	// remove one image, cancel ctx, remove second, and assert only first image is cleanedup
+	// Remove image
+	coordinator.RemoveImage(id1, callerID)
+	testutil.WaitForResult(func() (bool, error) {
+		if _, ok := mock.removed[id1]; ok {
+			return true, nil
+		}
+		return false, fmt.Errorf("expected image to delete found %v", mock.removed)
+	}, func(err error) {
+		require.NoError(t, err)
+	})
+
+	cancel()
+	coordinator.RemoveImage(id2, callerID)
+
+	// deletions occur in background, wait to ensure that
+	// the image isn't deleted after a timeout
+	time.Sleep(10 * time.Millisecond)
+
+	// Check that only no delete happened
+	require.Equal(t, map[string]int{id1: 1}, mock.removed, "removed images")
+}

drivers/docker/driver_linux_test.go

@@ -59,7 +59,7 @@ func TestDockerDriver_PidsLimit(t *testing.T) {
 	cfg.Args = []string{"-c", "sleep 5 & sleep 5 & sleep 5"}
 	require.NoError(task.EncodeConcreteDriverConfig(cfg))
 
-	_, driver, _, cleanup := dockerSetup(t, task)
+	_, driver, _, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	driver.WaitUntilStarted(task.ID, time.Duration(tu.TestMultiplier()*5)*time.Second)

drivers/docker/driver_test.go

@@ -120,15 +120,15 @@ func dockerTask(t *testing.T) (*drivers.TaskConfig, *TaskConfig, []int) {
 //
 //	task := taskTemplate()
 //	// do custom task configuration
-//	client, handle, cleanup := dockerSetup(t, task)
+//	client, handle, cleanup := dockerSetup(t, task, nil)
 //	defer cleanup()
 //	// do test stuff
 //
 // If there is a problem during setup this function will abort or skip the test
 // and indicate the reason.
-func dockerSetup(t *testing.T, task *drivers.TaskConfig) (*docker.Client, *dtestutil.DriverHarness, *taskHandle, func()) {
+func dockerSetup(t *testing.T, task *drivers.TaskConfig, driverCfg map[string]interface{}) (*docker.Client, *dtestutil.DriverHarness, *taskHandle, func()) {
 	client := newTestDockerClient(t)
-	driver := dockerDriverHarness(t, nil)
+	driver := dockerDriverHarness(t, driverCfg)
 	cleanup := driver.MkAllocDir(task, true)
 
 	copyImage(t, task.TaskDir(), "busybox.tar")
@@ -181,6 +181,7 @@ func dockerDriverHarness(t *testing.T, cfg map[string]interface{}) *dtestutil.Dr
 	if cfg == nil {
 		cfg = map[string]interface{}{
 			"gc": map[string]interface{}{
+				"image":       false,
 				"image_delay": "1s",
 			},
 		}
@@ -912,7 +913,7 @@ func TestDockerDriver_Labels(t *testing.T) {
 	}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -940,7 +941,7 @@ func TestDockerDriver_ForcePull(t *testing.T) {
 	cfg.ForcePull = true
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
@@ -971,7 +972,7 @@ func TestDockerDriver_ForcePull_RepoDigest(t *testing.T) {
 	cfg.Args = busyboxLongRunningCmd[1:]
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -994,7 +995,7 @@ func TestDockerDriver_SecurityOptUnconfined(t *testing.T) {
 	cfg.SecurityOpt = []string{"seccomp=unconfined"}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1021,7 +1022,7 @@ func TestDockerDriver_SecurityOptFromFile(t *testing.T) {
 	cfg.SecurityOpt = []string{"seccomp=./test-resources/docker/seccomp.json"}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1042,7 +1043,7 @@ func TestDockerDriver_Runtime(t *testing.T) {
 	cfg.Runtime = "runc"
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1470,7 +1471,7 @@ func TestDockerDriver_DNS(t *testing.T) {
 	cfg.DNSOptions = []string{"ndots:1"}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
@@ -1523,7 +1524,7 @@ func TestDockerDriver_MACAddress(t *testing.T) {
 	cfg.MacAddress = "00:16:3e:00:00:00"
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1544,7 +1545,7 @@ func TestDockerWorkDir(t *testing.T) {
 	cfg.WorkDir = "/some/path"
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1573,7 +1574,7 @@ func TestDockerDriver_PortsNoMap(t *testing.T) {
 	res := ports[0]
 	dyn := ports[1]
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1622,7 +1623,7 @@ func TestDockerDriver_PortsMapping(t *testing.T) {
 	}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -1706,7 +1707,7 @@ func TestDockerDriver_CleanupContainer(t *testing.T) {
 	cfg.Args = []string{"hello"}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	waitCh, err := d.WaitTask(context.Background(), task.ID)
@@ -1940,7 +1941,7 @@ func TestDockerDriver_Stats(t *testing.T) {
 	cfg.Args = []string{"1000"}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	_, d, handle, cleanup := dockerSetup(t, task)
+	_, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -2405,7 +2406,7 @@ func TestDockerDriver_Device_Success(t *testing.T) {
 	cfg.Devices = []DockerDevice{config}
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, driver, handle, cleanup := dockerSetup(t, task)
+	client, driver, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -2431,7 +2432,7 @@ func TestDockerDriver_Entrypoint(t *testing.T) {
 
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, driver, handle, cleanup := dockerSetup(t, task)
+	client, driver, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))
@@ -2458,7 +2459,7 @@ func TestDockerDriver_ReadonlyRootfs(t *testing.T) {
 	cfg.ReadonlyRootfs = true
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, driver, handle, cleanup := dockerSetup(t, task)
+	client, driver, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))
 

drivers/docker/driver_unix_test.go

@@ -159,7 +159,7 @@ func TestDockerDriver_CPUCFSPeriod(t *testing.T) {
 	cfg.CPUCFSPeriod = 1000000
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, _, handle, cleanup := dockerSetup(t, task)
+	client, _, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 
 	waitForExist(t, client, handle.containerID)
@@ -184,7 +184,7 @@ func TestDockerDriver_Sysctl_Ulimit(t *testing.T) {
 	cfg.Ulimit = expectedUlimits
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))
 
@@ -678,7 +678,12 @@ func TestDockerDriver_Cleanup(t *testing.T) {
 
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, driver, handle, cleanup := dockerSetup(t, task)
+	client, driver, handle, cleanup := dockerSetup(t, task, map[string]interface{}{
+		"gc": map[string]interface{}{
+			"image":       true,
+			"image_delay": "1ms",
+		},
+	})
 	defer cleanup()
 
 	require.NoError(t, driver.WaitUntilStarted(task.ID, 5*time.Second))

drivers/docker/reconciler_test.go

@@ -61,7 +61,7 @@ func TestDanglingContainerRemoval(t *testing.T) {
 	defer freeport.Return(ports)
 	require.NoError(t, task.EncodeConcreteDriverConfig(cfg))
 
-	client, d, handle, cleanup := dockerSetup(t, task)
+	client, d, handle, cleanup := dockerSetup(t, task, nil)
 	defer cleanup()
 	require.NoError(t, d.WaitUntilStarted(task.ID, 5*time.Second))