Repository navigation
fix(buildkit): heal + retry builds hitting a missing cache-record lease; grace-window the dead-record sweep - #194
Merged
Merged
Conversation
…ase; grace-window the dead-record sweep (#193) The #193 failure — failed to solve: lease "4igb...": not found — is the lease half of the two-databases-one-store corruption the dangling-snapshot healer already covers: a BuildKit cache record survives in the bbolt metadata while the containerd lease it owns (same 25-char base36 ID) is gone. Nothing on the record-load path checks the lease, so the stale record keeps being served from cache and detonates only when a build cache-hits it and attaches a resource to the missing lease (a cached FROM in buildkit's source/containerimage/pull.go, or setBlob on export), failing the job with the raw containerd error. The desync creators are the out-of-band classes heal.go already documents (manual ctr lease removal, a restored data dir, a crash between a lease deletion and the matching metadata clear); no ephemerd sweep deletes leases in the shared namespace — the sweeps only remove image records, which cannot take a lease with them. Fix, same principle as the snapshot healer: recognise the signature (narrowly — only a quoted 25-char record-lease ID, so BuildKit's temporary "<nanos>-<b64>" leases never trigger a cache wipe), run the existing prune -> rebuild escalation ladder, and retry the build once. The cheap rung genuinely repairs it: the failed solve released its refs, PruneAll collects the stale record (its remove tolerates the already-missing lease) and the retry re-pulls a clean copy — which is why the incident's plain rerun passed. Also close a TOCTOU in the dead-build-record sweep while in there: the live set is a container listing captured before the record listing, so a job that starts and exports a build between the two would have its records judged dead and deleted mid-job. Records written within a 15-minute grace are now exempt (an unknown timestamp fails safe), same never-collect-a-live-owner rule as runtime.ReapDeadContainers.
…a canceled ctx Review findings on the parent commit: - Both dangling-signature regexes matched anywhere in the error string, and job-influenced text reaches that string: a hostile registry's HTTP reason phrase (preserved verbatim by containerd's remote error) or a RUN command's own echoed output could embed a fake match and trigger a shared-cache PruneAll — repair throws away every job's warm cache, and a repeated fake key escalates to a Rebuild that stops the shared gRPC core. Genuine containerd not-founds are always the TAIL of the wrap chain while job-influenced text always gets further wrapping appended, so anchoring both matchers to end-of-string closes the vector without losing a single real signature (all existing genuine-string tests pass unchanged). Matters doubly with the fork-PR flip on the roadmap. - A canceled ctx (runner disconnect mid-build) made PruneAll fail with context.Canceled, which the ladder read as store inconsistency and escalated to Rebuild under the same dead ctx — the worst path, since a failed Rebuild leaves the shared core stopped. applyHealAction now refuses to repair or escalate under a canceled ctx; a genuine desync re-fires the ladder from the next live build. - Pinned the near-miss shapes review flagged as untested: history-lease ref_ prefix, 26/24-char ids, temporary-lease shape, id-less variant, and both wrapped job-influenced forms.
Contributor
Author
|
Adversarial review completed (diagnosis re-verified against vendored containerd v2.2.2 + buildkit v0.25.1 sources — the record-lease ID space, the exact error format, the warm-cache detonation sites, and the prune rung's tolerance of a missing lease all CONFIRMED; the grace predicate's timestamp threading verified populated at both callers, so the fail-safe cannot become fail-dead). Verdict was safe-to-merge with two pre-existing-exposure MAJORs worth closing here rather than in a follow-up — both fixed in
Deferred to the 0.2.10 hardening PR (pre-existing, review-identified): |
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.
Fixes #193 (
failed to solve: lease "4igb5uptxddpdjw9lwatd9psk": not found, intermittent, rerun passes).Root cause — and the issue's hypothesis disproven
The failing ID is a 25-char base36
identity.NewID— the ID space of exactly one lease kind: a BuildKit cache record's own lease (leases.WithID(recordID)+gc.flat, no expiry). The raw unwrapped error comes from containerd'sAddResourceon a missing lease bucket, reached from the solve path's cached-FROMhandling (source/containerimage/pull.go) — matching a trivial busybox build failing instantly on a warm cache.No ephemerd sweep can cause this. Exhaustively enumerated: the only lease deletions in the repo are
CleanupJobNamespace(per-job dind namespaces only) and the cache mirror-lease release.PruneDeadBuildRecords,CleanupJobBuildRecords,imagegc.Collect,SweepBrokenImageChains, andCachePrunedelete image records — an image delete cannot remove a lease, and containerd's own GC only deletes expired leases (record leases never carrygc.expire). #187's reaper and #190 are likewise exonerated (runtime namespace only).What actually happened: a metadata↔lease desync — BuildKit's bbolt store kept a cache record whose containerd lease is gone — the same "two databases describe one store" corruption class as #149's dangling snapshot (
heal.goalready documents the creators: hard kill between the two stores' writes, ENOSPC during a prune's metadata clear — this fleet's signature failure — or out-of-band lease removal). The desync sits latent until a build cache-hits the record. The rerun passing is the tell: the failed solve's released refs let BuildKit's own GC collect the stale record (record removal tolerates the missing lease), healing the store between runs.Fix
-view/-variants) + containerd's exact phrasing, so BuildKit's temporary<nanos>-<base64>leases can never trigger it — feeds the dangling-lease signature into the existing fix(buildkit): repair the shared build store instead of failing every job (#149) #150 Healer ladder (prune → rebuild → give-up) with a single retry. The prune rung provably clears the broken state; the incident class becomes one healed solve instead of a red check.DeadBuildRecords): the live-job set is captured before the record listing, so a job starting between the two listings could have its fresh export judged dead. Records younger than a 15-minute grace — or with unknown timestamps, failing safe — are never selected; genuinely dead jobs' records still are, one tick later. Same principle asReapDeadContainers: never collect a resource whose owning operation could still be live.Diagnostics
Post-fix, the incident logs
build failed on a lease the shared store references but containerd does not have; repairing the shared store→shared build store repaired; retrying the build oncewith the lease ID in the heal report. On the original node, look for an earlier metadata rebuild, failed prune, or unclean restart adjacent to image-GC activity to date the desync's origin.Testing
heal_test.gopins the matcher (record IDs match; temporary-lease shapes, other not-founds, and near-misses don't) and the ladder integration;buildkit_cleanup_test.gopins the grace predicate (in-flight/young/unknown-age never selected; dead-past-grace still swept).go vet ./pkg/...clean; dind/buildkit/imagegc/cacheprune/runtime suites green.