ci: pre-pull Docker Hub images with retries in start-services - #3410
Conversation
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 1a51f6d. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Incomplete Hub image pre-pull list
- Added clickhouse/clickhouse-server:25.4.5.24 to the pre-pull loop in the Docker Hub images step to prevent Hub 401/429 failures during the Clickhouse service startup.
Or push these changes by commenting:
@cursor push d4b1623a09
Preview (d4b1623a09)
diff --git a/.github/actions/start-services/action.yml b/.github/actions/start-services/action.yml
--- a/.github/actions/start-services/action.yml
+++ b/.github/actions/start-services/action.yml
@@ -37,7 +37,7 @@
- name: Pre-pull Docker Hub images
shell: bash
run: |
- for image in postgres:latest redis:latest otel/opentelemetry-collector-contrib:0.146.0; do
+ for image in postgres:latest redis:latest otel/opentelemetry-collector-contrib:0.146.0 clickhouse/clickhouse-server:25.4.5.24; do
for attempt in 1 2 3 4 5; do
docker pull --quiet "$image" && break
if [ "$attempt" = 5 ]; then echo "giving up on $image"; exit 1; fiYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit b074e18. Configure here.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
LGTM — straightforward CI reliability fix.
Reviewed the composite action's new pre-pull step and the three subsequent service steps (Postgres, Clickhouse via packages/clickhouse, Redis) to confirm the images they invoke (postgres:latest, redis:latest, clickhouse/clickhouse-server:25.4.5.24) are all covered. The otel-collector image pulled by ./scripts/start-service.sh is also included. Retry/backoff loop is simple bash with a bounded attempt count and clear failure exit, no risk of infinite hangs.
Extended reasoning...
Overview
Single-file, CI-only change to .github/actions/start-services/action.yml that adds a first step pre-pulling four Docker Hub images (postgres, redis, otel-collector, clickhouse-server) with a bounded retry/backoff loop (5 attempts, linear backoff), to avoid job failures from transient Docker Hub rate-limiting (401/429) during later implicit docker pulls.
Security risks
None. This is a CI workflow change with no production code path, no secrets handling changes, and no new inputs from untrusted sources — the image list is a fixed set of hardcoded, already-used tags.
Level of scrutiny
Low. This is a mechanical reliability fix to test infrastructure, self-contained to one composite action file, with a bounded retry loop and clear failure behavior (exits non-zero after 5 failed attempts, so it fails visibly rather than hanging or silently masking errors).
Other factors
Cursor's bugbot had flagged an earlier revision for omitting the clickhouse-server image from the pre-pull list; that was addressed in a follow-up commit (visible in recent commit history) and the image is now present in the list shown in the diff. The bug hunting system found no issues in the final version, and ruled out two related candidates about buildx-pulled base images (golang/alpine) not being covered — those are pulled via a separate caching path (docker/build-push-action with GHA cache) and aren't materially exposed to the same anonymous-Hub-pull rate limiting this PR addresses.
) ## What Rebase + extension of #3235 (cc @dobrac), squashed onto current main after #3462 landed. Opened as a **draft for a measuring run** — the shard timings from this PR's CI decide the final shard balance. Two things changed since #3235 was written, and this branch reconciles both: - #3462 already landed the coverage philosophy (uncompressed runs the whole suite once; zstd1/lz4 only re-run the allow-list). This rebase **keeps that decision** and applies #3235's sharding on top — PRs now shard the *uncompressed* config, not zstd1. - The distro-family template tests (#3437/#3444) landed into what is now templates shard 2, so the name-prefix split needs rebalancing from this run's junits. ## PR-path matrix (8 jobs; push to main unchanged: 3 unsharded configs) | Job | Shard | Runs | |---|---|---| | uncompressed-templates-1/2 | `^TestTemplateBuild` split by name prefix | 11 + 9 real-build tests | | uncompressed-sandboxes | sandboxes + metrics + volumes + proxies | 121 tests | | uncompressed-rest | everything else, `-skip '^TestTemplateBuild'` | 86 tests | | zstd1-templates / lz4-templates | templates package ∩ allow-list | 6 snapshot-build entries | | zstd1-other / lz4-other | package complement ∩ allow-list | 33 entries | Static partition verified: 227 top-level tests land exactly once across the uncompressed shards; templates/no-templates is an exact package complement. `TESTS_ONLY` composes only with package-pure shards (the name-split shards' own `-run` would collide — guarded, fails loudly). Unknown shard names fail loudly instead of silently running the whole suite. ## Carried from #3235 (unchanged in spirit) - DB containers start right after checkout and overlap the Go builds (`start-databases`); `start-services` waits with bounded health loops. Docker Hub pre-pull retries (#3410) folded in, now covering the pinned tags + otel. - ClickHouse migrations via goose on the host (`migrate-host`) — no migrator image build. - Salted optimized Go cache; `DEBUG_GCFLAGS` opt-out (race stays on, `-N -l` off in CI). envd keeps the musl-static link from #3444, gcflags parametrized. - 7 metadata/authz template tests use `RequestTemplateWithoutBuild`; envd process tests poll instead of fixed sleeps. ## Expected (to be confirmed by this run) Baseline on main (run 30626752076): integration jobs 8.4–9.9 min, PR wall ~10–12 min. Expected here: **~5–5.5 min wall**, long pole likely uncompressed-templates-2 (it inherited the distro suite). Follow-ups after measurement: rebalance `TEMPLATE_BUILDS_SHARD1_RE`, cache the built sandbox template keyed on inputs. Relationship to #3235: this supersedes the mechanical parts; adopt into the original PR or take this one over — @dobrac's call. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
) ## What Rebase + extension of #3235 (cc @dobrac), squashed onto current main after #3462 landed. Opened as a **draft for a measuring run** — the shard timings from this PR's CI decide the final shard balance. Two things changed since #3235 was written, and this branch reconciles both: - #3462 already landed the coverage philosophy (uncompressed runs the whole suite once; zstd1/lz4 only re-run the allow-list). This rebase **keeps that decision** and applies #3235's sharding on top — PRs now shard the *uncompressed* config, not zstd1. - The distro-family template tests (#3437/#3444) landed into what is now templates shard 2, so the name-prefix split needs rebalancing from this run's junits. ## PR-path matrix (8 jobs; push to main unchanged: 3 unsharded configs) | Job | Shard | Runs | |---|---|---| | uncompressed-templates-1/2 | `^TestTemplateBuild` split by name prefix | 11 + 9 real-build tests | | uncompressed-sandboxes | sandboxes + metrics + volumes + proxies | 121 tests | | uncompressed-rest | everything else, `-skip '^TestTemplateBuild'` | 86 tests | | zstd1-templates / lz4-templates | templates package ∩ allow-list | 6 snapshot-build entries | | zstd1-other / lz4-other | package complement ∩ allow-list | 33 entries | Static partition verified: 227 top-level tests land exactly once across the uncompressed shards; templates/no-templates is an exact package complement. `TESTS_ONLY` composes only with package-pure shards (the name-split shards' own `-run` would collide — guarded, fails loudly). Unknown shard names fail loudly instead of silently running the whole suite. ## Carried from #3235 (unchanged in spirit) - DB containers start right after checkout and overlap the Go builds (`start-databases`); `start-services` waits with bounded health loops. Docker Hub pre-pull retries (#3410) folded in, now covering the pinned tags + otel. - ClickHouse migrations via goose on the host (`migrate-host`) — no migrator image build. - Salted optimized Go cache; `DEBUG_GCFLAGS` opt-out (race stays on, `-N -l` off in CI). envd keeps the musl-static link from #3444, gcflags parametrized. - 7 metadata/authz template tests use `RequestTemplateWithoutBuild`; envd process tests poll instead of fixed sleeps. ## Expected (to be confirmed by this run) Baseline on main (run 30626752076): integration jobs 8.4–9.9 min, PR wall ~10–12 min. Expected here: **~5–5.5 min wall**, long pole likely uncompressed-templates-2 (it inherited the distro suite). Follow-ups after measurement: rebalance `TEMPLATE_BUILDS_SHARD1_RE`, cache the built sandbox template keyed on inputs. Relationship to #3235: this supersedes the mechanical parts; adopt into the original PR or take this one over — @dobrac's call. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>


Integration-test jobs intermittently die in Start Services when an anonymous Docker Hub pull returns "unauthorized: authentication required" (Hub rate limiting on shared runners) — most recently one matrix job on a main push while its 12 siblings passed. Pre-pull the three Hub images (postgres, redis, otel-collector) with retry/backoff at the top of the composite action so the implicit pulls inside service steps always hit the local cache; a transient 401/429 now retries instead of failing the job.