Skip to content

Conversation

@jsternberg
Copy link
Collaborator

@jsternberg jsternberg commented Oct 3, 2025

This utilizes the container filesystem for the file explorer instead of
using the reference directly. For the error condition, this allows us to
gain access to the filesystem when the error happened with its current
state at the time of the error rather than before the step even ran.

A better solution for resolving #3410. Relies on functionality in moby/buildkit#6262.

@jsternberg jsternberg force-pushed the dap-container-fs-requests branch from af9b8a1 to 144a51e Compare January 8, 2026 22:12
@jsternberg jsternberg force-pushed the dap-container-fs-requests branch from 144a51e to f4f3898 Compare January 14, 2026 21:51
@jsternberg jsternberg force-pushed the dap-container-fs-requests branch from f4f3898 to fe002ca Compare January 26, 2026 19:06
@jsternberg jsternberg force-pushed the dap-container-fs-requests branch 2 times, most recently from 98779f9 to e9d6ba0 Compare January 26, 2026 20:23
@jsternberg jsternberg marked this pull request as ready for review January 26, 2026 20:25
@jsternberg
Copy link
Collaborator Author

Finally had time to come back to this and update it for what got merged. It's tested and works with the capability detection. When the capability isn't present, it will not attempt to use the result handle which causes it not to include cache mounts. When the capability is there, cache mounts are present.

tonistiigi
tonistiigi previously approved these changes Feb 4, 2026
dap/thread.go Outdated
rCtx := t.rCtx

getContainer := sync.OnceValues(func() (gateway.Container, error) {
return rCtx.NewContainer(ctx, &build.InvokeConfig{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way this calls ReleaseContainer.

The resources should be released anyway when the gateway session expires, but I am wondering if we can be explicit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this offline. This was intended to use build.NewContainer which registers this cleanup with the result context and executes it when Done() is called. I mistakenly called the function directly instead of through build.NewContainer. I've modified this to use build.NewContainer and added for methods to forward the ReadFile/ReadDir/StatFile calls to the container directly to *build.Container so we can just reuse that code here.

…ile system

The container filesystem request API that has been added to buildkit
allows a container created through the `NewContainer` API to also access
the filesystems. This is useful when an error occurs because it allows
us to grab the mutable state of the mounts used during the actual build
rather than the input version copies which don't contain any files that
were added as part of the failed command.

This gives us a more accurate view of the filesystem that was previously
only accessible through using `exec` and `ls`/`cat` commands that may
not always exist.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
@jsternberg jsternberg force-pushed the dap-container-fs-requests branch from c9f31cd to 1da36d1 Compare February 5, 2026 15:45
@jsternberg jsternberg requested a review from tonistiigi February 5, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants