Add ESC to abort image pull with fall-back to local#341
Merged
gtsiolis merged 4 commits intoJul 1, 2026
Conversation
gtsiolis
added
semver: patch
docs: needed
gtsiolis
force-pushed
the
pro-324-esc-to-abort-image-pull-with-fall-back-to-local-b20c
branch
3 times, most recently
from
8c36b88 to
2919f92
Compare
Member
Author
|
@carole-lavillonniere I've added you as reviewer since you originally proposed this[1]! 😁 |
gtsiolis
force-pushed
the
pro-324-esc-to-abort-image-pull-with-fall-back-to-local-b20c
branch
from
2919f92 to
ef3a7bb
Compare
During a floating-tag pull, if a local copy of the image is already present and we're interactive, show "press esc to keep current version" once real layer download begins. Pressing ESC cancels only the in-flight pull (Ctrl+C still quits) and starts the emulator with the existing local image. A failed pull (e.g. offline) with a local copy present now falls back automatically instead of aborting the start. The domain stays UI-free: pullImage runs the pull under a per-pull cancel context and selects on a skip channel the TUI signals via the new PullSkippableEvent. Generated with [Linear](https://linear.app/localstack/issue/PRO-324/esc-to-abort-image-pull-with-fall-back-to-local#agent-session-ee6a32f4) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Two correctness fixes in the ESC-to-abort-pull flow (PRO-324): 1. Deadlock on an immediate pull error. DockerRuntime.PullImage returned the error from client.ImagePull before registering `defer close(progress)`, so the progress channel was never closed on that path. pullImage now waits on the progress goroutine to drain (`<-progressDone`), turning that prior goroutine leak into a hang of the whole start flow — exactly in the offline/unreachable-registry scenario this feature targets. Close progress unconditionally so callers can never hang. Regression test points a DockerRuntime at an unreachable socket and asserts progress is closed. 2. Ctrl+C during a pull was conflated with a pull failure. A cancelled parent context surfaced as a pull error and, when a local image existed, was swallowed by the local-image fall-back (returning success), so the start continued on an already-cancelled context instead of aborting. Propagate context.Canceled before the fall-back. Regression test cancels the parent context mid-pull and asserts the cancellation propagates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gtsiolis
force-pushed
the
pro-324-esc-to-abort-image-pull-with-fall-back-to-local-b20c
branch
from
ef3a7bb to
a7a39d0
Compare
anisaoshafi
reviewed
Jul 1, 2026
|
|
||
| // pullSkipHint replaces the "Pulling <image>" spinner text once a skippable pull | ||
| // starts downloading, telling the user they can bail out to the local image. | ||
| const pullSkipHint = "Pulling new version… (press esc to keep current version)" |
Collaborator
There was a problem hiding this comment.
tiny nit: probably uppercase ESC is more user friendly and stands out? 
Member
Author
There was a problem hiding this comment.
We don't have a strong consistency around this yet, but sounds good.
@linear can you make the change?
Contributor
There was a problem hiding this comment.
On it. I’m updating that copy now.
Contributor
There was a problem hiding this comment.
Done — the copy now uses uppercase ESC, and the matching test was updated too. The UI tests passed and the change is pushed.
Address review feedback: uppercase ESC stands out better in the spinner text. Generated with [Linear](https://linear.app/localstack/issue/PRO-324/esc-to-abort-image-pull-with-fall-back-to-local#agent-session-ee6a32f4) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
Member
Author
|
Thanks, both! Merging... 🌮 🌮 🌮 |
gtsiolis
deleted the
pro-324-esc-to-abort-image-pull-with-fall-back-to-local-b20c
branch
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.
Makes a floating-tag (
latest/empty) image pull interruptible and resilient, so a slow pull no longer blocks the whole start with no way out. Implements PRO-324; builds on PRO-323 (ImageExists). Idea: @carole-lavillonniere .Behavior
Pulling new version… (press esc to keep current version). ESC cancels only the pull and starts on the local image; Ctrl+C /qstill quit.Could not pull <image> (<err>); using the local image.ErrorEvent+ silent error + telemetry, as before.The hint only arms once Docker reports real layer download, never on a cache hit.
How it works
pullImageschecksImageExistsand delegates each pull to a newpullImage, which runs under a per-pull cancel context andselects on the pull result or a buffered skip channel. The domain stays UI-free: it emitsoutput.PullSkippableEventwith a cancel channel it owns; the TUI binds ESC to signal it during the pulling phase. Gated onlocalExists && interactive.Tests
start_test.go(skip/use-local, fall-back, fatal, non-interactive, cancel-propagates) ·docker_test.go(progress closes on immediate pull error) ·app_test.go(ESC signals skip, inert otherwise) ·plain_sink_test.go(event suppressed). PTY coverage skipped — both triggers are timing/registry-dependent and are covered by the deterministic unit/UI tests.ESC