-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
CI: only make install.tools when needed #15896
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago 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 |
80714bf
to
2cae850
Compare
@cevich any idea what's going on with CI today? Seen in multiple in-flight PRs:
|
2cae850
to
e8b169c
Compare
I'm pretty sure it's GitHub's fault, it's affecting everybody. Re-running doesn't seem to work (for who-knows-why), Waiting 5-10m and force-pushing a small change seems to un-stuck it most of the time. But if everybody does it, it may make the problem worse but IDK for sure. |
Reintroduce .install.foo targets into Makefile, and invoke only the bare-minimum ones needed for each individual CI step in setup_environment.sh. Also add a retry to the golangci-lint curl, in hopes of dealing with network flakes. And remove the -f (fail) because it produces unhelpful logs. Reason: saw about 25% CI flakes yesterday due to the golangci-lint fetch, something about a timeout, and this was especially frustrating because none of the steps actually needed lint. Quick reminder: avoid network fetches unless absolutely necessary. Fixes: containers#15892 Signed-off-by: Ed Santiago <[email protected]>
e8b169c
to
0fb95f9
Compare
FWIW, we perform this install at VM image build time, so maybe it's unnecessary at runtime? IIRC, the complaint was that makes it harder for developers to update, since it (formerly) required a new image build + Makefile update (here). IMHO, installing stuff (in general) with |
LGTM |
Agreed, and I also agree with finding a way to install these tools in the VMs, not at runtime. But that's a big can of worms, and my purpose here is to do something fast easy simple trivial to get rid of nasty flakes. |
Yep, I gotcha, I wasn't calling for that here, just mentioning it since it keeps popping up for me. Seems like something that should go into a Cabal meeting and/or Jira card at the least. |
Ugh...woops...brain-fart: There's a merge bot. |
Reintroduce .install.foo targets into Makefile, and invoke
only the bare-minimum ones needed for each individual CI
step in setup_environment.sh.
Also add a retry to the golangci-lint curl, in hopes of
dealing with network flakes. And remove the -f (fail)
because it produces unhelpful logs.
Reason: saw about 25% CI flakes yesterday due to the golangci-lint
fetch, something about a timeout, and this was especially frustrating
because none of the steps actually needed lint. Quick reminder:
avoid network fetches unless absolutely necessary.
Fixes: #15892
Signed-off-by: Ed Santiago [email protected]