feat(ec2): RegisterImage with explicit BDM snapshot-id for shared snapshots (#328)#329
Merged
Merged
Conversation
…pshots (#328) Follow-up to #322. The block-device-mapping.snapshot-id filter already returned ALL matching AMIs, but two CreateImage calls never shared a snapshot (each gets a fresh snap-id, as in real AWS), so a shared snapshot couldn't exist to test the 'retain shared snapshot' path. Adds RegisterImage honoring BlockDeviceMapping.N.Ebs.SnapshotId so a caller can register an AMI pointing at an existing snapshot — the AWS-faithful way to share a snapshot across AMIs. DescribeImages then returns every AMI referencing it. Regression test TestEC2_RegisterImage_SharedSnapshot: CreateImage A -> snap-X, RegisterImage B on snap-X, filter by snap-X returns both A and B. make test (race) + lint clean. Closes #328
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Merged
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.
Follow-up to #322. Enables the "retain a snapshot shared by multiple AMIs" path in spore.host/spawn's
DeleteAMIto be tested end-to-end.Diagnosis
The issue reported the
block-device-mapping.snapshot-idfilter returning only one AMI for a shared snapshot. Reproduced empirically — and the filter was already correct (it returns all matching AMIs, no early break). The real gap: twoCreateImagecalls from the same instance produce different snapshots (each gets a freshsnap-id, matching real AWS), so a shared snapshot never existed to be found.Fix
Add
RegisterImagehonoringBlockDeviceMapping.N.Ebs.SnapshotId, so a caller can register an AMI pointing at an existing snapshot — the AWS-faithful way to share a snapshot across AMIs (vs.CreateImage, which always materializes a new one).DescribeImagesfiltered by that snapshot then returns every referencing AMI.Verification
TestEC2_RegisterImage_SharedSnapshot: CreateImage A → snap-X; RegisterImage B on snap-X; filter by snap-X returns both A and B.make test(race) +make lintclean. Param shapes (BlockDeviceMapping.N.Ebs.SnapshotId,RegisterImageResponse) verified against the EC2 API reference.Closes #328