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

Runner label checks seem overly tight #1714

Closed
cbrianhill opened this issue Feb 7, 2022 · 3 comments
Closed

Runner label checks seem overly tight #1714

cbrianhill opened this issue Feb 7, 2022 · 3 comments

Comments

@cbrianhill
Copy link

Hi, we recently upgraded to version 0.34.0, and did some testing with ephemeral runners. When enabling the ephemeral runners, it seemed desirable to set runner_enable_workflow_job_labels_check to true, so that we didn't create a new runner for every job (most of our jobs run on GH-hosted runners).

We add a unique label for the jobs we want to run on these self-hosted runners, and it looks like we're being tripped up here. For our use case, it seems like we want to run the job if jobMatch evaluates to true, even if runnerMatch is false. It's true that we could add the additional labels to our jobs, but it seems more complex than what we need.

I'm curious about the other use cases that need strict checking, and whether the maintainers would be interested in a pull request that separated this flag into two: one to check that all runner labels are present on the workflow_job event, and another to check that all labels on the workflow_job event are present on the runner. For us, the former check is not useful, but the latter is.

@npalm
Copy link
Member

npalm commented Feb 9, 2022

@cbrianhill thanks for creating the issue. And we also struggle a bit to find out the best way to move forward. We made it very strict to avoid indeed all kind of useless runners are crated. In this discussion I also added an idea to add kind of filter chain. To enable support of multiple runners by one webhook and leverage all the logica already in the runner module. See option 2.

However, we are open to find ways to make the checking more flexible to decide which events to accept and dropped. Maybe helpfull to write down a couple of concrete cases. We certainly welcome PR's!

@driskell
Copy link

I have hit an issue with this.

I set runner-extra-labels to "self-hosted" and it never matches even though my runner was "self-hosted". I hadn't realised that the workflow needed to specify the architecture and os name as well. So I have a workaround which is to specify in the labels for the runner x64 and linux too.

Ideally the check would not require that the workflow match exactly. I would expect if linux wasn't provided than effectively "any" runner would do that is available.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants