Skip to content

Commit

Permalink
Handle createdResourcs=nil
Browse files Browse the repository at this point in the history
Combined with b522c47 this fixes #2256

Without these two commits in place upgrades to 0.5.3 panics.
  • Loading branch information
schmichael committed Jan 31, 2017
1 parent b522c47 commit 2822dd7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 1 deletion.
3 changes: 2 additions & 1 deletion client/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions client/driver/mock_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
5 changes: 5 additions & 0 deletions client/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions client/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 2822dd7

Please sign in to comment.