-
Notifications
You must be signed in to change notification settings - Fork 785
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
Tests: make _prefetch() parallel-safe #5841
Tests: make _prefetch() parallel-safe #5841
Conversation
This was pointed to you earlier. Changes in this pull request introduce a potentially dangerous statement that shows up in shellcheck:
You are assuming both of these variables are always set. It should be fixed here and at other places before merging. |
The _prefetch helper, introduced in containers#2036, is not parallel-safe: two or more parallel jobs fetching the same image can step on each other and produce garbage images. Although we still can't run buildah tests in parallel (see containers#5552), we can at least set up the scaffolding for that to happen. This commit reworks _prefetch() such that the image work is wrapped inside flock. It has been working fine for months in containers#5552, and is IMO safe for production. This can then make it much easier to flip the parallelization switch once the final zstd bug is squashed. Signed-off-by: Ed Santiago <[email protected]>
e371fcf
to
75c8852
Compare
Fixed, thank you |
Changed LGTM, but a rebase is needed @edsantiago |
Is there a way to shut up the anti-helpful "branch is out-of-date" message in this repo? |
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.
/lgtm
Yes these are settings in the branch protection github settings AFAIK. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, Luap99, nalind 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 |
The _prefetch helper, introduced in #2036, is not parallel-safe: two
or more parallel jobs fetching the same image can step on each other
and produce garbage images.
Although we still can't run buildah tests in parallel (see #5552),
we can at least set up the scaffolding for that to happen. This
commit reworks _prefetch() such that the image work is wrapped
inside flock. It has been working fine for months in #5552,
and is IMO safe for production. This can then make it much
easier to flip the parallelization switch once the final zstd
bug is squashed.
Signed-off-by: Ed Santiago [email protected]