-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Do not use gh-hosted-runners-16cores-1
runners on forks
#17237
base: main
Are you sure you want to change the base?
Do not use gh-hosted-runners-16cores-1
runners on forks
#17237
Conversation
…low is running in the `vitessio/vitess` repository. Signed-off-by: Arthur Schreiber <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17237 +/- ##
==========================================
+ Coverage 67.39% 67.40% +0.01%
==========================================
Files 1570 1570
Lines 252903 252912 +9
==========================================
+ Hits 170451 170485 +34
+ Misses 82452 82427 -25 ☔ View full report in Codecov by Sentry. |
@@ -16,7 +16,7 @@ env: | |||
jobs: | |||
build: | |||
name: Run endtoend tests on Cluster (onlineddl_vrepl) | |||
runs-on: gh-hosted-runners-16cores-1 | |||
runs-on: ${{ github.repository_owner == 'vitessio' && 'gh-hosted-runners-16cores-1' || 'ubuntu-latest' }} |
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 thunk we should pin a to a specific known working ubuntu version and not latest
. That we do that in other jobs in a mistake imho, we explicitly pinned it in the past to avoid having to scramble when it gets updated.
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'm okay with that, but I think that should probably go into a separate PR, right? 🤔
Description
We (GitHub) and other members of the community maintain our own Vitess fork. We rely on the CI builds to ensure that when we backport changes into our fork, we don't introduce any bugs or other issues.
We only have access to the "normal" runner types, and don't have access to
gh-hosted-runners-16cores-1
, which is specific to thevitessio
organization and I guess is provided by CNCF.By checking whether a workflow runs in the scope of the
vitessio
organization, and then deciding betweengh-hosted-runners-16cores-1
or the regularubuntu-latest
runners, we can allow workflows to run in forks of the vitess repo as well.cc
@timvaillancourt as you might be interested in this as well.I'd like to see this backported in all supported branches, as this will make my live considerably easier without having any real impact on the upstream Vitess repository. 😅
Related Issue(s)
N/A
Checklist
Deployment Notes