From 0200c6441904139480f9d5447681964e1c5f01ab Mon Sep 17 00:00:00 2001 From: Erik Osterman Date: Wed, 5 Aug 2026 19:46:06 -0500 Subject: [PATCH 1/3] fix(ci): pull docker buildx/binfmt images from Google mirror, not AWS ECR The release Docker build job was rate-limited pulling its buildx builder and QEMU binfmt images from public.ecr.aws. Bumps cloudposse/github-action-docker-build-push to v3.1.0, which switches the buildkit image default to mirror.gcr.io, and overrides binfmt-image to the equivalent Google-mirrored tonistiigi/binfmt image. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c1441a0ed7..f1baef25af 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -50,7 +50,7 @@ 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 }}" @@ -58,6 +58,7 @@ jobs: login: "${{ github.actor }}" password: "${{ secrets.GITHUB_TOKEN }}" platforms: linux/amd64,linux/arm64 + binfmt-image: mirror.gcr.io/tonistiigi/binfmt:qemu-v7.0.0 file: Dockerfile build-args: | ATMOS_VERSION=${{ github.event.release.tag_name }} From 3339e0276324e1e7011036f1d8d4ef52a590947d Mon Sep 17 00:00:00 2001 From: Erik Osterman Date: Wed, 5 Aug 2026 21:09:59 -0500 Subject: [PATCH 2/3] fix(output): serialize whole flush, not per-line, in LinePrefixWriter writeLine acquired/released the shared writeMu once per line, so a single Write() call that produced multiple lines (e.g. a hook's buffered "\r"-then-"\n" progress update) could have another node's writer interleave a line in between, corrupting concurrent Terraform node output. Hold writeMu for the entire flush instead. Reproduced with `go test -race -count=200` on TestExecuteTerraformConcurrentHooksUseNodeWriters, which was flaking in CI (Acceptance Tests macos, job 92489708710); now passes 200/200. Co-Authored-By: Claude Sonnet 5 --- pkg/io/line_prefix_writer.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/io/line_prefix_writer.go b/pkg/io/line_prefix_writer.go index fab9487956..c65ecec862 100644 --- a/pkg/io/line_prefix_writer.go +++ b/pkg/io/line_prefix_writer.go @@ -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 } @@ -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 } @@ -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) @@ -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) From 119ba5491ce4f56ec45dc64bfe583479f794e2bf Mon Sep 17 00:00:00 2001 From: Erik Osterman Date: Thu, 6 Aug 2026 09:34:31 -0500 Subject: [PATCH 3/3] docs(fixes): add fix records for Docker build mirror and writer race Co-Authored-By: Claude Sonnet 5 --- ...ker-build-buildkit-binfmt-google-mirror.md | 62 +++++++++++++++++++ ...ine-prefix-writer-concurrent-flush-race.md | 52 ++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 docs/fixes/2026-08-06-docker-build-buildkit-binfmt-google-mirror.md create mode 100644 docs/fixes/2026-08-06-line-prefix-writer-concurrent-flush-race.md diff --git a/docs/fixes/2026-08-06-docker-build-buildkit-binfmt-google-mirror.md b/docs/fixes/2026-08-06-docker-build-buildkit-binfmt-google-mirror.md new file mode 100644 index 0000000000..0e87418565 --- /dev/null +++ b/docs/fixes/2026-08-06-docker-build-buildkit-binfmt-google-mirror.md @@ -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. diff --git a/docs/fixes/2026-08-06-line-prefix-writer-concurrent-flush-race.md b/docs/fixes/2026-08-06-line-prefix-writer-concurrent-flush-race.md new file mode 100644 index 0000000000..86bbf7128f --- /dev/null +++ b/docs/fixes/2026-08-06-line-prefix-writer-concurrent-flush-race.md @@ -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.