From baea4716b7912ca85b79651f906badfe1b89c90d Mon Sep 17 00:00:00 2001 From: Ben Buzbee Date: Sat, 3 Jul 2021 01:39:35 +0000 Subject: [PATCH] Don't treat a failed recover + successful destroy as a successful recover This code just seems incorrect. As it stands today it reports a successful restore if RecoverTask fails and then DestroyTask succeeds. This can result in a really annoying bug where it then calls RecoverTask again, whereby it will probably get ErrTaskNotFound and call DestroyTask once more. I think the only reason this has not been noticed so far is because most drivers like Docker will return Success, then nomad will call RecoverTask, get an error (not found) and call DestroyTask again, and get a ErrTasksNotFound err. --- client/allocrunner/taskrunner/task_runner.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index 0dbfbfe34..1a209380f 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -1136,10 +1136,9 @@ func (tr *TaskRunner) restoreHandle(taskHandle *drivers.TaskHandle, net *drivers "error", err, "task_id", taskHandle.Config.ID) } - return false } - return true + return false } // Update driver handle on task runner