Skip to content

fix(launch): auto-detect minimum root volume size from AMI snapshot (#25) - #45

Merged
scttfrdmn merged 2 commits into
mainfrom
fix/25-ami-volume-size
Jun 3, 2026
Merged

fix(launch): auto-detect minimum root volume size from AMI snapshot (#25)#45
scttfrdmn merged 2 commits into
mainfrom
fix/25-ami-volume-size

Conversation

@scttfrdmn

Copy link
Copy Markdown
Contributor

Closes #25.

Problem

Launching from a custom AMI whose root snapshot is larger than spawn's 20 GiB default fails:

InvalidBlockDeviceMapping: Volume of size 20GB is smaller than snapshot 'snap-…', expect size >= 80GB

Users had to pass --volume-size 80 with a number they had to discover out of band. (Also the root cause behind nf-spawn#21.)

Fix

Launch now queries the AMI's BlockDeviceMappings (new rootVolumeSizeFromAMI helper) and floors the root volume at the snapshot minimum — final size is max(chosen, amiMin). This rescues:

  • the default-20 case (custom AMI, no --volume-size), and
  • an explicit --volume-size that's smaller than the snapshot.

Best-effort: any DescribeImages error (AMI not found, no permission) returns 0 and leaves the chosen size unchanged — a launch never fails because the lookup didn't work. Root device is matched by RootDeviceName, falling back to the largest EBS mapping as a safe floor.

Test

buildBlockDevices gains an amiMinGiB floor param; unit tests cover floor-raises-default, floor-beats-too-small-explicit, and larger-explicit-is-kept. Verified Tier 0 launch path unaffected (substrate's synthetic AMIs have no registered BDM, so the lookup returns 0 — no regression).

Follow-ups

  • nf-spawn#21 can be closed once this ships — the ext.volumeSize workaround becomes unnecessary (spawn does the right thing automatically).
  • A full Tier 0 assertion of the floor would need substrate to register an AMI with BlockDeviceMappings and return them from DescribeImages; will file that as a substrate enhancement. The unit tests cover the floor logic directly in the meantime.

)

Launching from a custom AMI whose root snapshot is larger than spawn's
default (20 GiB) failed with InvalidBlockDeviceMapping ('Volume of size
20GB is smaller than snapshot, expect size >= 80GB'), forcing users to
pass --volume-size explicitly with a number they had to know out of band.

Launch now queries the AMI's BlockDeviceMappings (rootVolumeSizeFromAMI)
and floors the root volume at the snapshot minimum: the final size is
max(chosen, amiMin). This fixes both the default-20 case and an explicit
--volume-size that's too small. Best-effort: any DescribeImages error
returns 0 and leaves the chosen size unchanged, so a launch never fails
because the lookup didn't work.

buildBlockDevices gains an amiMinGiB floor parameter; unit tests cover
floor-raises-default, floor-beats-too-small-explicit, and
larger-explicit-is-kept.

Closes #25
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 51.28205% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/aws/client.go 51.28% 19 Missing ⚠️

📢 Thoughts on this report? Let us know!

Split the BDM root-device selection out of rootVolumeSizeFromAMI (which
needs a live EC2 client) into a pure rootVolumeSizeFromMappings, and cover
the tricky bits: root-device match by name, largest-EBS fallback when the
root name doesn't match, skipping non-EBS mappings, and the empty case.
@scttfrdmn
scttfrdmn merged commit 0789116 into main Jun 3, 2026
3 checks passed
@scttfrdmn
scttfrdmn deleted the fix/25-ami-volume-size branch June 3, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spawn launch: should auto-detect minimum volume size from AMI snapshot

1 participant