Implement Runtime NVMe Instance Storage Discovery Using AWS EBS Symlinks

[neddp]

Problem

On AWS Nitro-based instances with NVMe devices, the kernel's PCIe enumeration order is non-deterministic. This means:

• /dev/nvme0n1 could be the root EBS volume OR instance storage
• /dev/nvme1n1 could be instance storage OR the root EBS volume
• The order varies between boots and instance types
• There is no guaranteed ordering

Solution

Implemented runtime discovery to reliably identify instance storage by excluding EBS volumes.

Discovery Algorithm

1. Glob all NVMe devices: /dev/nvme*n1
2. Glob EBS symlinks: /dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*
3. Resolve each symlink to its target device
4. Subtract EBS devices from all NVMe devices = instance storage
5. Validate count matches CPI expectations
6. Partition only the discovered instance storage devices

Why EBS Symlinks Are Reliable

AWS automatically creates persistent symlinks for all EBS volumes via udev rules:

/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol{volume_id}

Backwards Compatibility

Non-NVMe instances: No changes to behavior

• Traditional Xen instances (/dev/xvdb, /dev/sdb) use CPI paths directly
• Paravirtual instances work as before

This must be merged together with the CPI changes - cloudfoundry/bosh-aws-cpi-release#196

---

Pair @Ivaylogi98

[rkoster]

In general I would have expected this logic to go into the https://github.com/cloudfoundry/bosh-agent/tree/main/infrastructure/devicepathresolver package.

[neddp]

In general I would have expected this logic to go into the https://github.com/cloudfoundry/bosh-agent/tree/main/infrastructure/devicepathresolver package.

Thank you for the review! That's was a big oversight on my end, I'll look into it.

[rkoster]

No worries 🙂

[beyhan]

We discussed this during the FI WG meeting and this have to relay on the stemcell agent settings and agent strategy for disc handling.

[rkoster]

As discussed during the working group meeting, focus is now on validating: cloudfoundry/bosh-aws-cpi-release#196 (comment)

[rkoster]

As per: cloudfoundry/bosh-aws-cpi-release#196 (comment) this change is still needed. Please continue reviewing.

[rkoster]

Still think this PR could be done in an IaaS agnostic way.

[rkoster]

@neddp could you take a look at these failing unit tests?

[neddp]

Hi @rkoster,

We still haven't had the time to test the changes on an actual deployment. I will move the PR to draft until we can confirm everything is working fine.

We'll address the tests as well.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c29fbe6f-67f0-467e-ae03-ea81376b9d3a

📥 Commits

Reviewing files that changed from the base of the PR and between 9491b7e and 3c4db0c.

📒 Files selected for processing (2)

• infrastructure/devicepathresolver/symlink_device_resolver.go
• infrastructure/devicepathresolver/symlink_device_resolver_test.go

---

Walkthrough

Adds a SymlinkDeviceResolver (NVMe constants, constructor) with ResolveSymlinksToDevices, GetDevicesByPattern, and FilterDevices. Implements udev trigger/settle, globbing, read-and-follow link resolution, and exclusion filtering. Adds Ginkgo tests for resolving, glob errors, and filtering. Updates fake device-path resolver to record all diskSettings calls and adjusts platform tests to inject and use the new symlink resolver for NVMe instance-storage discovery.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main implementation: runtime NVMe instance storage discovery using AWS EBS symlinks, which matches the core changes in symlink_device_resolver.go, linux_platform.go, and related modifications.
Description check	✅ Passed	The description is well-related to the changeset, explaining the problem (non-deterministic PCIe enumeration), the discovery algorithm, why EBS symlinks are reliable, and backwards compatibility—all directly reflected in the code changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-nvme-instance-storage-discovery

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[neddp]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infrastructure/devicepathresolver/symlink_device_resolver_test.go`:
- Around line 72-92: The test currently expects ResolveSymlinksToDevices to
silently skip unresolved symlinks; instead modify the resolver
(ResolveSymlinksToDevices in the symlink device resolver) to treat any existing
symlink that cannot be resolved as an error (fail-closed) and return that error
to the caller; update the symlink_device_resolver_test.go case that created an
unresolved "/dev/disk/by-id/..." entry so it now asserts that
ResolveSymlinksToDevices returns a non-nil error (and no successful mapping)
rather than skipping the broken symlink. Ensure the change surfaces the
underlying readlink/resolve error from the resolver.

In `@platform/linux_platform.go`:
- Around line 811-825: The current logic gates NVMe runtime discovery on
CPI-reported path prefixes via hasNVMeDevices(devices), which can skip NVMe
resolution when CPI reports aliases; change the condition so that if a managed
volume pattern (p.options.InstanceStorageManagedVolumePattern != "") and the
symlink resolver (p.symlinkDeviceResolver != nil) are present, always call
p.discoverNVMeInstanceStorage(devices) instead of falling back to
p.discoverIdentityInstanceStorage(devices), removing the hasNVMeDevices(devices)
check; keep references to discoverNVMeInstanceStorage,
discoverIdentityInstanceStorage, hasNVMeDevices,
p.options.InstanceStorageManagedVolumePattern, and p.symlinkDeviceResolver to
locate and update the conditional.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ed13b328-8e26-46ab-81e0-b8bed7f74223

📥 Commits

Reviewing files that changed from the base of the PR and between a47020a and bb98869.

📒 Files selected for processing (10)

• agent/bootstrap_test.go
• infrastructure/devicepathresolver/fakes/fake_device_path_resolver.go
• infrastructure/devicepathresolver/fallback_device_path_resolver_test.go
• infrastructure/devicepathresolver/scsi_device_path_resolver_test.go
• infrastructure/devicepathresolver/symlink_device_resolver.go
• infrastructure/devicepathresolver/symlink_device_resolver_test.go
• infrastructure/devicepathresolver/virtio_device_path_resolver_test.go
• platform/linux_platform.go
• platform/linux_platform_test.go
• platform/provider.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infrastructure/devicepathresolver/symlink_device_resolver_test.go`:
- Around line 88-90: The test file symlink_device_resolver_test.go has a
formatting/indentation issue around the assertion block that calls
resolver.ResolveSymlinksToDevices and checks the error; run goimports (or gofmt)
on the file to normalize imports and indentation so the block with
ResolveSymlinksToDevices, Expect(err).To(HaveOccurred()), and
Expect(err.Error()).To(ContainSubstring("nvme-invalid")) is properly formatted.
Ensure the file compiles and lints cleanly after running the formatter.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d923cdb-2fa3-4c19-bc3c-66c23b259996

📥 Commits

Reviewing files that changed from the base of the PR and between bb98869 and 9491b7e.

📒 Files selected for processing (4)

• infrastructure/devicepathresolver/symlink_device_resolver.go
• infrastructure/devicepathresolver/symlink_device_resolver_test.go
• platform/linux_platform.go
• platform/linux_platform_test.go