Skip to content

scaleset: tolerate invalid instance status transitions in job message handlers #812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
benoit-nexthop wants to merge 1 commit into from
Closed

scaleset: tolerate invalid instance status transitions in job message handlers #812

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions workers/scaleset/scaleset_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,25 @@ func (w *Worker) HandleJobsCompleted(jobs []params.ScaleSetJobMessage) (err erro
locking.Lock(job.RunnerName, w.consumerID)
_, err := w.store.UpdateInstance(w.ctx, job.RunnerName, runnerUpdateParams)
if err != nil {
if !errors.Is(err, runnerErrors.ErrNotFound) {
locking.Unlock(job.RunnerName, false)
return fmt.Errorf("updating runner %s: %w", job.RunnerName, err)
if errors.Is(err, runnerErrors.ErrNotFound) {
locking.Unlock(job.RunnerName, true)
continue
}
if errors.Is(err, runnerErrors.ErrBadRequest) {

@gabriel-samfira gabriel-samfira Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could indeed race if agent mode is used, but when in agent mode, the runner is set in pending_delete and eventually reaped. A retry here would hit ErrNotFound and should just continue. But we could find out more from logs if it you could share some (anonymized). You can find me on slack and we can debug this in private if it helps.

// The instance is no longer in a state that can transition to
// pending_delete (e.g. it was already deleted or is being deleted
// by some other codepath, such as the reconciler noticing it went
// missing from the provider/github). There is nothing further for
// us to do here. Treating this as fatal would cause this message
// to be retried forever, since we never advance past it or delete
// it, blocking every subsequent message for this scale set.
slog.InfoContext(w.ctx, "runner already being handled/removed; ignoring completed job status update",
"runner_name", job.RunnerName, "error", err)
locking.Unlock(job.RunnerName, true)
continue
}
locking.Unlock(job.RunnerName, false)
return fmt.Errorf("updating runner %s: %w", job.RunnerName, err)
}
locking.Unlock(job.RunnerName, false)
}
Expand Down Expand Up @@ -160,6 +175,16 @@ func (w *Worker) HandleJobsStarted(jobs []params.ScaleSetJobMessage) (err error)
locking.Unlock(job.RunnerName, true)
continue
}
if errors.Is(err, runnerErrors.ErrBadRequest) {

@gabriel-samfira gabriel-samfira Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting. The HandleJobsStarted() function is called as a result of a message that comes over the scaleset longpoll session and only happens when a runner transitions from idle to active. While the runner is active, the github API does not allow the runner to be removed.

A reap operation usually tries to remove the runner from github first, then marks the runner as pending_delete. The runners should then be cleaned up from the provider. Unless I messed up and didn't do that, there should be no way a runner is reaped by GARM while active. If a runner is forcefully removed from the provider while idle, it goes offline and cannot pick up a job. So it can't transition to active and a message that triggers HandleJobsStarted() won't arrive.

If a runner is forcefully removed from the provider while active it times out after 10 minutes, but then a completed status is sent and a different function should fire.

So I am unsure how this code path was triggered in your env. It may be a bug somewhere else, and I would like to track it down and fix the cause rather than treat the symptom.

Could you share some debug logs by any chance that tracks a runner that has hit this? Are you using agent mode? Were any custom changes made to GARM that could impact this?

// The runner is already in a terminal/being-deleted state (e.g. it
// was reaped after going missing from the provider or github). Same
// reasoning as in HandleJobsCompleted: don't fail the whole message,
// or we'll retry it forever and block the scale set's message queue.
slog.InfoContext(w.ctx, "runner already being handled/removed; ignoring started job status update",
"runner_name", job.RunnerName, "error", err)
locking.Unlock(job.RunnerName, true)
continue
}
locking.Unlock(job.RunnerName, false)
return fmt.Errorf("updating runner %s: %w", job.RunnerName, err)
}
Expand Down