Skip to content

Builder::Remote.inspect_builder requires both checks to pass#1611

Merged
djmb merged 1 commit intobasecamp:mainfrom
flavorjones:flavorjones/remote-builder-check
Aug 11, 2025
Merged

Builder::Remote.inspect_builder requires both checks to pass#1611
djmb merged 1 commit intobasecamp:mainfrom
flavorjones:flavorjones/remote-builder-check

Conversation

@flavorjones
Copy link
Member

@flavorjones flavorjones commented Jul 17, 2025 •
edited
Loading

This change updates Builder::Remote#inspect_builder to require that the checks for the builder and the remote context both pass in order for the inspection to succeed.

Previously, if exactly one of the builder or the remote context were in a bad state, the build would proceed anyway and fail.

Reproduction

To reproduce the troublesome scenario, set up a remote builder, and then remove the remote context.

So if you have this builder and context:

kamal-remote-ssh---app-docker-builder-102               docker-container
 \_ kamal-remote-ssh---app-docker-builder-1020           \_ kamal-remote-ssh---app-docker-builder-102-context   running    v0.22.0          linux/amd64 (+4), linux/386

then run docker context rm kamal-remote-ssh---app-docker-builder-102-context. At this point, the builder check succeeds:

$ docker buildx inspect kamal-remote-ssh---app-docker-builder-102 | grep Endpoint:.*kamal-remote-ssh---app-docker-builder-102-context
Endpoint: kamal-remote-ssh---app-docker-builder-102-context

and the remote context check would fail if it was checked:

$ docker context inspect kamal-remote-ssh---app-docker-builder-102-context --format '{{.Endpoints.docker.Host}}'

context "kamal-remote-ssh---app-docker-builder-102-context": context not found: open /home/redacted/.docker/contexts/meta/748847b8af66e1db8361df82b767956490f4c71ac49acc684b121298fb190c59/meta.json: no such file or directory

But since the builder check passes, the remove context is never checked. And so the build continues and fails:

  INFO [fd296da4] Running /usr/bin/env git -C /tmp/kamal-clones/redacted-4558155abc396/redacted/ rev-parse HEAD as redacted@localhost
  INFO [fd296da4] Finished in 0.003 seconds with exit status 0 (successful).
  INFO [6b98f861] Running docker buildx inspect kamal-remote-ssh---app-docker-builder-102 | grep -q Endpoint:.*kamal-remote-ssh---app-docker-builder-102-context || docker context inspect kamal-remote-ssh---app-docker-builder-102-context --format '{{.Endpoints.docker.Host}}' | grep -xq ssh://app@docker-builder-102 || (echo no compatible builder && exit 1) as redacted@localhost
  INFO [6b98f861] Finished in 0.079 seconds with exit status 0 (successful).
  INFO [cdd0f1ea] Running docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-remote-ssh---app-docker-builder-102 -t redacted/redacted/redacted-rails:1c76ea0259fa0810fa2691dc9c74ad55f8f6b8b7 -t redacted/redacted/redacted-rails:latest-beta --label service="redacted" --build-arg [REDACTED] --build-arg [REDACTED] --secret id="GITHUB_TOKEN" --secret id="RAILS_MASTER_KEY" --file Dockerfile . 2>&1 as redacted@localhost
 DEBUG [cdd0f1ea] Command: docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-remote-ssh---app-docker-builder-102 -t redacted/redacted/redacted-rails:1c76ea0259fa0810fa2691dc9c74ad55f8f6b8b7 -t redacted/redacted/redacted-rails:latest-beta --label service="redacted" --build-arg [REDACTED] --build-arg [REDACTED] --secret id="GITHUB_TOKEN" --secret id="RAILS_MASTER_KEY" --file Dockerfile . 2>&1
 DEBUG [cdd0f1ea]       ERROR: failed to build: no valid drivers found: unable to parse docker host `kamal-remote-ssh---app-docker-builder-102-context`
  Finished all in 2.7 seconds
  ERROR (SSHKit::Command::Failed): docker exit status: 256
docker stdout: ERROR: failed to build: no valid drivers found: unable to parse docker host `kamal-remote-ssh---app-docker-builder-102-context`
docker stderr: Nothing written

With the change in this PR, the check successfully fails, and the builder and remote context are both cleaned up and recreated:

  INFO [3276f18c] Running /usr/bin/env git -C /tmp/kamal-clones/redacted-4558155abc396/redacted/ rev-parse HEAD as redacted@localhost
  INFO [3276f18c] Finished in 0.017 seconds with exit status 0 (successful).
  INFO [4fc82cda] Running docker buildx inspect kamal-remote-ssh---app-docker-builder-102 | grep -q Endpoint:.*kamal-remote-ssh---app-docker-builder-102-context && docker context inspect kamal-remote-ssh---app-docker-builder-102-context --format '{{.Endpoints.docker.Host}}' | grep -xq ssh://app@docker-builder-102 && (echo no compatible builder && exit 1) as redacted@localhost
  WARN Missing compatible builder, so creating a new one first
  INFO [c627c2f9] Running docker context rm kamal-remote-ssh---app-docker-builder-102-context ; docker buildx rm kamal-remote-ssh---app-docker-builder-102 as redacted@localhost
  INFO [c627c2f9] Finished in 0.093 seconds with exit status 0 (successful).
  INFO [274886eb] Running /usr/bin/env true on docker-builder-102
  INFO [274886eb] Finished in 0.857 seconds with exit status 0 (successful).
  INFO [f5f04513] Running docker context create kamal-remote-ssh---app-docker-builder-102-context --description 'kamal-remote-ssh---app-docker-builder-102 host' --docker 'host=ssh://app@docker-builder-102' ; docker buildx create --name kamal-remote-ssh---app-docker-builder-102 kamal-remote-ssh---app-docker-builder-102-context as redacted@localhost
  INFO [f5f04513] Finished in 2.637 seconds with exit status 0 (successful).

Test coverage

Currently there doesn't appear to be any test coverage for the builder inspection, since the "build create" command is separate from the inspection/creation logic in "build push".

I think it would be nice if the same inspection/creation logic was used in both commands, but that feels a little out of scope for this bug fix. LMK if you'd like to see a pull request that does that!

But in the meantime, the test I added is pretty blunt and checks for the presence of the entire inspection command, with a "&&" joining the first two clauses.

@flavorjones flavorjones force-pushed the flavorjones/remote-builder-check branch from 35148fd to 887ffac Compare July 17, 2025 19:24
@flavorjones
Builder::Remote.inspect_builder requires both checks to pass
8e470ed 
Previously, if either of the builder or the remote context were in a
bad state, the build would proceed anyway and fail.
@flavorjones flavorjones force-pushed the flavorjones/remote-builder-check branch from 887ffac to 8e470ed Compare July 17, 2025 19:41
@flavorjones flavorjones requested a review from djmb July 17, 2025 19:42
@djmb djmb merged commit 5dd8eba into basecamp:main Aug 11, 2025
7 checks passed
@flavorjones flavorjones deleted the flavorjones/remote-builder-check branch August 11, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djmb djmb Awaiting requested review from djmb

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@flavorjones @djmb