From 940eb1bd8153b3ba2d032733334a1daa16050d5b Mon Sep 17 00:00:00 2001 From: Benoit Sigoure Date: Tue, 7 Jul 2026 12:09:57 +0000 Subject: [PATCH] scaleset: fix reapTimedOutRunners deadlock on creating instances reapTimedOutRunners skipped runners in pending_delete/pending_force_delete/ deleting/deleted status, but not creating/pending_create. When a runner's scale-set-level bootstrap timeout elapsed while its instance was still being created in the provider (e.g. CloudStack deploy running long due to host iowait) and it had never joined github, this tried to transition the instance straight from creating to pending_delete. That transition isn't allowed (InstanceCreating only permits -> error or -> running), so UpdateInstance returned a BadRequestError. That error was treated as fatal: reapTimedOutRunners immediately unlocked everything it had processed so far and returned early, and its caller (consolidateRunnerState) aborted the rest of consolidation for that scale set on any error. This meant a single runner still legitimately booting (just slow) could block reaping of every other eligible runner in the same scale set, repeating every consolidate tick (~5 min) until the poisoned runner resolved on its own via the provider worker's separate create- timeout handling. Observed in prod: 'invalid instance status transition from creating to pending_delete' logged repeatedly for the micro scale set while CloudStack was slow due to iowait. Fix: * Skip InstanceCreating/InstancePendingCreate in reapTimedOutRunners, mirroring the same skip already done later in consolidateRunnerState. The provider worker owns create timeouts/failures for these instances. * Don't let one runner's transition error abort the whole batch: log it, release just that runner's lock, and keep processing the rest instead of returning early. --- workers/scaleset/scaleset.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/workers/scaleset/scaleset.go b/workers/scaleset/scaleset.go index abc2426f9..a46ff2b09 100644 --- a/workers/scaleset/scaleset.go +++ b/workers/scaleset/scaleset.go @@ -393,6 +393,15 @@ func (w *Worker) reapTimedOutRunners(runners map[string]params.RunnerReference) case commonParams.InstancePendingDelete, commonParams.InstancePendingForceDelete, commonParams.InstanceDeleting, commonParams.InstanceDeleted: continue + case commonParams.InstanceCreating, commonParams.InstancePendingCreate: + // Instance is still being created in the provider, or is about to be + // created. The provider worker owns create timeouts/failures for this + // instance and will move it through Error -> PendingDelete on its own + // if creation fails or times out. Nothing for the scale-set-level + // reaper to do here; jumping straight to pending_delete from here is + // also not a valid status transition (creating only allows -> error + // or -> running). + continue } if runner.RunnerStatus != params.RunnerPending && runner.RunnerStatus != params.RunnerInstalling { @@ -404,17 +413,21 @@ func (w *Worker) reapTimedOutRunners(runners map[string]params.RunnerReference) slog.DebugContext(w.ctx, "runner is locked; skipping", "runner_name", runner.Name) continue } - lockNames = append(lockNames, runner.Name) slog.InfoContext( w.ctx, "reaping timed-out/failed runner", "runner_name", runner.Name) if err := w.removeRunnerFromGithubAndSetPendingDelete(runner.Name, runner.AgentID); err != nil { + // Don't let a single poisoned runner (e.g. one that raced into a + // status that can no longer transition to pending_delete through + // some other codepath) abort reaping for the rest of the batch. + // Log it, release just this runner's lock, and keep going. slog.ErrorContext(w.ctx, "error removing runner", "runner_name", runner.Name, "error", err) - unlockFn() - return nil, fmt.Errorf("removing runner %s: %w", runner.Name, err) + locking.Unlock(runner.Name, false) + continue } + lockNames = append(lockNames, runner.Name) } } return unlockFn, nil