Say what failed a build, not that its stream closed - #2185
Closed
MayCXC wants to merge 7 commits into
Closed
Conversation
A named build context crosses the shim boundary as the local-dir name BuildKit assigned it, and the host resolves that name against the directories the command line declared: BuildFSSync validates each declared directory at init exactly as it validates the primary context, resolves every walk, read and info transfer through the one name-to-root lookup, and refuses a name that was never declared. The CLI resolves local context values to absolute paths when it validates the flag, so neither the API server's nor the builder VM's view of a relative path can change which directory is served, and non-local references (image, git, URL, oci-layout) pass through to the builder untouched.
The dockerfile frontend takes a set of per-RUN sandbox settings that container build had no way to ask for: extra host entries, a sandbox hostname, the size of /dev/shm, resource limits, a cgroup parent, and the network mode. Each is validated against the shape the frontend parses, so a malformed value is refused before a builder VM starts, and --shm-size additionally accepts the suffixed sizes the other size flags take and hands the frontend the plain byte count it wants. https://github.com/moby/buildkit/blob/v0.29.0/frontend/dockerui/attr.go
Root selection from the relayed dir-name: a declared name serves from its root, the reserved names and absent metadata serve from the primary context, an undeclared name is refused with unknownNamedContext. The symlink boundary enforcement is shown holding under a named root, and init refuses a named directory that does not exist.
An oci-layout build-context names a layout directory on this machine; when the directory does not exist the value used to fall through to image-reference parsing and fail with a misleading invalid-domain error. Name the actual problem before a builder starts.
A build exports its image somewhere, and exporting it into the store the runtime reads from is what makes the image runnable without a trip through a tar. The exporter writes the blobs and the build records the images naming them. Between a blob arriving in the store and a record claiming it, the blob belongs to nobody, and the sweep that reclaims blobs no image references takes it: a build running alongside `image rm` lost the manifest it had just exported. So the blobs a build exports gather in an ingest session, which the store does not hold and no sweep lists, and the store takes the whole set in the same held lock that records the images naming them. The blobs appear at a moment when no sweep can run, and the records claiming them are there when the lock is released. Pulling an image and loading an OCI layout land their content the same way and for the same reason. The lock is what makes that a moment rather than a window. The store's operations suspend mid-flight and the actor admits other calls at every suspension, so a sweep overlapping a landing computed its keep set without the arriving image; it is held across each whole mutating operation now, the way containerd guards the same window with leases. https://github.com/containerd/containerd/blob/main/docs/garbage-collection.md Only an export into the store gathers, so only then is there a session, and that export is what hands it over: a build that reaches its end has nothing left to give back, and one that does not is what the catch returns it for. Two answers the store owes a writer follow from where the blobs are. A writer asks what the store holds before writing and skips whatever it says it has, so a blob it asks about is one this build will not write and still needs: the gathering takes a copy, the way importing an image copies the blobs the store already holds into its own ingest. And a blob already gathered is answered for out of the gathering, since the store does not hold it yet, which is how importing an image resolves a blob it may itself have only just written. A blob is named for its digest alone under the store's algorithm directory, which is the name a later read resolves; a name carrying the algorithm again lands a file nothing looks for.
A build's host side serves the builder over one stream, and a handler that throws ends it. The ending is what the call reports, so a build whose context walk, content proxy, or image resolve failed said only that its stream had closed unexpectedly, and the failure that closed it was never shown: every one of them read the same, whatever went wrong. Keep the first handler failure and raise it in place of the closure.
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
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.
Type of Change
Motivation and Context
A build's host side serves the builder over one stream, and a handler that throws ends it. The ending is what the call reports, so a build whose context walk, content proxy, or image resolve failed said only that its stream had closed unexpectedly, and the failure that closed it was never shown: every one of them read the same, whatever went wrong.
The first handler failure is kept and raised in place of the closure.
This is the reason several build reports in this repository are hard to act on: an unreadable context directory, a registry that refuses a base image, and a blob the store cannot serve all surface as the same sentence about a closed stream. #2089 is one instance of the pattern, where the real cause is an unreadable context.
Dependency and Merge Order
Depends on #2184 (carried in this branch, and through it on #2183 and #2020), which is where the failure-recording sits in the pipeline this raises from. Merge order: apple/container-builder-shim#88, #2020, apple/container-builder-shim#99, #2183, apple/container-builder-shim#98, #2184, then this.
Testing
A build against an unreadable context directory, a base image that cannot be resolved, and a blob the store cannot serve each report their own cause rather than the closed stream.
Integration suite: 397 passed. Unit suite: 772 passed.
make fmt,make checkclean.