-
Notifications
You must be signed in to change notification settings - Fork 337
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
Add support for multiple runners #82
base: main
Are you sure you want to change the base?
Conversation
const timeoutMinutes = 8; | ||
const retryIntervalSeconds = 20; | ||
const quietPeriodSeconds = 40; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these changes are not intended to be merged - they were just for testing
'curl -O -L https://github.com/actions/runner/releases/download/v2.284.0/actions-runner-linux-${RUNNER_ARCH}-2.284.0.tar.gz', | ||
'tar xzf ./actions-runner-linux-${RUNNER_ARCH}-2.284.0.tar.gz', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the version update here is not required, could be pulled out - would still be nice for this to just be automatically the most recent release (#16)
outputs: | ||
label: | ||
description: >- | ||
Name of the unique label assigned to the runner. | ||
The label is used in two cases: | ||
- to use as the input of 'runs-on' property for the following jobs; | ||
- to remove the runner from GitHub when it is not needed anymore. | ||
ec2-instance-id: | ||
ec2-instance-ids: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the name change here is considered breaking api-wise, we could always revert this and still treat it as one or more. I just thought it was more clear to make it plural.
@@ -11,7 +11,7 @@ async function getRunner(label) { | |||
try { | |||
const runners = await octokit.paginate('GET /repos/{owner}/{repo}/actions/runners', config.githubContext); | |||
const foundRunners = _.filter(runners, { labels: [{ name: label }] }); | |||
return foundRunners.length > 0 ? foundRunners[0] : null; | |||
return foundRunners.length > 0 ? foundRunners : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is worth a bit of discussion. Technically only 1 of N runners may be ready at this point. Do we want to wait for all the expected runners, or can we let the workflow proceed even if some runners may not yet be ready? I'm more inclined to let it proceed even if not all runners come online. It will probably take care of itself, and if not, it will just run slower than expected.
@jpalomaki I am not sure how getting outputs works in the context of a matrix. Regardless, it seems inefficient to run the action multiple times when the underlying aws api call supports the desired functionality directly. |
The code here is based upon machulav#82 with some updates to match the latest runner version. My IDE also decided to run a formatter, so there are a few formatting changes that don't change functionality.
The code here is based upon machulav#82 with some updates to match the latest runner version. My IDE also decided to run a formatter, so there are a few formatting changes that don't change functionality.
Here's a proposal for adding support for launching multiple runners. The docs are not updated and the input/output name changes may be undesired, but the gist of the feature is covered. If this approach looks good I can work to clean it up. It could definitely use some more testing (I tried it with 2 runners so far).
Closes #8