Merge branch 'master' of github.com:hashicorp/nomad

client/task_runner.go

@@ -95,6 +95,7 @@ type TaskRunner struct {
 
 	// createdResources are all the resources created by the task driver
 	// across all attempts to start the task.
+	// Simple gets and sets should use {get,set}CreatedResources
 	createdResources     *driver.CreatedResources
 	createdResourcesLock sync.Mutex
 
@@ -247,9 +248,10 @@ func (r *TaskRunner) RestoreState() error {
 	}
 	r.artifactsDownloaded = snap.ArtifactDownloaded
 	r.taskDirBuilt = snap.TaskDirBuilt
-	r.createdResources = snap.CreatedResources
 	r.payloadRendered = snap.PayloadRendered
 
+	r.setCreatedResources(snap.CreatedResources)
+
 	if err := r.setTaskEnv(); err != nil {
 		return fmt.Errorf("client: failed to create task environment for task %q in allocation %q: %v",
 			r.task.Name, r.alloc.ID, err)
@@ -303,17 +305,13 @@ func (r *TaskRunner) SaveState() error {
 	r.persistLock.Lock()
 	defer r.persistLock.Unlock()
 
-	r.createdResourcesLock.Lock()
-	res := r.createdResources.Copy()
-	r.createdResourcesLock.Unlock()
-
 	snap := taskRunnerState{
 		Task:               r.task,
 		Version:            r.config.Version,
 		ArtifactDownloaded: r.artifactsDownloaded,
 		TaskDirBuilt:       r.taskDirBuilt,
 		PayloadRendered:    r.payloadRendered,
-		CreatedResources:   res,
+		CreatedResources:   r.getCreatedResources(),
 	}
 
 	r.handleLock.Lock()
@@ -1029,14 +1027,7 @@ func (r *TaskRunner) cleanup() {
 		return
 	}
 
-	r.createdResourcesLock.Lock()
-	res := r.createdResources.Copy()
-	r.createdResourcesLock.Unlock()
-
-	if res == nil {
-		// No created resources to cleanup
-		return
-	}
+	res := r.getCreatedResources()
 
 	ctx := driver.NewExecContext(r.taskDir, r.alloc.ID)
 	attempts := 1
@@ -1047,9 +1038,7 @@ func (r *TaskRunner) cleanup() {
 
 		// Copy current createdResources state in case SaveState is
 		// called between retries
-		r.createdResourcesLock.Lock()
-		r.createdResources = res.Copy()
-		r.createdResourcesLock.Unlock()
+		r.setCreatedResources(res)
 
 		// Retry 3 times with sleeps between
 		if !retry || attempts > 3 {
@@ -1474,6 +1463,30 @@ func (r *TaskRunner) Destroy(event *structs.TaskEvent) {
 	close(r.destroyCh)
 }
 
+// getCreatedResources returns the resources created by drivers. It will never
+// return nil.
+func (r *TaskRunner) getCreatedResources() *driver.CreatedResources {
+	r.createdResourcesLock.Lock()
+	if r.createdResources == nil {
+		r.createdResources = driver.NewCreatedResources()
+	}
+	cr := r.createdResources.Copy()
+	r.createdResourcesLock.Unlock()
+
+	return cr
+}
+
+// setCreatedResources updates the resources created by drivers. If passed nil
+// it will set createdResources to an initialized struct.
+func (r *TaskRunner) setCreatedResources(cr *driver.CreatedResources) {
+	if cr == nil {
+		cr = driver.NewCreatedResources()
+	}
+	r.createdResourcesLock.Lock()
+	r.createdResources = cr.Copy()
+	r.createdResourcesLock.Unlock()
+}
+
 // emitStats emits resource usage stats of tasks to remote metrics collector
 // sinks
 func (r *TaskRunner) emitStats(ru *cstructs.TaskResourceUsage) {

client/task_runner_test.go

@@ -1285,9 +1285,9 @@ func TestTaskRunner_SimpleRun_Dispatch(t *testing.T) {
 	}
 }
 
-// TestTaskRunner_CleanupNil ensures TaskRunner doesn't call Driver.Cleanup if
-// no resources were created.
-func TestTaskRunner_CleanupNil(t *testing.T) {
+// TestTaskRunner_CleanupEmpty ensures TaskRunner works when createdResources
+// is empty.
+func TestTaskRunner_CleanupEmpty(t *testing.T) {
 	alloc := mock.Alloc()
 	task := alloc.Job.TaskGroups[0].Tasks[0]
 	task.Driver = "mock_driver"
@@ -1295,14 +1295,12 @@ func TestTaskRunner_CleanupNil(t *testing.T) {
 	ctx := testTaskRunnerFromAlloc(t, false, alloc)
 	ctx.tr.MarkReceived()
 
-	ctx.tr.createdResources = nil
-
 	defer ctx.Cleanup()
 	ctx.tr.Run()
 
 	// Since we only failed once, createdResources should be empty
-	if ctx.tr.createdResources != nil {
-		t.Fatalf("createdResources should still be nil: %v", ctx.tr.createdResources)
+	if len(ctx.tr.createdResources.Resources) != 0 {
+		t.Fatalf("createdResources should still be empty: %v", ctx.tr.createdResources)
 	}
 }