fix(dockerutil): retry a pull from the local registry

[gilescope]

Docker Integrations fails intermittently on a pull that has nothing wrong with it.

The failure

The surfaced error names the mechanism:

Error: pull ping error: pull ping response: rpc error: code = Unknown desc =
  image pull: command failed: docker pull 127.0.0.1:PORT/sess-ID/pullping:img-0

The cause is two hundred lines earlier, inside an escaped buildkitd log line:

failed to copy: httpReadSeeker: failed open: failed to do request:
  Get "https://127.0.0.1:PORT/v2/sess-ID/pullping/blobs/sha256:...": EOF

A bare EOF, no status, on loopback, for a blob of an image buildkitd has just published to its
own session registry. That is a server closing an idle keep-alive connection under a client about
to reuse it — not an answer about the image. dockerPullLocalImage pulled once and gave up.

How often

Counted across four consecutive CI runs on giles-post-buildkit-engine:

runs	Docker Integrations job-runs	failures
4	96	1

About 1% per job-run — rare enough that a green run proves nothing, common enough to fail a build
a few times a month. That is the band where a fault gets rediagnosed rather than recognised.

The change

Three attempts, 150ms then 300ms.

Every error is retried rather than a matched subset, deliberately. The ref names an image that
exists, on a registry this process reaches over loopback, so there is no permanent failure worth
failing fast on — and classifying would mean matching text in a subprocess's stderr, making this
code depend on the wording of another program's messages.

What this does not fix

This is the client half of the race. The server half is buildkit's session registry
(moby/buildkit/session/pullping), which wants its idle timeout above the client's — that needs
the fork and is not attempted here.

The error also still surfaces as the mechanism rather than the cause. Worth fixing separately:
dockerPullLocalImage should include the frontend's stderr in its wrap.

Testing

Against a frontend that fails the first pull with the error CI produced: one failure is retried and
tags exactly once, and a pull that never works still fails with the transport error intact rather
than "tried three times".

At 1 in 96 a green CI run is not evidence either way, which is why the test drives the failure
directly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv

Summary by CodeRabbit

• Bug Fixes
  • Image pulls now automatically retry up to three times after transient failures.
  • Pull operations return promptly when canceled during a retry delay.
  • Persistent pull failures continue to report the underlying error.

[github-actions]

➖ Are we earthbuild yet?

No change in "earthly" occurrences

📈 Overall Progress

Branch	Total Count
main	3612
This PR	3612
Difference	+0

---

Keep up the great work migrating from Earthly to Earthbuild! 🚀

💡 Tips for finding more occurrences

Run locally to see detailed breakdown:

./.github/scripts/count-earthly.sh

Note that the goal is not to reach 0.
There is anticipated to be at least some occurrences of earthly in the source code due to backwards compatibility with config files and language constructs.

[janishorsts]

Here's the analysis by Gemini 3.8 Flash.

Investigation: Flaky pull ping error: pull ping response: ... image pull: command failed: docker pull 127.0.0.1:PORT/... EOF

During recent CI runs (e.g., run 34451234362 and 34409930995), jobs building ./buildkitd+buildkitd --TAG=race-test failed in +test-no-qemu-slow (-race) with:

Error: pull ping error: pull ping response: rpc error: code = Unknown desc = image pull: command failed: docker pull 127.0.0.1:39603/sess-zqwtd81o81fwqwcq0jzq2fp50/pullping:img-0: exit status 1: img-0: Pulling from sess-zqwtd81o81fwqwcq0jzq2fp50/pullping
...
failed to copy: httpReadSeeker: failed open: failed to do request: Get "https://127.0.0.1:39603/v2/sess-zqwtd81o81fwqwcq0jzq2fp50/pullping/blobs/sha256:8e1dfa9c4ce2...": EOF: exit status 1

---

How Image Export Works via regproxy

When an image is exported to Docker without --push (e.g. ./buildkitd+buildkitd), EarthBuild uses earthlyoutputs + pullping + regproxy:

sequenceDiagram
    autonumber
    participant Docker as Docker Daemon
    participant RegProxy as EarthBuild regproxy<br/>(127.0.0.1:PORT)
    participant BKClient as EarthBuild Client<br/>(gRPC)
    participant BKServer as BuildKit Server<br/>(Server.Proxy)
    participant Registry as Embedded Registry<br/>(127.0.0.1:8371)

    Note over BKClient,BKServer: Build finishes -> pullping callback triggered
    BKClient->>Docker: docker pull 127.0.0.1:PORT/sess-ID/pullping:img-0
    Docker->>RegProxy: HTTP GET /v2/.../blobs/sha256:...
    RegProxy->>BKServer: Open gRPC bidirectional stream (Registry.Proxy)
    BKServer->>Registry: Dial 127.0.0.1:8371 & forward HTTP request
    Registry-->>BKServer: HTTP/1.1 200 OK (stream blob bytes)
    BKServer-->>RegProxy: Stream ByteMessages over gRPC
    RegProxy-->>Docker: Forward bytes over TCP
    
    Note over BKServer,RegProxy: FAILURE POINT: Connection drops mid-stream (EOF)
    RegProxy--xDocker: Unexpected EOF
    Docker--xBKClient: Pull failed: EOF (exit status 1)

Loading

---

Root Causes

The issue is caused by two compounding bugs in api/services/registry/server.go in the EarthBuild buildkit fork (used by both client and server):

1. Broken io.Reader implementation in StreamRW.Read

Located in api/services/registry/server.go:

// Read implements io.Reader.
func (s *StreamRW) Read(p []byte) (int, error) {
	l := 0
	if len(s.last) > 0 {
		l = copy(p, s.last)
	}

	msg, err := s.stream.Recv()
	if err != nil {
		return 0, err
	}

	s.last = msg.GetData()
	n := copy(p, s.last)
	s.last = s.last[n:]

	return n + l, nil
}

This violates the standard Go io.Reader contract:

1. Buffer Overwrite: If len(s.last) > 0, it copies s.last into p[0:l]. But instead of returning the buffered data, it calls s.stream.Recv() and executes n := copy(p, s.last). This copies into p[0:n], overwriting the l bytes that were just written to p[0:l].
2. Buffer Overflow (n + l > len(p)): Because it returns n + l, if n == len(p) (e.g. 32 KB buffer from io.Copy), n + l is greater than len(p). In Go, returning n > len(p) causes callers like io.Copy to slice past capacity:

   panic: runtime error: slice bounds out of range [:1536] with capacity 1024
   goroutine ... [running]:
   bytes.(*Buffer).ReadFrom(...)
   io.copyBuffer(...)
   

   (Reproduced and confirmed in isolated tests).
3. Data Loss on EOF: If len(s.last) > 0 and the gRPC stream closes (Recv() returns io.EOF), Read() returns 0, io.EOF, silently dropping the remaining bytes in s.last.

---

2. Fragile 50ms Read Deadline in CopyWithDeadline

Located in api/services/registry/server.go:

const readDeadline = 50 * time.Millisecond

func CopyWithDeadline(conn net.Conn, w io.Writer) (int64, error) {
	var (
		t   = int64(0)
		buf = make([]byte, 32*1024)
	)
	for {
		err := conn.SetReadDeadline(time.Now().Add(readDeadline))
		if err != nil {
			return t, err
		}
		n, err := conn.Read(buf)
		if err != nil {
			if errors.Is(err, io.EOF) || isNetTimeout(err) {
				break
			}
			return t, err
		}
		n, err = w.Write(buf[0:n])
		t += int64(n)
		if err != nil {
			return t, err
		}
	}
	return t, nil
}

• Why it exists: Docker uses HTTP/1.1 with Keep-Alive. Because the embedded registry keeps the TCP connection open after sending a response, standard io.Copy(rw, conn) was hanging. The 50ms deadline was introduced in commit d5e9d257c3 to detect response completion by socket silence.
• Why it breaks: When pulling large layers (such as ./buildkitd+buildkitd which has 27 layers including large binaries like earth_debugger) on a heavily loaded runner (e.g. GitHub Actions running -race integration tests):
  • If the registry or runner takes > 50ms between reading/streaming chunks, isNetTimeout(err) triggers.
  • CopyWithDeadline treats the response as finished, exits the loop, and returns nil.
  • Server.Proxy finishes and closes the TCP connection and gRPC stream.
  • Docker's HTTP client was still expecting the rest of the payload (matching Content-Length) and fails with:
    failed to copy: httpReadSeeker: failed open: failed to do request: Get "...": EOF: exit status 1

---

Suggested Fixes

1. Fix StreamRW.Read in buildkit/api/services/registry/server.go

Return buffered bytes immediately without calling Recv():

// Read implements io.Reader.
func (s *StreamRW) Read(p []byte) (int, error) {
	if len(s.last) > 0 {
		n := copy(p, s.last)
		s.last = s.last[n:]
		return n, nil
	}

	msg, err := s.stream.Recv()
	if err != nil {
		return 0, err
	}

	s.last = msg.GetData()
	n := copy(p, s.last)
	s.last = s.last[n:]
	return n, nil
}

2. Robust HTTP Response Framing in Server.Proxy

Instead of relying on a 50ms TCP silence heuristic:

• Parse the HTTP response headers (or Content-Length / chunked body) so the proxy knows exactly when the HTTP response body has finished streaming.
• Alternatively, inject Connection: close into the proxied request headers so the embedded registry closes the connection cleanly at the end of each response, enabling clean EOF detection without artificial deadlines.

[kmannislands]

Ah yes, my agents have stumbled upon the same a few different times.

I think most comprehensive was when I sent fable at a full review of our buildkitd fork, highlighted the same points as Janis.

Full review here so I don't lose it:
FORK-REVIEW.md

[kmannislands]

Created #912 to track the symptom. I'll have a look at the backend fix as well. I have a scenario that reproduces this pretty reliably that I can test with at o8t.

[kmannislands]

Raised EarthBuild/buildkit#24
Checks out. @gilescope I'll stack something on top of your PR here when the buildkit side is ready.
Realized we need to revive CI there so I'm working on that first.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8555f0cb-b5bc-4757-8086-1bd2eb7d1df7

📥 Commits

Reviewing files that changed from the base of the PR and between 9be3f25 and 5cce955.

📒 Files selected for processing (2)

• util/dockerutil/docker.go
• util/dockerutil/pullretry_test.go

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Local Docker image pulls now retry failed pulls up to three times with increasing delays. Context cancellation interrupts delays. Tests cover transient recovery and persistent failure behavior.

Changes

Docker pull retry

Layer / File(s)	Summary
Retry flow and validation util/dockerutil/docker.go, util/dockerutil/pullretry_test.go	pullWithRetry retries failed pulls up to three times, waits 150 ms and 300 ms between attempts, returns context errors during waits, and returns the final pull error after exhaustion. Local image pulls use this function. Tests verify recovery, tagging, attempt counts, and persistent EOF failures.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Unblocks: 1 PR

Merge Risk: ⚪ Minimal · up to 5cce9

Local registry pull failures now receive bounded retries without leaving an actionable correctness or availability risk in the changed flow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: retrying local registry pulls in dockerutil.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch giles-pullping-retry

---

_{Comment @coderabbitai help to get the list of available commands.}

[janishorsts]

🐑 it!

Combined with @kmannislands's stacked PR, this should be fixed.