Recreate the shared external network on a lost teardown race - #6083
Merged
Conversation
deleteNetworks removes the shared "toolhive-external" Docker network opportunistically whenever a workload teardown observes zero remaining toolhive-labeled containers -- a point-in-time check with no cross-process synchronization. Since concurrent Ginkgo processes (or any concurrent `thv run` invocations) share one Docker daemon, one process's teardown can delete the network while another's container is mid-creation and still needs it, surfacing as "network toolhive-external not found" from ContainerCreate/ContainerStart. Observed in CI: a DNS sidecar container failed this way in the "core" e2e job after it moved to 4 concurrent Ginkgo processes. Recreate the network and retry once when this specific not-found condition is hit, and clean up any container Docker leaves behind in "created" state after a failed start so the retry doesn't collide on the same name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jhrozek
requested review from
ChrisJBurns,
JAORMX,
amirejaz,
blkt and
rdimitrov
as code owners
July 28, 2026 08:43
JAORMX
approved these changes
Jul 28, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6083 +/- ##
==========================================
- Coverage 72.30% 72.30% -0.01%
==========================================
Files 728 728
Lines 75496 75505 +9
==========================================
+ Hits 54590 54591 +1
- Misses 16987 16991 +4
- Partials 3919 3923 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
pkg/container/docker/client.go'sdeleteNetworksremoves theshared
toolhive-externalDocker network opportunistically whenever aworkload teardown observes zero remaining
toolhive=true-labeledcontainers — a point-in-time check with no cross-process synchronization.
Since concurrent Ginkgo processes (or any concurrent
thv runinvocationson one host) share a single Docker daemon, one process's teardown can
delete the network while another process's container is mid-creation and
still needs it, surfacing as
network toolhive-external not foundfromContainerCreate/ContainerStart. Hit live in CI on PR Retry dual-era backend startup on a lost port-bind race #6080: a DNSsidecar container failed this way in the
E2E Tests Core (core)job afterit moved to 4 concurrent Ginkgo processes (Speed up e2e(core) suite with Ginkgo procs and concurrent waits #6070) sharing one daemon.
createContainernow retries once, recreatingtoolhive-externalfirst, if container create/start fails with anot-found error and the container's endpoint config references that
network.
createAndStartContainer(extracted from the existingcreate+start logic, unchanged) also force-removes a container Docker
leaves behind in "created" state after a failed start, so the retry
doesn't collide with it on the same name.
Type of change
Test plan
go build ./...andgo vet ./pkg/container/docker/...both pass cleanly.Not yet covered by a unit or e2e test — this was root-caused and fixed from
a single CI occurrence rather than a local stress reproduction. Suggested
verification: run the "core" e2e suite with concurrent procs against a
workload-churn script that creates/removes workloads on the same daemon
while another spec is mid-creation, to force the teardown-vs-creation race
directly (e.g.
ginkgo --label-filter='core && groups' --repeat=20 -v ./test/e2e/ -- --procs=4).API Compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.Does this introduce a user-facing change?
No. Internal container-runtime resilience fix; no API or CLI surface change.
Special notes for reviewers
This closes the loop on the opposite side of the same shared-network
lifecycle that #6071 already made resilient to concurrent creation races
(treating
NetworkCreateconflict as success). This PR handles theteardown side: a concurrent deletion racing another workload's creation.
No local stress reproduction was performed for this one (unlike the
port-bind-race fix in #6080) — the root cause and fix were derived from
tracing the actual CI failure and the relevant code paths. Flagging this
explicitly since it means the fix is confidence-based on code inspection
rather than confirmed-live-under-stress; happy to attempt a local repro if
useful before merge.
Generated with Claude Code