-
Notifications
You must be signed in to change notification settings - Fork 663
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
Limit scheduling jobs to iree-org #19224
Conversation
@@ -25,6 +25,7 @@ concurrency: | |||
|
|||
jobs: | |||
setup: | |||
if: ${{ github.repository_owner == 'iree-org' || github.event_name != 'schedule' }} |
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.
We could also change jobs like this to configure their runs-on
based on the repository owner, so if the workflow does run on pull_request
triggers, it has a chance of succeeding without needing to add self-hosted runners to the repo.
# runs-on: azure-linux-scale
runs-on: ${{ github.repository_owner == 'iree-org' && 'azure-linux-scale' || 'ubuntu-20.04' }}
(Same comment for all workflows that have pull_request
triggers.... except Windows right now)
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.
I think would work for ci_linux_x64_clang_debug.yml
and ci_linux_x64_clang_tsan.yml
but not for ci_linux_arm64_clang.yml
? Generally I agree and that it would be a great improvement, but do you think we should do it in addition or instead of disabling running on schedule?
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.
Well, the failure mode for requesting a label that is not available in the repository is pretty poor: the workflow just waits until it times out. I think extending this pull request to also handle at least those Linux jobs that have the pull_request
trigger makes sense.
Could either:
A) Keep current behavior (so PRs on forks would wait until GitHub's timeout, and the scheduled runs would similarly fail)
B) Skip unless the event is workflow_dispatch
(sorta weird, would unbreak pull_request
and schedule
triggers but keep workflow_dispatch
broken unless someone goes through the effort of registering a self-hosted running with the same label(s))
C) Switch runner based on the github.repository_owner
D) Switch runner based on the github.repository_owner
and also skip if the event is not workflow_dispatch
E) Switch runner based on the github.repository_owner
and also skip if the event is schedule (so they would run on pull_request but be slowwww)
I like option D or E.
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.
I switched it for some of the jobs where it makes sense. The runner is changed to a GH-hosted one only for those job where a run would be triggered by a workflow dispatch.
@@ -21,6 +21,7 @@ concurrency: | |||
|
|||
jobs: | |||
windows_x64_msvc: | |||
if: ${{ github.repository_owner == 'iree-org' || github.event_name != 'schedule' }} |
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.
Here we could ideally also select the runs-on
based on the repository[_owner], if this comment were addressed: #18967 (comment)
Ideally we could make this workflow more agnostic to the specific runner, so it can run in forks. One code pattern for that is https://github.com/openxla/stablehlo/blob/b1c1115f070493845ad92bd4a4461f77a2aa628d/.github/workflows/buildAndTestBazel.yml#L39
runs-on: ${{ github.repository == 'openxla/stablehlo' && 'ubuntu-22.04-64core' || 'ubuntu-22.04' }}Something to aim for later. All the build directory setup that is specific to these Azure runners will make that difficult and will limit workflow debugging to only developers with write access to this project.
A middle ground is possible where the
runs-on
field has a branch and theCreate build dir
step only runsif ${{ github.repository ==
orif ${{ runs-on == arc-runner-set
.
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.
I think in particular the Windows build wont run on a GH-hosted runner (due to all the sccache and Azure stuff).
859ca9f
to
ddc54a4
Compare
If running not in `iree-org` switch the runner to a GH-hosted one.
ef59ef0
to
f9ae84d
Compare
Limits the execution of cron-scheduled jobs to in iree-org, while allowing to trigger via
workflow_dispatch
also from forks.