fix(ecs,lambda): a transient image pull failure no longer fails the launch - #2516
Merged
Merged
Conversation
A bare `docker pull` always contacts the registry, even for an image already in the local cache, so any registry error failed the launch. Anonymous pulls from public.ecr.aws are rate limited per source IP, and a burst of task launches -- or a shared CI runner address -- gets `429 Too Many Requests`. The ECS task then stopped with TaskFailedToStart and every Batch job on it went FAILED. That is the cause of the batch_real_execution e2e flakes seen on nearly every CI run: array, depends_on, timeout, retry and submit tests all launch alpine containers concurrently. Reproduced by running the suite with a container CLI that answers every pull with a 429: the old pull path fails 6 of 7 tests with the 429 in statusReason, the new one passes all 7. The ECS agent's default ECS_IMAGE_PULL_BEHAVIOR uses the cached image when a pull fails and retries pulls with backoff. The new fakecloud_core::container_image::pull_image does the same and is used by the ECS task runtime (so Batch) and by Lambda PackageType=Image function starts and prewarms: - pull succeeds -> use it - pull fails, image cached -> use the cached image, log the pull error - pull rate limited, nothing cached -> retry with backoff (5 attempts) - any other failure with nothing cached -> fail at once with the registry's error, so a missing image still fails fast The Batch e2e wait helper now asserts the expected terminal status itself and, on a mismatch, prints each job's status, statusReason and exit code, including every array child, so a CI failure names its cause.
A refused pull -- an image deleted from the registry, or one a repository policy denies -- must fail the launch even when an earlier launch left a copy in the local cache. Neither Fargate nor Lambda has a per-host image cache, so on AWS such a pull never succeeds. Restrict both the cache fallback and the retry to transient failures: throttling, registry 5xx, and network timeouts or dropped connections.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ent failure - Check refusal wording (not found, denied, unauthorized, forbidden) first, on the unmodified message, so a refused pull of a repository named like a throttling code (toomanyrequests) is not read as transient and does not launch a stale cached copy. A repository name cannot contain a space, so it can only spell the one-word markers. - Treat any 5xx status as transient, parsed from the status position, instead of four fixed reason phrases. - The Batch e2e diagnostics no longer index an empty DescribeJobs result.
…e cache on the same daemon - Remove the image's own name (the reference, and each trailing path of its repository) from the error before matching, whole names only. A throttled pull of a repository named like a refusal (acme/access-denied-page) is transient again, and a refused pull of one named like a throttle code stays refused. - Run the cache check with the pull's DOCKER_CONFIG. The config selects the Docker context, so without it the check could consult a different daemon than the one that pulls and runs the image.
…ssifying it URLs in the error quote the registry host on its own, so a host named like a refusal (denied.example) would have hidden a 429 from it.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…med reference The fake CLI pull always used alpine:3.20, so the marker-named repository test never exercised name removal. The fake now takes the reference, and a pull-level test covers a throttled pull of acme/access-denied-page falling back to the cache, which fails if the name is not removed first.
This was referenced Sep 13, 2026
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
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.
Summary
The
batch_real_executione2e tests flake on nearly every CI run, onmainand on every PR:array_job_runs_every_child_and_parent_succeedsneeds a retry in almost every E2E general partition, anddepends_on_job_waits_for_its_dependency,timeout_fails_an_overrunning_job,retry_strategy_reattempts_a_failing_joband bothsubmit_job_*tests show up too. On #2510 the array test lost all three tries.Cause
A bare
docker pullalways contacts the registry, even when the image is already cached. Anonymous pulls frompublic.ecr.awsare rate limited per source IP, and the Batch tests launch several alpine containers at once from a shared GitHub runner address. The registry answers429 Too Many Requests, the ECS task stops withTaskFailedToStart, and the Batch job goesFAILED.Confirmed rather than inferred:
alpine:3.20from one machine: one came back429 Too Many Requests.pullwith that 429 and passes everything else to docker: onmain's pull path, 6 of 7 tests fail (exactly the ones that flake in CI), each child'sstatusReasonreadingimage pull failed: ... 429 Too Many Requests. With this change, 7 of 7 pass.Fix
fakecloud_core::container_image::pull_image, used by the ECS task runtime (so Batch) and by LambdaPackageType=Imagefunction starts and prewarms (the same unconditional pull lived in both places; RDS already falls back):Transient = throttling (
429 Too Many Requests/toomanyrequests/Rate exceeded), a registry 5xx, or a network timeout or dropped connection. A refused pull stays final on purpose: an image deleted from ECR, or one a repository policy denies, must not keep launching from a local copy. Neither Fargate nor Lambda has a per-host image cache, so on AWS that pull never succeeds. A missing image still fails fast, soecs_run_task_missing_ecr_repo_fails_fastis unaffected.Diagnosability
The Batch e2e wait helper now asserts the expected terminal status itself. On a mismatch it prints each job's
status,statusReasonand exit code, including every array child. Before, a failure read onlyleft: "FAILED" right: "SUCCEEDED"and the server's stderr is not captured by CI, which is why this sat unexplained.Surfaces
website/content/docs/services/ecs.md(task execution step 1) andlambda.md(gotchas) describe the pull behavior.Test plan
fakecloud-coredriving a fake container CLI: success, throttled with cache, 503 with cache, throttled then success, attempt cap, missing image not retried, refused pull (not found / access denied) failing despite a cached copy, and transient-vs-refused classification.batch_real_executionwith real Docker: 7/7, and 7/7 with every pull forced to 429. Reverting the ECS pull path under the same forced 429: 6/7 fail with the 429 instatusReason(the fix is non-vacuous).cargo clippy --workspace --all-targets -- -D warningsclean;cargo fmt --checkclean.ecr_cross_serviceneeds Linux (the daemon must reach the local registry); CI runs it.Summary by cubic
Fixes
batch_real_executione2e flakes caused by transient image pull failures like429 Too Many Requestsfrompublic.ecr.aws; these no longer fail ECS and LambdaPackageType=Imagelaunches.DOCKER_CONFIGas the pull, so it inspects the same daemon.Written for commit 147a0b1. Summary will update on new commits.