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

Change minRunners behavior and fix the new listener min runners #3139

Merged
merged 4 commits into from
Dec 13, 2023

Conversation

nikola-jokic
Copy link
Member

This PR implements the proposed ADR and changes its status.

Target runner count is now calculated as min(assigned + minRunners, maxRunners) where maxRunners is set to the maximum number that can fit in int32 if not configured.

Also, this PR fixes a bug in the new listener that is not yet published, introduced by #3096

Fixes #2707

@nikola-jokic nikola-jokic marked this pull request as draft December 8, 2023 13:59
@nikola-jokic nikola-jokic marked this pull request as ready for review December 11, 2023 12:16
@nikola-jokic nikola-jokic added the gha-runner-scale-set Related to the gha-runner-scale-set mode label Dec 11, 2023
done
kubectl wait --timeout=30s --for=condition=ready pod -n arc-systems -l actions.github.com/scale-set-name=$ARC_NAME
kubectl get pod -n arc-systems
- name: Ensure 5 runners are up
Copy link
Collaborator

@Link- Link- Dec 13, 2023

Choose a reason for hiding this comment

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

I think this test is not sufficient, what I think the test should do is also to trigger one or more workflow runs and make sure we have 5 runners all the time

Copy link
Collaborator

@Link- Link- left a comment

Choose a reason for hiding this comment

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

I don't want to delay the release for an e2e test. I'm fine with this moving forward as long as we address the e2e right after

@nikola-jokic nikola-jokic merged commit f7eb88c into master Dec 13, 2023
17 checks passed
@nikola-jokic nikola-jokic deleted the nikola-jokic/min-runners-semantics branch December 13, 2023 18:39
sergelogvinov pushed a commit to sergelogvinov/actions-runner-controller that referenced this pull request Dec 17, 2023
Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
sergelogvinov added a commit to sergelogvinov/actions-runner-controller that referenced this pull request Dec 17, 2023
Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
sergelogvinov added a commit to sergelogvinov/actions-runner-controller that referenced this pull request Dec 17, 2023
Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
sergelogvinov added a commit to sergelogvinov/actions-runner-controller that referenced this pull request Dec 18, 2023
Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
nikola-jokic pushed a commit that referenced this pull request Dec 18, 2023
Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>
@Link- Link- mentioned this pull request Jan 4, 2024
@remover
Copy link
Contributor

remover commented Jan 10, 2024

@nikola-jokic will this also apply to legacy ARC? i'm guessing not but would it be easy enough to apply there also?

@Link-
Copy link
Collaborator

Link- commented Jan 11, 2024

@remover this does not apply to legacy ARC. Legacy ARC is maintained by the community led by @mumoshu and we do not backport changes.

@remover
Copy link
Contributor

remover commented Jan 11, 2024

@mumoshu do you think this would be an easy enough change in legacy ARC? happy to help if so... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gha-runner-scale-set Related to the gha-runner-scale-set mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gha-runner-scale-set - pre-emptively scale runners based on new "min idle runners" setting
3 participants