From 6a2129c7eee019d6404be70c7c18800902b2cf0b Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 28 Jul 2026 10:43:24 +0200 Subject: [PATCH] Recreate the shared external network on a lost teardown race 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 --- pkg/container/docker/client.go | 35 +++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/pkg/container/docker/client.go b/pkg/container/docker/client.go index b8ab1f73a0..99e038747f 100644 --- a/pkg/container/docker/client.go +++ b/pkg/container/docker/client.go @@ -1458,7 +1458,34 @@ func (c *Client) createContainer( EndpointsConfig: endpointsConfig, } - // Create the container + id, err := c.createAndStartContainer(ctx, containerName, config, hostConfig, networkConfig) + if err != nil && errdefs.IsNotFound(err) { + if _, sharesExternalNetwork := endpointsConfig["toolhive-external"]; sharesExternalNetwork { + // "toolhive-external" is shared, concurrency-created infrastructure + // reused by every workload; deleteNetworks removes it opportunistically + // once it looks like no workload needs it anymore. Under concurrent + // `thv` processes that teardown can race this container's own creation, + // so the network can vanish between our create-time existence check and + // this container actually attaching to it, surfacing as a not-found + // error from either Create or Start. Recreate it and retry once instead + // of failing the whole workload for what is a transient condition. + if recreateErr := c.createExternalNetworks(ctx); recreateErr == nil { + id, err = c.createAndStartContainer(ctx, containerName, config, hostConfig, networkConfig) + } + } + } + return id, err +} + +// createAndStartContainer creates and starts a container, wrapping any +// Docker API error with contextual information. +func (c *Client) createAndStartContainer( + ctx context.Context, + containerName string, + config *container.Config, + hostConfig *container.HostConfig, + networkConfig *network.NetworkingConfig, +) (string, error) { resp, err := c.api.ContainerCreate(ctx, mobyclient.ContainerCreateOptions{ Config: config, HostConfig: hostConfig, @@ -1477,6 +1504,12 @@ func (c *Client) createContainer( // Start the container _, err = c.api.ContainerStart(ctx, resp.ID, mobyclient.ContainerStartOptions{}) if err != nil { + // Docker leaves the container behind in a "created" state on a failed + // start (e.g. its network vanished underneath it). Clean it up so a + // caller-driven retry under the same name doesn't collide with it. + if _, rmErr := c.api.ContainerRemove(ctx, resp.ID, mobyclient.ContainerRemoveOptions{Force: true}); rmErr != nil { + slog.Debug("failed to remove container after failed start", "id", resp.ID, "error", rmErr) + } return "", NewContainerError(err, resp.ID, fmt.Sprintf("failed to start container: %v", err)) }