[NOT READY YET] Don't block on (or restore from) third-party snapshot copies - #227
Draft
huydhn wants to merge 1 commit into
Draft
[NOT READY YET] Don't block on (or restore from) third-party snapshot copies#227huydhn wants to merge 1 commit into
huydhn wants to merge 1 commit into
Conversation
…hots CopySnapshot carries the source snapshot's tags onto the copy, so a Wiz security-scanner copy of a user's disk is tagged gpu-dev-user + disk_name and looks like ours. Two consequences, both hit in prod: 1. Blocking. The pending-snapshot gate matched the scanner's copy, which can stay pending for hours (one sat at 4% for 10h). Every reserve for that disk queued behind it. On 2026-08-17 this cost drisspg 3 failed reservations; a scan batch on 08-18 armed the same trap for 8 users. 2. Silent stale restores. A *completed* copy also matched, and since its StartTime is the copy time it can win max(StartTime) and be chosen as the restore source -- handing back a point-in-time older than it looks. Simulated against prod: 7 of the 8 affected users would have restored from a scanner copy. Ignore snapshots we did not author in both the gate and the restore-source selection, and only wait on a pending snapshot that is newer than the newest completed one -- an older pending snapshot has nothing to offer. Matched on copy markers rather than requiring a gpu-dev tag: 133 of 6612 real snapshots carry no SnapshotType tag, including 5 hand-made recovery snapshots that a whitelist would have orphaned. Validated over all 6612: 6596/6596 real snapshots kept, 16/16 scanner copies excluded. Test: 1220 passed, 3 xfailed.
huydhn
marked this pull request as draft
August 18, 2026 19:09
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.
Follow-up to #226, kept separate as discussed. #226 stopped the Lambda blocking on a pending snapshot; this fixes which snapshots we consider ours in the first place.
Why
CopySnapshotcopies the source snapshot's tags onto the copy. The Wiz security scanner copies gpu-dev disks, so its copies arrive taggedgpu-dev-user+disk_name+ManagedBy=gpu-dev-cliand our disk logic treats them as ours. Two failures, both observed in prod:1. Blocking. The pending-snapshot gate matched the scanner's copy, which can sit pending for hours — one lived 10h (
snap-01da8673…, CloudTrail:CopySnapshotbyWizScannerRole, descriptionauto-generated by Wiz). On 2026-08-17 that produced 3 failed reservations for drisspg. A scan batch at 17:38 UTC on 08-18 armed the same trap for 8 users simultaneously.2. Silent stale restores — the more dangerous one. A completed copy also matched, and its
StartTimeis the copy time, so it can winmax(StartTime)and be picked as the restore source — returning a point-in-time older than it appears. Simulated against prod data, 7 of the 8 affected users would have restored from a scanner copy on their next reservation.What
is_foreign_snapshot()— ignore snapshots we didn't author, in both the pending gate and the restore-source selection.Matched on copy markers (
wiz*tags,VolumeId == vol-ffffffff, description) rather than requiring a gpu-dev tag. A whitelist looked cleaner but 133 of 6612 real snapshots carry noSnapshotTypetag, including 5 hand-made recovery snapshots (mreso, nshulga, ezyang) it would have orphaned.Validation
Against all 6612 production snapshots: 6596/6596 real kept, 16/16 scanner copies excluded, zero false exclusions.
Replaying the new logic over the 8 currently-affected users — all go blocked → ok, and 7 get corrected restore sources:
Test plan
Written TDD — 5 new tests in
tests/unit/lambda_fn/test_foreign_snapshots.py, all confirmed failing first (incl. one reproducing the stale-restore path asassert 'snap-wiz' == 'snap-ours').tofu validatenot run — my local provider cache is empty; the tf change is a one-characterLAMBDA_VERSIONbump to 0.6.7.Notes
tofu apply(prod) to take effect. Until then the 8 users above stay blocked;--no-persistis the interim workaround.queuedstate from Retry pending disk snapshots without blocking Lambda #226 (a stuck snapshot still queues forever with no deadline) — separate PR.shared/snapshot_utils.py::get_latest_snapshothas the same hole, but its only callerget_latest_user_snapshot→migrate_ebs_across_azis dead code, so I left it. Worth deleting or fixing if it's ever revived.gpu-dev-user/disk_name, not just this gate.