fix: fence pod incarnation writes - #89
Merged
Merged
Conversation
CMGS
force-pushed
the
codex/fix-pod-incarnation-writes
branch
from
September 3, 2026 08:30
f89f20d to
97e6038
Compare
…indRuntime detachIncarnation duplicated untrackIncarnation's table teardown; it now calls it and re-indexes the VM by name, which is the only property it needs beyond untrack. The name re-index replaces the insert-if-absent in trackPodIncarnation with overwrite semantics, shared with indexOrphanByName via indexOrphanByNameLocked: the freshly booted VM is the live one. trackPodIncarnation + applyVMRuntime + detach-on-false was spelled twice; bindRuntime owns it and takes the runtime record, so applyRuntime is one line over it and registerMacosVM passes its VNC-carrying record. The macOS record resolves its lease through seedLeaseIP before tracking, the same pre-track seed CreatePod already ran inline. The tracked-pod UID mismatch was spelled five times; supersededLocked owns it. reconcileMacosPod discarded registerMacosVM's fence result and published readiness for a pod key a newer incarnation already owned.
A create whose incarnation fence loses the key returned nil with no log
line and no metric, so a pod that never got its VM looked like a clean
create. skipSuperseded logs the pod key and UID and counts the exit as
pod_lifecycle_total{result=skipped,reason=superseded}.
Covers both CreatePod exits, createMacosPod, wake, adoptByVMName and the
macOS startup adoption.
wake returned nil on a lost incarnation fence, so UpdatePod still counted update ok and ran refreshAndNotify with the old incarnation. The framework then held that pod as the provider's last status and retried UpdateStatus against the successor until another notify replaced it. wake now reports the fence and UpdatePod returns before the counter and the notify.
Detach took p.mu twice, so a successor entering between the untrack and the name re-index could miss the VM during adoption and then have its own binding overwritten by the predecessor's record. Later adoption by name or fork snapshotting would then pick the stale VM. Both halves now run under one lock, and the re-index yields to a successor that already bound that VM name.
A probe that ignores its context outlives Forget: the macOS crash recovery detaches from the probe deadline for up to five minutes. When the pod was recreated under the same key meanwhile, that probe still wrote the successor's result and ran its readiness callback, and its recovery rewrote the successor's VM record, VNC reservation and endpoint annotations, because the macOS VM id is derived from the VM name and is identical across incarnations. The probe loop now records a result only while it still owns the key, and the macOS probe carries the pod UID it started for and checks it before every recovery write.
patchSuperseded also matched NotFound, so a pod deleted during bring-up made bindRuntime detach its VM; DeletePod then had nothing to remove and the VM ran until the next restart. Only Invalid and Conflict mean another incarnation owns the name; NotFound keeps the binding. failCreate reuses supersededLocked.
virtual-kubelet compares pods by key, so a pod recreated under its old name while the predecessor is still tracked arrives as UpdatePod, not CreatePod. UpdatePod tracked the successor unconditionally: it inherited the predecessor's VM table entry without a VMID/IP annotation of its own, or, when the predecessor had been detached, fell into the wake branch. UpdatePod now detaches the old incarnation (the VM stays reachable by name) and hands the successor to CreatePod, which adopts the VM by name and publishes the runtime under the successor's UID.
… creates 20347c2 routed every untracked UpdatePod to CreatePod, which turned a wake after a restart between unhibernate and wake (startup reconcile tracks only pods still marked hibernated) into a fresh VM and lost the hibernated state; TestWakeClearsStalePostCloneMarker caught it. UpdatePod now probes for a wake source first — a local snapshot, or a registry hibernate tag when a puller is configured — and only an untracked pod with nothing to wake from goes to CreatePod; the deleting fence applies to the untracked track as well.
…M-gone eviction The deleting-map read was spelled six times and the in-flight message three times (a contract the test string-matches). evictGoneIncarnation now logs the same skip line as removeThenEvict instead of returning silently, and evictPod's godoc states the Conflict branch it gained in 20347c2.
DeletePod returns the in-flight error and keeps the other holder's fence; a VM-gone eviction issues no apiserver delete once a successor owns the key.
DeletePod resolved the VM by pod key, so a delete retried after a same-name recreate had been tracked removed the successor's VM and dropped its tracking; the new pod then sat Running/Ready=False with a dead VM id. Reproduced on kind: an out-of-band VM removal made the first delete fail, and its retry landed after the recreate.
A superseded delete still took the deleting fence for an instant, and a VM-gone eviction whose claim met that fence was dropped for good, leaving the successor pod with a dead VM id. The claim now answers superseded, in-flight or claimed under one lock, so a superseded delete never touches the fence and a recreate cannot slip in between the check and the claim.
The delete claim enum sits at the head of the const block so iota starts at zero, and the deleteClaim type precedes the Provider godoc.
A bind rejected during an in-flight delete indexes its VM by name so the successor can adopt it, but the old incarnation's untrack dropped that name entry unconditionally and the VM leaked until the next restart.
UpdatePod read the VM and the tracked UID under two locks; a single snapshot removes the torn read. The probe start reused setOwned's guard inline; trackedPodUID had no caller left.
A same-name recreate adopted whatever the name index held. When the old VM's removal had failed once, its record stayed indexed after the runtime dropped it and its VM-gone event had already been skipped under the delete fence, so the successor bound a dead VM for good. Adoption now asks the runtime first; a VM it no longer knows leaves the index and the pod is created afresh.
Any inspect error other than not-found counted as the VM still being there, so a CLI hiccup while a dead VM sat in the name index bound it as adopted with no later path to notice. Adoption now inspects with the inline retry, drops a VM the runtime no longer knows, and returns the residual error so the controller retries the create.
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
1 participant
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
Verification
gofmt, andgit diff --check