Skip to content

fix(ci): retry docker run on name conflicts and give test retries fresh container names#24477

Closed
vezenovm wants to merge 6 commits into
mv/e2e-onchain-delivery-harnessfrom
mv/ci-docker-name-race-retry
Closed

fix(ci): retry docker run on name conflicts and give test retries fresh container names#24477
vezenovm wants to merge 6 commits into
mv/e2e-onchain-delivery-harnessfrom
mv/ci-docker-name-race-retry

Conversation

@vezenovm

@vezenovm vezenovm commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #24469. The inspect-polling loop there waits for a stale same-name container to disappear before docker run --name, but dockerd can release the name in its registry a beat after the container stops being inspectable, so docker run can still fail with "Conflict, name already in use".

Changes:

  • ci3/run_test_cmd: before the flake retry, export a unique NAME_POSTFIX (_retry_<uuid>) so the retry's docker_isolate containers and run_compose_test compose projects never contend with the failed attempt's in-progress teardown at all. Appends to any existing postfix (grind slots).
  • ci3/docker_isolate: wrap docker run in a bounded retry (6 attempts, exponential backoff totaling ~30s) that on an "already in use by container" error force-removes the name's holder and retries. Any other error still exits immediately with docker's exit code, and docker's stderr (including warnings on success) still reaches the test log.
  • ci3/docker_isolate: drop fix(ci): wait for dockerd to finish removing a stale container before reusing its name #24469's inspect-polling block, which is redundant now that the retry loop covers the same race with the same ~30s budget (the initial best-effort docker rm -f stays so the common stale-container case doesn't burn a retry).

Known gap, out of scope: the ha case in yarn-project/end-to-end/scripts/run_test.sh overrides NAME_POSTFIX, so HA compose retries rely on compose's own project teardown rather than the fresh postfix.

Verified against a stubbed docker binary: conflict-then-success recovers (fresh attempt after rm -f), persistent conflict exits non-zero after 6 attempts (~30s of backoff), unrelated errors don't retry, and cleanup removes both the stale name and the final cid.

Based on #24373 so it can be exercised end to end against the automine cluster before merging down to merge-train/fairies-v5.

…etries fresh names

The polling loop from #24469 waits for a stale same-name container to
disappear before docker run, but dockerd can still release the name a beat
after the container stops being inspectable. Wrap docker run in a bounded
retry that force-removes the name's holder on 'already in use' conflicts.

Also export a unique NAME_POSTFIX before the flake retry in run_test_cmd so
the retry's containers/compose projects never contend with the failed
attempt's teardown in the first place.
@vezenovm
vezenovm requested a review from charlielye as a code owner July 2, 2026 20:08
@vezenovm vezenovm added ci-full Run all master checks. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure ci-full-no-test-cache and removed ci-full Run all master checks. labels Jul 2, 2026
vezenovm added 3 commits July 2, 2026 16:36
The #24469 inspect-poll waited up to 30s for a stale container's name to
free before docker run, but the retry loop now covers the same race
(including the window after the container stops being inspectable) while
actively force-removing the name's holder between attempts. Keep the
initial best-effort docker rm -f so the common stale-container case does
not burn a retry, and give the retry loop exponential backoff
(1+2+4+8+16s, ~30s total) to preserve the old tolerance window.
@vezenovm vezenovm added ci-full Run all master checks. and removed ci-full-no-test-cache labels Jul 6, 2026
#24547)

Stacked on #24477. Fixes the 49-per-run automine FLAKEs reported there
and on this PR's first run.

**Root cause** (`yarn-project/end-to-end/bootstrap.sh`): the `tests`
array listed the automine subdirectories explicitly and also kept the
`src/automine/!(simulation)/**/*.test.ts` catch-all (a merge artifact
from merging merge-train/fairies-v5 into
mv/e2e-onchain-delivery-harness), so every nested automine test was
emitted as two byte-identical cmds (268 jobs for 145 tests). Both copies
run under the same docker container name. When the two copies overlap in
time, the later one's docker_isolate force-removes the earlier one's
live container (or wins the name-conflict retry added in #24477), the
earlier test fails, and its automatic retry (which gets a fresh
`NAME_POSTFIX`) passes: a deterministic FLAKED-with-code-0 for every
overlapping pair, which is why the count sits near 49 on every run and
why only `src/automine/**` flakes. When the copies do not overlap, the
second is skipped by the test cache, hiding the duplication. The parent
branch's "docker container name conflicts" were the same duplication
seen from the other side.

- Remove the seven redundant per-directory globs; the deduplicated
unique cmd set is unchanged (verified byte-identical before/after, 145
automine jobs, zero duplicates in both `CI_FULL` modes).
@vezenovm

vezenovm commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Closing. This code is defensive but I was able to resolve the container conflicts with the test deduplication. If we don't need it I would rather not add it.

@vezenovm vezenovm closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-full Run all master checks. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant