From 99d5e388ee3ff456faf080774b2141759c0a3c1f Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 4 Oct 2023 16:10:14 -0400 Subject: [PATCH 1/2] client: ensure task only runs with prestart hooks Since the allocation in the task runner is updated in a separate goroutine, a race condition may happen where the task is started but the prestart hooks are skipped because the allocation became terminal. Checking for a terminal allocation before proceeding with the task start ensures the task only runs if the prestart hooks are also executed. Since `shouldShutdown()` only uses terminal allocation status, it remains `true` after the first transition, so it's safe to check it again after the prestart hooks as it will never revert to `false`. --- client/allocrunner/taskrunner/task_runner.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/allocrunner/taskrunner/task_runner.go b/client/allocrunner/taskrunner/task_runner.go index 9cf953c031f0..1747601885a8 100644 --- a/client/allocrunner/taskrunner/task_runner.go +++ b/client/allocrunner/taskrunner/task_runner.go @@ -599,6 +599,12 @@ MAIN: goto RESTART } + // Check for a terminal allocation once more before proceeding as the + // prestart hooks may have been skipped. + if tr.shouldShutdown() { + break MAIN + } + select { case <-tr.killCtx.Done(): break MAIN From 8210ce6ecfafcee4428df69bbf9d210c59ec0417 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 4 Oct 2023 16:48:23 -0400 Subject: [PATCH 2/2] changelog: add entry for #18662 --- .changelog/18662.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/18662.txt diff --git a/.changelog/18662.txt b/.changelog/18662.txt new file mode 100644 index 000000000000..6621656b066f --- /dev/null +++ b/.changelog/18662.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: prevent tasks from starting without the prestart hooks running +```