Skip to content

Commit 9f4309d

Browse files
benoit-nexthopgabriel-samfira
authored andcommitted
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.
1 parent f8e2c6c commit 9f4309d

1 file changed

Lines changed: 16 additions & 3 deletions

File tree

workers/scaleset/scaleset.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,15 @@ func (w *Worker) reapTimedOutRunners(runners map[string]params.RunnerReference)
393393
case commonParams.InstancePendingDelete, commonParams.InstancePendingForceDelete,
394394
commonParams.InstanceDeleting, commonParams.InstanceDeleted:
395395
continue
396+
case commonParams.InstanceCreating, commonParams.InstancePendingCreate:
397+
// Instance is still being created in the provider, or is about to be
398+
// created. The provider worker owns create timeouts/failures for this
399+
// instance and will move it through Error -> PendingDelete on its own
400+
// if creation fails or times out. Nothing for the scale-set-level
401+
// reaper to do here; jumping straight to pending_delete from here is
402+
// also not a valid status transition (creating only allows -> error
403+
// or -> running).
404+
continue
396405
}
397406

398407
if runner.RunnerStatus != params.RunnerPending && runner.RunnerStatus != params.RunnerInstalling && runner.RunnerStatus != params.RunnerFailed {
@@ -404,17 +413,21 @@ func (w *Worker) reapTimedOutRunners(runners map[string]params.RunnerReference)
404413
slog.DebugContext(w.ctx, "runner is locked; skipping", "runner_name", runner.Name)
405414
continue
406415
}
407-
lockNames = append(lockNames, runner.Name)
408416

409417
slog.InfoContext(
410418
w.ctx, "reaping timed-out/failed runner",
411419
"runner_name", runner.Name)
412420

413421
if err := w.removeRunnerFromGithubAndSetPendingDelete(runner.Name, runner.AgentID); err != nil {
422+
// Don't let a single poisoned runner (e.g. one that raced into a
423+
// status that can no longer transition to pending_delete through
424+
// some other codepath) abort reaping for the rest of the batch.
425+
// Log it, release just this runner's lock, and keep going.
414426
slog.ErrorContext(w.ctx, "error removing runner", "runner_name", runner.Name, "error", err)
415-
unlockFn()
416-
return nil, fmt.Errorf("removing runner %s: %w", runner.Name, err)
427+
locking.Unlock(runner.Name, false)
428+
continue
417429
}
430+
lockNames = append(lockNames, runner.Name)
418431
}
419432
}
420433
return unlockFn, nil

0 commit comments

Comments
 (0)