Skip to content

rm: reject context builders before loading nodes#3817

Merged
tonistiigi merged 1 commit into
docker:masterfrom
crazy-max:rm-reject-context
Apr 24, 2026
Merged

rm: reject context builders before loading nodes#3817
tonistiigi merged 1 commit into
docker:masterfrom
crazy-max:rm-reject-context

Conversation

@crazy-max
Copy link
Copy Markdown
Member

@crazy-max crazy-max commented Apr 23, 2026

Current ordering is wrong for offline sockets. It forced driver detection and a daemon ping before rejecting a context-backed builder, which changed the user-facing error from the expected context removal guidance to an unrelated connection error:

$ docker buildx rm dev-mode
failed to remove dev-mode: failed to connect to the docker API at unix:///Users/foo/bar/dev/.docker/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /Users/foo/bar/dev/.docker/run/docker.sock: connect: no such file or directory
ERROR: failed to remove one or more builders

The removal path now checks whether the selected builder came from a Docker context immediately after builder lookup and returns the existing docker context rm guidance without calling LoadNodes.

cc @fiam

Copy link
Copy Markdown
Contributor

@fiam fiam left a comment

Choose a reason for hiding this comment

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

Tested and working. LGTM!

@crazy-max crazy-max marked this pull request as ready for review April 23, 2026 21:11
Comment thread commands/rm.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is still needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, I don't think so. DockerContext is already set when we synthesize implicit context builders, and docker driver does not allow instances, so the post LoadNodes ContextName() check no longer covers a case that isn't already handled by the earlier one.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max added this to the v0.34.0 milestone Apr 24, 2026
@tonistiigi tonistiigi merged commit f9c1a46 into docker:master Apr 24, 2026
160 checks passed
@crazy-max crazy-max deleted the rm-reject-context branch April 24, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants