fix(orchestrator): bound the Consul calls behind network slot acquire and release

[tomassrnka]

What was broken

The Consul client backing network-slot allocation is built from consulApi.DefaultConfig(), whose http.Client has no Timeout. An agent that accepts the TCP connection and then goes silent blocks the caller forever — the transport's dial/TLS timeouts do not help once a connection is established.

Release does two such round trips (kv.Get + kv.DeleteCAS) on every network-slot teardown, and Pool.Close waits on in-flight slot returns during shutdown. So a wedged Consul agent could hang a slot return indefinitely and stall the orchestrator's shutdown drain, with nothing able to cut it short. Acquire had the same exposure on the create path — up to 10 kv.CAS round trips plus a kv.Keys scan and a fallback loop over 32766 slots — while taking a context.Context it then discarded entirely (func (s *StorageKV) Acquire(_ context.Context)).

Fix

Two bounds, no interface change:

• Explicit http.Client.Timeout on the Consul client. This is the load-bearing part: it caps every Consul call regardless of caller, including the teardown paths that deliberately run on non-cancellable contexts.
• Release gets its own deadline so a whole release (both round trips) cannot outlive it.
• Acquire now honours the context it was already given — WithContext on every query/write, plus a ctx.Err() bail-out inside the fallback scan, which otherwise walks all 32766 entries (skipping reserved slots costs no round trip, so a cancelled context would not be noticed until the first CAS).

Deliberate design note: Release still takes no context

The issue suggests threading context.Context through the Storage interface. I built that first and then backed it out, because honouring cancellation in Release introduces a resource leak:

Pool.Close receives closeCtx, which is already cancelled under ForceStop. With a cancellable Release, every slot in both drain loops would fail instantly on an expired deadline and skip the Consul delete — leaking up to NewSlotsPoolSize + ReusedSlotsPoolSize = 132 node-scoped KV keys per force-stop. Nothing reclaims them: ReclaimLeakedSlots only scans /var/run/netns and never touches Consul, so on a stable nodeID those reservations persist across restarts and slowly eat the slot space. The createNetworkSlot rollback would skip its compensating delete for the same reason.

Threading the context but not honouring it is pointless — context.WithoutCancel strips deadlines too, leaving only trace values. So Release is unconditional but bounded, which is also consistent with the teardown paths in pool.go that already WithoutCancel on purpose. Happy to flip it if you'd rather have fast-abort-on-force-stop than key retention; it's a small change.

Timeouts are kept modest (5s per request, 10s per release) because Close drains slots serially, so the release bound is the per-slot cost of an unreachable agent during shutdown. Today that cost is unbounded, so this is a strict improvement either way.

How verified

New storage_kv_test.go drives a real consulApi.Client against an httptest "wedged agent" that accepts requests and never answers — what a hung Consul looks like on the wire. Three tests, all pure unit, run in a linux container (golang:1.26):

--- PASS: TestNewConsulConfig_SetsHTTPRequestTimeout (0.00s)
--- PASS: TestStorageKV_AcquireHonoursContextCancellation (0.05s)
--- PASS: TestStorageKV_ReleaseIsBounded (0.12s)

Both are mutation-proven, not just green. Reverting the release deadline to a plain context.Background():

--- FAIL: TestStorageKV_ReleaseIsBounded (5.02s)
    Release stayed blocked on the wedged agent instead of hitting its own deadline

Removing httpClient.Timeout (back to DefaultConfig() behaviour):

--- FAIL: TestNewConsulConfig_SetsHTTPRequestTimeout
    "0s" is not positive: Consul HTTP client must have an overall request timeout
    expected: 5s   actual: 0s

gofmt clean; GOOS=linux go vet ./pkg/sandbox/network/... clean; golangci-lint v2.11.4 (repo-pinned) with GOOS=linux GOARCH=amd64 reports 0 issues.

Stated plainly — what I did not run locally: the full ./pkg/sandbox/network/... suite. Two tests there (TestDenyEgress_InstallsAllProtocolDrop, TestCreateNetwork_TagsEgressWithDSCP) need CAP_SYS_ADMIN + iptables/nftables, and my local Docker VM ran out of capacity partway through. The diff is confined to storage_kv.go, which no other test touches, and the Storage interface is unchanged so pool.go/pool_test.go are untouched — CI runs the orchestrator shard with sudo and covers the rest. Nothing was verified against a real Consul agent; all Consul-facing tests use the hermetic fake.

Follow-ups, not done here

• Pool.Close's bare p.returnsWG.Wait() has no context selection, so Close is now bounded but still not interruptible.
• Both drain loops are serial, so worst-case shutdown scales linearly with pooled slots.
• NewClient replaces config.HttpClient for unix:// addresses, which would silently drop the timeout. No CONSUL_HTTP_ADDR is configured anywhere in this repo, so the default HTTP scheme applies — noted in a comment.

Linear: https://linear.app/e2b/issue/EN-981/networkstoragekvrelease-makes-unbounded-consul-calls-without-context

[cursor]

PR Summary

Medium Risk
Touches sandbox network slot allocation and shutdown paths; bounded timeouts can cause acquire/release failures under Consul outages, though that replaces indefinite blocking.

Overview
Prevents unbounded hangs when the Consul agent accepts connections but stops responding—a gap left by the default Consul HTTP client having no overall timeout.

Consul client setup now applies a 5s per-request HTTP timeout and 10s end-to-end cap on slot Release (Get + DeleteCAS), which matters because pool shutdown waits on releases serially. Acquire propagates the caller’s context into every KV operation and bails out of the large fallback slot scan on cancellation instead of ignoring context. Release still does not take a context so teardown is not aborted in a way that would skip Consul deletes and leak reservations; it is only time-bounded. New unit tests use a wedged fake agent to assert timeout behavior and context cancellation on Acquire.

^{Reviewed by Cursor Bugbot for commit 1031302. Bugbot is set up for automated code reviews on this repo. Configure here.}

[codecov]

Codecov Report

❌ Patch coverage is 40.54054% with 22 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ges/orchestrator/pkg/sandbox/network/storage_kv.go	40.54%	21 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!