perf(create): singleflight same-ref snapshot and cloud-image imports

[CMGS]

Refs #35.

Problem

Concurrent pods using the same cold OCI artifact independently download and import the same bytes — ensureSnapshot and ensureRunImage have no synchronization between the local check and the pull/import. The user-visible failure is worse than wasted bandwidth: vk's SnapshotImport removes the target name before importing (retry idempotency), while cocoon binds the name only when an import completes. In an N-way cold burst, N-1 pods fail snapshot name already in use, and a late rm can unbind the name a finished import just registered, failing a pending clone with "not found".

Note the issue text's corruption framing does not hold against current cocoon: imports extract into a unique data dir and insertRecord rejects name collisions, and live VMs keep their hardlinked memory files across a snapshot rm. The real harms are the availability failure above plus N× registry bytes.

Change

• Two provider-owned singleflight.Groups beside the existing forkSnapshotSF, same idiom.
• ensureSnapshot: local-hit fast path stays flight-free; the flight wraps check+pull+inspect keyed on the local name. The re-check inside the flight is load-bearing — it is what closes the rm-vs-registered-import window.
• ensureRunImage: the whole resolution (manifest fetch, classify, all four destinations including the cocoon image pull fallbacks) runs in a flight keyed on the raw ref, with force folded into the key: concurrent force requests share one import, a force caller never adopts a non-force flight's local hit, and a later force re-imports because keys evict on flight return.
• Shared work runs on a provider-scoped context (WithTimeout(p.lifecycleCtx, importDetachTimeout)): one caller's aborted CreatePod can't fail the rest, Close aborts it (so a stalled cocoon child can't outlive the provider), and the 30m deadline backstops a registry stream that hangs without either firing.
• Callers join via DoChan + select on their own ctx, so a cancelled CreatePod returns immediately instead of occupying its worker until the flight ends, while the flight keeps running for the remaining waiters.
• resolveWakeSource is deliberately not wrapped: it always re-pulls so a crashed prior wake's stale -hibernate-import is never reused, and virtual-kubelet serializes per pod, so there is no fan-out to dedup.
• Zero metrics changes: SnapshotPullDuration now observes only real pulls (leader-only) with no line moved; SnapshotPullTotal keeps its ensure-outcome-per-pod meaning.

Tests

Test fakes now mirror cocoon's contract — import names become visible only when wait() completes — which makes the concurrency assertions deterministic: 16-way cold burst → exactly 1 import + 1 manifest fetch (snapshot and image paths); warm hit → 0 transfers/imports; different refs provably overlap in flight; a cancelled leader doesn't fail the shared import (fake import dies on a cancelled ctx, so a plain ctx pass-through fails this test); shared failure reaches all waiters and a later call retries; force semantics (concurrent-share, warm-skip-then-force-reimport, later-force). The two tests that need "second caller parked in the flight" use testing/synctest. Fixtures are minimal real streamable artifacts (digest-verified blobs served from memory).

Evidence

• go test -race -run 'TestEnsureSnapshot|TestEnsureRunImage' -count=20 ./provider/cocoon/ ok.
• make lint 0 issues on linux+darwin; go test -race ./... all 8 packages ok; make build ok.
• Pending (tracked in perf: singleflight same-ref OCI snapshot and cloud-image imports #35): testbed cold-burst runs recording registry bytes, import process count (execsnoop), CreatePod p95, and name already in use failures ~15 → 0.

Follow-up candidate (pre-existing, out of scope here): ensureForkSnapshot's detached flight uses a bare context.WithoutCancel with no timeout backstop, so it is now the only one of the three provider flights without a deadline. A fix should not just reuse importDetachTimeout — SnapshotSave scales ~2s/GiB and is a different operation family than a registry-stream import, so it needs its own bound. Tracked for a separate change.

[Copilot]

Pull request overview

This PR addresses a concurrency/performance issue in the Cocoon provider where multiple pods requesting the same cold OCI snapshot or cloud image could redundantly pull/import the same artifact and race on local name registration. It introduces provider-owned singleflight.Groups to deduplicate snapshot pulls by local snapshot name and run-image materialization by ref (including force semantics), using cancel-detached bounded contexts to prevent one caller’s cancellation from aborting shared work.

Changes:

• Add snapshotPullSF and runImageSF singleflight groups to deduplicate snapshot pulls and run-image resolution/imports.
• Wrap snapshot pull+inspect and run-image resolution paths in singleflight with a cancel-detached, time-bounded import context (importDetachTimeout).
• Add/adjust concurrency-focused tests with in-memory OCI artifacts and deterministic synchronization.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
provider/cocoon/provider.go	Adds new singleflight.Group fields for snapshot pulls and run-image materialization deduplication.
provider/cocoon/create.go	Introduces detached import context timeout and wraps snapshot/image resolution in singleflight to avoid duplicate imports and name races.
provider/cocoon/create_test.go	Adds singleflight concurrency tests and supporting fake runtime/registry behavior for deterministic assertions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@CMGS Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub.

You can ask me to try again later by mentioning me in a new comment.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: f082992f-1252-4612-b5db-14121c1554c9

Sorry for the inconvenience!

[CMGS]

Updated at f4696cf after a Codex review round (thread 019f69b0), which found the original detach design left two real holes:

1. singleflight.Do parks a duplicate caller in c.wg.Wait(), which takes no ctx (x/sync@v0.21.0/singleflight/singleflight.go:98), and the leader runs doCall inline — so a cancelled CreatePod could not leave the flight and stayed pinned for up to importDetachTimeout (30m).
2. context.WithoutCancel put the shared work outside Close's reach: it could neither cancel nor await it, so a stalled cocoon child could outlive the provider.

Fix: shared work now runs on WithTimeout(p.lifecycleCtx, importDetachTimeout) — provider-scoped, so Close aborts it and the exec child dies with it — and callers join via DoChan + select on their own ctx, so a cancelled caller returns immediately while the flight continues for everyone else. This also matches issue #35's wording more literally ("a bounded provider-lifecycle context") than the original did.

Two synctest-based regression tests cover it; both were verified to FAIL against the pre-fix shape (Do + WithoutCancel) before being kept, so they discriminate rather than pass vacuously.

Codex re-reviewed at f4696cf and reported no findings on either PR, with the round-1 finding closed and no code blockers remaining.