fix(ci): retry docker run on name conflicts and give test retries fresh container names#24477
Closed
vezenovm wants to merge 6 commits into
Closed
fix(ci): retry docker run on name conflicts and give test retries fresh container names#24477vezenovm wants to merge 6 commits into
vezenovm wants to merge 6 commits into
Conversation
…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.
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.
#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).
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. |
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.
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, sodocker runcan still fail with "Conflict, name already in use".Changes:
ci3/run_test_cmd: before the flake retry, export a uniqueNAME_POSTFIX(_retry_<uuid>) so the retry'sdocker_isolatecontainers andrun_compose_testcompose projects never contend with the failed attempt's in-progress teardown at all. Appends to any existing postfix (grind slots).ci3/docker_isolate: wrapdocker runin 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-effortdocker rm -fstays so the common stale-container case doesn't burn a retry).Known gap, out of scope: the
hacase inyarn-project/end-to-end/scripts/run_test.shoverridesNAME_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.