Skip to content
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.

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

fix(runner): tweak runner idling retry logic #3324

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

TBonnin
Copy link
Collaborator

@TBonnin TBonnin commented Jan 16, 2025

Not really a bug but the runner currently keeps notifying jobs that it is idle every 10s.
However 10s might not be enough for jobs/fleet to terminate the runner (if we are in the middle of a deploy for example, fleet might need to start/terminate dozen of services and it can take quite a long time). Following requests to /idle then fail with an error since the runner is already considered idled. This commit increase the timeout when /idle was called successfully to give enough time to jobs/fleet to terminate the runner and avoid logging an error.
In theory we could also simply stop the loop once /idle has been successful

Currently the runner keeps notifying `jobs` that it is idle every 10s
However 10s might not be enough for jobs/fleet to terminate the runner
and following request to /idle fails with an error.
This commit increase the timeout when /idle was called successfully to
give enough time to jobs/fleet to terminate the runner
In theory we could also simply stop the loop once /idle has been successful
@TBonnin TBonnin requested a review from a team January 16, 2025 19:48
@bodinsamuel bodinsamuel enabled auto-merge (squash) January 17, 2025 13:06
@bodinsamuel bodinsamuel merged commit c11a8cd into master Jan 17, 2025
16 checks passed
@bodinsamuel bodinsamuel deleted the tbonnin/fix-runner-idle-timeout branch January 17, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants