push: workaround deadlock in containerd pusher #2461
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Tonis Tiigi <[email protected]>
Collaborator
|
Doesn't this cause containerd/containerd#2706 ? Deadlock issue will be fixed in containerd/containerd#6243. |
Member
Author
We have our own handling for that case #1548 |
ktock
approved these changes
Nov 14, 2021
AkihiroSuda
approved these changes
Nov 14, 2021
flavorjones
added a commit
to sparklemotion/nokogiri
that referenced
this pull request
Nov 15, 2021
This reverts commit 38a7679. See moby/buildkit#2461 for context
jonathangreen
added a commit
to ThePalaceProject/circulation
that referenced
this pull request
Nov 17, 2021
docker/build-push-action#498 (comment) docker/buildx#834 moby/buildkit#2461 Looks like a fix is merged into buildkit, but we have to wait for a new release to get the fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes docker/buildx#834
Since 1.5 containerd pusher has an issue containerd/containerd#6242 and deadlocks after first error appears(eg. connection dropped or registry error). Purely by coincidence, this error was not affecting buildkit until v0.9.1 because the incorrect logic is behind an interface detection check https://github.com/containerd/containerd/blob/5b09dc5eb0c671d87dc3482d5f63326a4416b74d/remotes/handlers.go#L172-L173 and Buildkit used a wrapper around pusher object.
In v0.9.2 while fixing another issue that wrap was removed and exposed this issue. https://github.com/moby/buildkit/pull/2403/files#diff-9f53bfee1c63146562c6d43b0bc3e476d9e966c93a1b8904031d37ea2f0896bfL82
I have reported this to containerd. Meanwhile, this adds arbitrary wrap around all our usage of
Pusherto work around this.Signed-off-by: Tonis Tiigi [email protected]