Merge pull request #2257 from hashicorp/b-nil-resources-panic

Handle createdResourcs=nil

client/driver/driver.go

@@ -169,7 +169,8 @@ type Driver interface {
 	Open(ctx *ExecContext, handleID string) (DriverHandle, error)
 
 	// Cleanup is called to remove resources which were created for a task
-	// and no longer needed.
+	// and no longer needed. Cleanup is not called if CreatedResources is
+	// nil.
 	//
 	// If Cleanup returns a recoverable error it may be retried. On retry
 	// it will be passed the same CreatedResources, so all successfully

client/driver/mock_driver.go

@@ -131,6 +131,10 @@ func (m *MockDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
 // Config.Options["cleanup_fail_num"] times. For failures it will return a
 // recoverable error.
 func (m *MockDriver) Cleanup(ctx *ExecContext, res *CreatedResources) error {
+	if res == nil {
+		panic("Cleanup should not be called with nil *CreatedResources")
+	}
+
 	var err error
 	failn, _ := strconv.Atoi(m.config.Options["cleanup_fail_num"])
 	failk := m.config.Options["cleanup_fail_on"]

client/task_runner.go

@@ -1033,6 +1033,11 @@ func (r *TaskRunner) cleanup() {
 	res := r.createdResources.Copy()
 	r.createdResourcesLock.Unlock()
 
+	if res == nil {
+		// No created resources to cleanup
+		return
+	}
+
 	ctx := driver.NewExecContext(r.taskDir, r.alloc.ID)
 	attempts := 1
 	var cleanupErr error

client/task_runner_test.go

@@ -1285,6 +1285,27 @@ 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) {
+	alloc := mock.Alloc()
+	task := alloc.Job.TaskGroups[0].Tasks[0]
+	task.Driver = "mock_driver"
+
+	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)
+	}
+}
+
 func TestTaskRunner_CleanupOK(t *testing.T) {
 	alloc := mock.Alloc()
 	task := alloc.Job.TaskGroups[0].Tasks[0]