Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ jobs:

- name: "Docker Build"
id: build
uses: cloudposse/github-action-docker-build-push@f06d0f4bd286898b613412d2fcc6622e5b68bbdc # v3.0.0
uses: cloudposse/github-action-docker-build-push@02993d675b44dcc7082e6de7485c1ff8740bce9d # v3.1.0
with:
registry: ghcr.io
organization: "${{ github.event.repository.owner.login }}"
repository: "${{ github.event.repository.name }}"
login: "${{ github.actor }}"
password: "${{ secrets.GITHUB_TOKEN }}"
platforms: linux/amd64,linux/arm64
binfmt-image: mirror.gcr.io/tonistiigi/binfmt:qemu-v7.0.0
Comment thread
coderabbitai[bot] marked this conversation as resolved.
file: Dockerfile
build-args: |
ATMOS_VERSION=${{ github.event.release.tag_name }}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Fix: Release Docker build no longer pulls buildkit/binfmt images from AWS public ECR

**Date:** 2026-08-06

## Summary

The release `docker` job in `.github/workflows/build.yml` was rate-limited pulling its buildx
builder (`moby/buildkit`) and QEMU binfmt images from `public.ecr.aws`. Bumped
`cloudposse/github-action-docker-build-push` to v3.1.0 (Google-mirrored buildkit image by
default) and explicitly overrode the action's `binfmt-image` input to the equivalent
Google-mirrored `tonistiigi/binfmt` image, since that input still defaults to `public.ecr.aws`
even at v3.1.0.

## Context

CI run for `cloudposse/atmos` release build (job at
`https://github.com/cloudposse/atmos/actions/runs/31040133433/job/92444923915`) failed with
`jq: error (at inspect.json:78): Cannot iterate over null (null)` in the action's post-build
summary step, which turned out to be a separate, already-tracked upstream bug
(`cloudposse/github-action-docker-build-push#102`, cosmetic — the image itself built and pushed
fine) and not something fixable from this repo. On the next attempted run, the job failed again,
this time on rate limiting while pulling the buildx builder image from `public.ecr.aws`.

## Changes

- `.github/workflows/build.yml`: bumped
`uses: cloudposse/github-action-docker-build-push@...` from v3.0.0
(`f06d0f4bd286898b613412d2fcc6622e5b68bbdc`) to v3.1.0
(`02993d675b44dcc7082e6de7485c1ff8740bce9d`), which changes the action's `driver-opts` default
from `image=public.ecr.aws/vend/moby/buildkit:buildx-stable-1` to
`image=mirror.gcr.io/moby/buildkit:buildx-stable-1`.
- `.github/workflows/build.yml`: added an explicit `binfmt-image:
mirror.gcr.io/tonistiigi/binfmt:qemu-v7.0.0` input, since the action's `binfmt-image` default
(`public.ecr.aws/eks-distro-build-tooling/binfmt-misc:qemu-v7.0.0`) still pulls from AWS public
ECR and has no upstream fix yet. `binfmt-image` passes straight through to
`docker/setup-qemu-action`'s `image` input, so it can be overridden directly without waiting on
upstream.

## Validation

- Diffed `cloudposse/github-action-docker-build-push` v3.0.0...v3.1.0 upstream: only the
`driver-opts` default change and an unrelated arm64 `jq` install fix; no input/output contract
changes to any input this workflow uses (`registry`, `organization`, `repository`, `login`,
`password`, `platforms`, `file`, `build-args`).
- Confirmed `public.ecr.aws/eks-distro-build-tooling/binfmt-misc:qemu-v7.0.0` is an AWS rebuild of
upstream `tonistiigi/binfmt`, which publishes the identical `qemu-v7.0.0` tag on Docker Hub.
- Verified live against the registries: `docker buildx imagetools inspect
mirror.gcr.io/tonistiigi/binfmt:qemu-v7.0.0` and the `docker.io/tonistiigi/binfmt:qemu-v7.0.0`
equivalent both resolve to the same digest
(`sha256:66e11bea77a5ea9d6f0fe79b57cd2b189b5d15b93a2bdb925be22949232e4e55`) across all 7
published platforms, and `docker pull` of the mirrored tag succeeds.
- This is a `release`-triggered workflow (`on.release.types: [published]`), so it cannot be
exercised by a normal PR run; verification here is by inspection plus the live registry checks
above. The next actual release's Docker build job should be watched once to confirm no more
rate-limit failures.
- `./custom-gcl run --new-from-rev=origin/main` — 0 issues.

## Follow-ups

None. The action's `binfmt-image` default itself is still AWS-ECR-backed upstream with no fix in
flight; if it starts rate-limiting independently of this override, no further action is needed
here since this repo already pins its own Google-mirrored value.
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Fix: `LinePrefixWriter` held its shared output lock per line instead of per flush

**Date:** 2026-08-06

## Summary

`pkg/io/line_prefix_writer.go`'s `writeLine` acquired and released the shared `writeMu` once per
line instead of once per flush. When a single `Write()` call produced multiple lines (e.g. a
hook's buffered `"progress\r" + "complete\n"` update), the lock was released between those lines,
letting a concurrently running node's writer interleave its own line in between and corrupt the
expected per-node contiguous output block. `Write` and `Flush` now hold `writeMu` for their whole
flush operation, and `writeLine` is a lock-free helper that callers must call while holding it.

## Context

The macOS "Acceptance Tests" CI job (job ID 92489708710) failed with:

```
--- FAIL: TestExecuteTerraformConcurrentHooksUseNodeWriters (0.00s)
terraform_test.go:510:
Error: "[dev/app] hook progress\n[dev/db] hook progress\n[dev/db] hook complete\n[dev/app] hook complete\n" does not contain "[dev/app] hook progress\n[dev/app] hook complete\n"
```

Both `pkg/io/line_prefix_writer.go` and this test landed together in a prior PR (#2860, "render
concurrent carriage-return updates safely"), which correctly converts `\r` progress updates into
discrete prefixed lines but left a lock-granularity gap that let two concurrent nodes' lines
interleave mid-block. Locally the test only failed intermittently (timing-dependent), which is
why it passed in earlier local runs before reproducing it under `-race -count=N`.

## Changes

- `pkg/io/line_prefix_writer.go`: `Write()` and `Flush()` now acquire `w.writeMu` once, covering
the entire call to `flushCompleteLinesLocked()` (and, in `Flush()`, the trailing partial-line
write too), instead of `writeLine()` acquiring/releasing `writeMu` on every individual line.
`writeLine()` no longer touches `writeMu` itself; its doc comment now states callers must hold
it. Lock ordering is unchanged (per-writer `w.mu` outer, shared `writeMu` inner), so this
doesn't introduce new deadlock risk.

## Validation

- Reproduced the race before the fix: `go test ./pkg/scheduler/adapters/... -run
TestExecuteTerraformConcurrentHooksUseNodeWriters -race -count=200` failed intermittently
(multiple failures across 200 iterations, each showing the same interleaved-block pattern).
- After the fix, the same command passed 200/200 under `-race`.
- Full suites for touched/adjacent packages passed under `-race -count=1`: `pkg/io`,
`pkg/scheduler`, `pkg/scheduler/adapters`, `pkg/component/container`, `pkg/workflow`.
- `./custom-gcl run --new-from-rev=origin/main` — 0 issues.
- `gofmt -l pkg/io/line_prefix_writer.go` — no output (already formatted).

## Follow-ups

None.
16 changes: 13 additions & 3 deletions pkg/io/line_prefix_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ func (w *LinePrefixWriter) Write(p []byte) (int, error) {
}

w.buffer = append(w.buffer, p...)

// Hold writeMu for the whole flush so that every line produced by this
// single Write call reaches the shared writer as one contiguous block.
// Locking per-line let a concurrent node's writer interleave a line in
// between two lines emitted from the same Write call.
w.writeMu.Lock()
defer w.writeMu.Unlock()
if err := w.flushCompleteLinesLocked(); err != nil {
return 0, err
}
Expand All @@ -81,6 +88,10 @@ func (w *LinePrefixWriter) Flush() error {
if len(w.buffer) == 0 {
return nil
}

w.writeMu.Lock()
defer w.writeMu.Unlock()

if err := w.flushCompleteLinesLocked(); err != nil {
return err
}
Expand All @@ -95,7 +106,7 @@ func (w *LinePrefixWriter) Flush() error {
return nil
}

// flushCompleteLinesLocked writes buffered complete lines while w.mu is held.
// flushCompleteLinesLocked writes buffered complete lines while w.mu and w.writeMu are held.
func (w *LinePrefixWriter) flushCompleteLinesLocked() error {
for {
idx := lineEndIndex(w.buffer)
Expand All @@ -115,12 +126,11 @@ func (w *LinePrefixWriter) flushCompleteLinesLocked() error {
}

// writeLine writes one already-delimited line with the configured prefix.
// Callers must hold w.writeMu.
func (w *LinePrefixWriter) writeLine(line []byte) error {
if w.w == nil {
return nil
}
w.writeMu.Lock()
defer w.writeMu.Unlock()

line = bytes.ReplaceAll(line, crlfBytes, lfBytes)
line = bytes.ReplaceAll(line, crBytes, lfBytes)
Expand Down
Loading