Implement Runtime NVMe Instance Storage Discovery Using AWS EBS Symlinks#396
Implement Runtime NVMe Instance Storage Discovery Using AWS EBS Symlinks#396
Conversation
cf-foundation-community-automation
bot
moved this to Inbox
in Foundational Infrastructure Working Group
rkoster
left a comment
rkoster
left a comment
There was a problem hiding this comment.
In general I would have expected this logic to go into the https://github.com/cloudfoundry/bosh-agent/tree/main/infrastructure/devicepathresolver package.
platform/linux_platform.go
Outdated
| p.logger.Debug(logTag, "Found NVMe devices: %v", allNvmeDevices) | ||
|
|
||
| // Identify EBS volumes via symlinks | ||
| ebsSymlinks, err := p.fs.Glob("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*") |
There was a problem hiding this comment.
This should somehow be passed in via the agent config in the stemcell builder, because it is IaaS specific.
There was a problem hiding this comment.
Created a new component instanceStorageResolver for the linux platform. It has 3 variants:
autoDetectingInstanceStorageResolver- the linux platform uses this one, it detects which storage resolver to useawsNVMeInstanceStorageResolver- detects what are the instance storage devices; contains the new logicidentityInstanceStorageResolver- simple resolver for the traditional naming used before this change
github-project-automation
bot
moved this from Inbox
to Waiting for Changes | Open for Contribution
in Foundational Infrastructure Working Group
Thank you for the review! That's was a big oversight on my end, I'll look into it. |
|
No worries 🙂 |
|
We discussed this during the FI WG meeting and this have to relay on the stemcell agent settings and agent strategy for disc handling. |
neddp
changed the title
|
As discussed during the working group meeting, focus is now on validating: cloudfoundry/bosh-aws-cpi-release#196 (comment) |
|
As per: cloudfoundry/bosh-aws-cpi-release#196 (comment) this change is still needed. Please continue reviewing. |
rkoster
requested review from
a team and
ramonskie
and removed request for
a team
|
|
||
| type awsNVMeInstanceStorageResolver struct { | ||
| fs boshsys.FileSystem | ||
| devicePathResolver DevicePathResolver |
There was a problem hiding this comment.
The devicePathResolver field is stored in the struct but never used anywhere in the awsNVMeInstanceStorageResolver implementation. The DiscoverInstanceStorage method only uses fs for globbing and symlink resolution.
| nvmeDevicePattern string, | ||
| ) InstanceStorageResolver { | ||
| if ebsSymlinkPattern == "" { | ||
| ebsSymlinkPattern = "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" |
There was a problem hiding this comment.
I would still like this to be passed in as configuration, so that the strategy can be iaas agnostic. This pattern might some day be re-usable.
* Refactor instance storage discovery into configurable component Implement auto-detection for instance storage disk type * Fix windows tests * Fix windows tests (but for real this time)
neddp
force-pushed
the
fix-nvme-instance-storage-discovery
branch
from
e7d00b4 to
dcd857a
Compare
|
@neddp could you take a look at these failing unit tests? |
|
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. |
4 participants
Problem
On AWS Nitro-based instances with NVMe devices, the kernel's PCIe enumeration order is non-deterministic. This means:
/dev/nvme0n1could be the root EBS volume OR instance storage/dev/nvme1n1could be instance storage OR the root EBS volumeSolution
Implemented runtime discovery to reliably identify instance storage by excluding EBS volumes.
Discovery Algorithm
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
/dev/xvdb,/dev/sdb) use CPI paths directlyThis must be merged together with the CPI changes - cloudfoundry/bosh-aws-cpi-release#196
Pair @Ivaylogi98