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]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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. Wires a SymlinkDeviceResolver into NewProvider and passes it into NewLinuxPlatform to enable NVMe instance-storage discovery. Refactors SetupRawEphemeralDisks to discover instance-storage device paths (NVMe glob + symlink exclusion or identity resolution), sorts and validates discovered devices, and partitions discovered devices rather than per-CPI-disk resolved paths. Also updates fake resolver recording and platform tests to inject and use the new resolver.

Suggested reviewers

• aramprice

🚥 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 change: implementing runtime NVMe instance storage discovery using AWS EBS symlinks, which is the primary objective of this PR.
Description check	✅ Passed	The description clearly explains the problem (non-deterministic PCIe enumeration order), the solution (runtime discovery), the algorithm steps, and backwards compatibility considerations. It is directly related to the changeset.
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 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: 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

[Copilot]

Pull request overview

Adds runtime NVMe instance-storage discovery on Nitro-based clouds by enumerating NVMe block devices and excluding IaaS-managed volumes via /dev/disk/by-id symlinks, then partitions only the discovered instance-storage devices (with updated tests and fakes).

Changes:

• Injects a new SymlinkDeviceResolver into LinuxPlatform to support symlink-based device discovery.
• Updates SetupRawEphemeralDisks to discover and partition instance-storage NVMe devices by filtering out managed volumes, with strict count validation.
• Adds/updates Ginkgo tests (including new resolver tests) and adjusts the fake device path resolver to track multiple calls.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
platform/provider.go	Constructs and passes a symlink-based resolver into Linux platforms.
platform/linux_platform.go	Implements NVMe instance-storage discovery and uses it during raw ephemeral disk setup.
platform/linux_platform_test.go	Adds coverage for NVMe discovery scenarios and keeps existing raw ephemeral disk behavior covered.
infrastructure/devicepathresolver/symlink_device_resolver.go	New resolver: glob devices, resolve managed-volume symlinks, filter devices, and udev trigger/settle.
infrastructure/devicepathresolver/symlink_device_resolver_test.go	Unit tests for symlink resolution, globbing, and filtering behavior.
infrastructure/devicepathresolver/fakes/fake_device_path_resolver.go	Changes fake to record all GetRealDevicePath calls (slice), enabling multi-call assertions.
infrastructure/devicepathresolver/virtio_device_path_resolver_test.go	Updates assertions to match fake’s new call-recording behavior.
infrastructure/devicepathresolver/scsi_device_path_resolver_test.go	Updates assertions to match fake’s new call-recording behavior.
infrastructure/devicepathresolver/fallback_device_path_resolver_test.go	Updates assertions to match fake’s new call-recording behavior.
agent/bootstrap_test.go	Updates platform construction in tests to provide the new symlink resolver dependency.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rkoster]

@neddp could you look at these copilot suggestions?

[aramprice]

Can you take a look at the copilot suggestions.

[neddp]

Both suggestions were addressed.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

platform/linux_platform.go (1)

859-875: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fail closed when managed-volume symlink resolution is incomplete.

managedDevices is treated as authoritative here, but the resolver upstream currently skips unreadable symlinks. If one EBS symlink is missed and the filtered count still matches len(devices), the code below can mklabel a managed volume or even the root disk. Please make unresolved managed-volume symlinks abort discovery instead of continuing with a partial exclusion set.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@platform/linux_platform.go` around lines 859 - 875, ResolveSymlinksToDevices
currently may skip unreadable symlinks, letting managedDevices be incomplete;
change the resolver to surface skipped/unresolved symlinks (e.g., change
ResolveSymlinksToDevices to return (devices []string, skipped int, err error) or
return an error when any symlink cannot be read) and update this call site in
linux_platform.go to treat any skipped/unresolved count or non-nil error as
fatal: after calling p.symlinkDeviceResolver.ResolveSymlinksToDevices, if
skipped>0 (or err != nil) return an error instead of continuing, so
managedDevices cannot be partial before calling FilterDevices/instanceStorage
and proceeding with mklabel operations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@platform/linux_platform.go`:
- Around line 883-888: The loop calling devicePathResolver.GetRealDevicePath
ignores the boolean timedOut return; update the loop in linux_platform.go (where
devicePathResolver.GetRealDevicePath is invoked) to check the timedOut flag and
treat it as an explicit failure: if timedOut or realPath == "" return a wrapped
error (similar to other call sites) instead of continuing, so the function
returns a clear timeout error rather than allowing an empty path to be passed to
parted; use the same bosherr.WrapErrorf pattern and include context mentioning
the device and that resolution timed out.

---

Outside diff comments:
In `@platform/linux_platform.go`:
- Around line 859-875: ResolveSymlinksToDevices currently may skip unreadable
symlinks, letting managedDevices be incomplete; change the resolver to surface
skipped/unresolved symlinks (e.g., change ResolveSymlinksToDevices to return
(devices []string, skipped int, err error) or return an error when any symlink
cannot be read) and update this call site in linux_platform.go to treat any
skipped/unresolved count or non-nil error as fatal: after calling
p.symlinkDeviceResolver.ResolveSymlinksToDevices, if skipped>0 (or err != nil)
return an error instead of continuing, so managedDevices cannot be partial
before calling FilterDevices/instanceStorage and proceeding with mklabel
operations.

🪄 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: ab6aff62-71d1-4d01-9a14-e284e52205e7

📥 Commits

Reviewing files that changed from the base of the PR and between c43c6a1 and 3b63c11.

📒 Files selected for processing (1)

• platform/linux_platform.go

[aramprice]

/coderabbitai review