-
Notifications
You must be signed in to change notification settings - Fork 124
Implement Runtime NVMe Instance Storage Discovery Using AWS EBS Symlinks #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
neddp
wants to merge
15
commits into
main
Choose a base branch
from
fix-nvme-instance-storage-discovery
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
d5a06d6
Implement runtime NVMe instance storage discovery using EBS symlinks
neddp 3d6949d
Fix nvme instance storage discovery (#400)
Ivaylogi98 dcd857a
Remove leftover path normalization (#401)
Ivaylogi98 bb98869
Fix nvme instance storage discovery (#407)
Ivaylogi98 826c3ed
Merge branch 'main' into fix-nvme-instance-storage-discovery
neddp 08b053d
Fix missing closing bracket
neddp a797780
Update infrastructure/devicepathresolver/symlink_device_resolver.go
neddp 9491b7e
Update infrastructure/devicepathresolver/symlink_device_resolver_test.go
neddp 834ec2a
Fix lint identation
neddp 3c4db0c
Fix: skip unresolvable symlinks instead of returning error
neddp c43c6a1
Use the already constructed udev instance
neddp 3b63c11
fix: use %+v instead of %s for DiskSettings in error message
neddp f63ffb3
fix: handle timedOut in discoverIdentityInstanceStorage
neddp 32f65f1
resolver: fail hard on unresolvable symlinks in ResolveSymlinksToDevices
neddp 27f51d4
test: update linux_platform test for hard-fail on broken symlinks
neddp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
101 changes: 101 additions & 0 deletions
101
infrastructure/devicepathresolver/symlink_device_resolver.go
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| package devicepathresolver | ||
|
|
||
| import ( | ||
| bosherr "github.com/cloudfoundry/bosh-utils/errors" | ||
| boshlog "github.com/cloudfoundry/bosh-utils/logger" | ||
| boshsys "github.com/cloudfoundry/bosh-utils/system" | ||
|
|
||
| boshudev "github.com/cloudfoundry/bosh-agent/v2/platform/udevdevice" | ||
| ) | ||
|
|
||
| const ( | ||
| // NVMeDevicePattern is a glob pattern matching NVMe namespace devices. | ||
| NVMeDevicePattern = "/dev/nvme*n1" | ||
|
|
||
| // NVMeDevicePathPrefix is the common prefix for NVMe device paths. | ||
| // Used to detect if a device path is an NVMe device. | ||
| NVMeDevicePathPrefix = "/dev/nvme" | ||
| ) | ||
|
|
||
| type SymlinkDeviceResolver struct { | ||
| fs boshsys.FileSystem | ||
| udev boshudev.UdevDevice | ||
| logger boshlog.Logger | ||
| logTag string | ||
| } | ||
|
|
||
| // NewSymlinkDeviceResolver creates a new symlink device resolver. | ||
| func NewSymlinkDeviceResolver( | ||
| fs boshsys.FileSystem, | ||
| udev boshudev.UdevDevice, | ||
| logger boshlog.Logger, | ||
| ) *SymlinkDeviceResolver { | ||
| return &SymlinkDeviceResolver{ | ||
| fs: fs, | ||
| udev: udev, | ||
| logger: logger, | ||
| logTag: "SymlinkDeviceResolver", | ||
| } | ||
| } | ||
|
|
||
| // ResolveSymlinksToDevices resolves all symlinks matching the given pattern | ||
| // and returns a map of resolved device paths -> symlink paths. | ||
| // | ||
| // udevadm trigger and settle are called before globbing to avoid a race condition: | ||
| // NVMe block devices (/dev/nvme*) appear synchronously at boot, but the | ||
| // /dev/disk/by-id/ symlinks are created asynchronously by udev. Without waiting, | ||
| // globbing may return no symlinks, causing all NVMe devices to be misidentified | ||
| // as instance storage (instead of EBS/managed volumes). | ||
| func (r *SymlinkDeviceResolver) ResolveSymlinksToDevices(symlinkPattern string) (map[string]string, error) { | ||
| if err := r.udev.Trigger(); err != nil { | ||
| return nil, bosherr.WrapError(err, "Running udevadm trigger") | ||
| } | ||
| if err := r.udev.Settle(); err != nil { | ||
| return nil, bosherr.WrapError(err, "Running udevadm settle") | ||
| } | ||
|
|
||
| symlinks, err := r.fs.Glob(symlinkPattern) | ||
| if err != nil { | ||
| return nil, bosherr.WrapErrorf(err, "Globbing symlinks with pattern '%s'", symlinkPattern) | ||
| } | ||
|
|
||
| result := make(map[string]string) | ||
| for _, symlink := range symlinks { | ||
| absPath, err := r.fs.ReadAndFollowLink(symlink) | ||
| if err != nil { | ||
| return nil, bosherr.WrapErrorf(err, "Resolving managed volume symlink '%s'", symlink) | ||
| } | ||
|
|
||
| r.logger.Debug(r.logTag, "Resolved symlink: %s -> %s", symlink, absPath) | ||
| result[absPath] = symlink | ||
| } | ||
|
|
||
| return result, nil | ||
| } | ||
|
|
||
| // GetDevicesByPattern returns all devices matching the given pattern. | ||
| func (r *SymlinkDeviceResolver) GetDevicesByPattern(devicePattern string) ([]string, error) { | ||
| devices, err := r.fs.Glob(devicePattern) | ||
| if err != nil { | ||
| return nil, bosherr.WrapErrorf(err, "Globbing devices with pattern '%s'", devicePattern) | ||
| } | ||
|
|
||
| r.logger.Debug(r.logTag, "Found devices matching '%s': %v", devicePattern, devices) | ||
| return devices, nil | ||
| } | ||
|
|
||
| // FilterDevices returns devices that are NOT in the exclusion map. | ||
| // This is used to filter out IaaS-managed volumes (EBS, Azure Managed Disks, etc.) | ||
| // from the list of all NVMe devices, leaving only instance/ephemeral storage. | ||
| func (r *SymlinkDeviceResolver) FilterDevices(allDevices []string, excludeDevices map[string]string) []string { | ||
| var filtered []string | ||
| for _, device := range allDevices { | ||
| if _, excluded := excludeDevices[device]; !excluded { | ||
| filtered = append(filtered, device) | ||
| r.logger.Debug(r.logTag, "Including device: %s", device) | ||
| } else { | ||
| r.logger.Debug(r.logTag, "Excluding device: %s (symlink: %s)", device, excludeDevices[device]) | ||
| } | ||
| } | ||
| return filtered | ||
| } |
158 changes: 158 additions & 0 deletions
158
infrastructure/devicepathresolver/symlink_device_resolver_test.go
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| package devicepathresolver_test | ||
|
|
||
| import ( | ||
| "errors" | ||
| "os" | ||
| "runtime" | ||
|
|
||
| . "github.com/onsi/ginkgo/v2" | ||
| . "github.com/onsi/gomega" | ||
|
|
||
| boshlog "github.com/cloudfoundry/bosh-utils/logger" | ||
| fakesys "github.com/cloudfoundry/bosh-utils/system/fakes" | ||
|
|
||
| . "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver" | ||
| fakeudev "github.com/cloudfoundry/bosh-agent/v2/platform/udevdevice/fakes" | ||
| ) | ||
|
|
||
| var _ = Describe("SymlinkDeviceResolver", func() { | ||
| var ( | ||
| fs *fakesys.FakeFileSystem | ||
| udev *fakeudev.FakeUdevDevice | ||
| logger boshlog.Logger | ||
| resolver *SymlinkDeviceResolver | ||
| ) | ||
|
|
||
| BeforeEach(func() { | ||
| if runtime.GOOS == "windows" { | ||
| Skip("Not applicable on Windows") | ||
| } | ||
|
|
||
| fs = fakesys.NewFakeFileSystem() | ||
| udev = fakeudev.NewFakeUdevDevice() | ||
| logger = boshlog.NewLogger(boshlog.LevelNone) | ||
| resolver = NewSymlinkDeviceResolver(fs, udev, logger) | ||
| }) | ||
|
|
||
| Describe("ResolveSymlinksToDevices", func() { | ||
| It("returns empty map when no symlinks match the pattern", func() { | ||
| fs.SetGlob("/dev/disk/by-id/nvme-*", []string{}) | ||
|
|
||
| result, err := resolver.ResolveSymlinksToDevices("/dev/disk/by-id/nvme-*") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(result).To(BeEmpty()) | ||
| }) | ||
|
|
||
| It("resolves symlinks to their target device paths", func() { | ||
| err := fs.MkdirAll("/dev/disk/by-id", os.FileMode(0750)) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| // Create target device files | ||
| err = fs.WriteFileString("/dev/nvme1n1", "") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| err = fs.WriteFileString("/dev/nvme2n1", "") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| fs.SetGlob("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*", []string{ | ||
| "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123", | ||
| "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol456", | ||
| }) | ||
| err = fs.Symlink("/dev/nvme1n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| err = fs.Symlink("/dev/nvme2n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol456") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| result, err := resolver.ResolveSymlinksToDevices("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(result).To(HaveLen(2)) | ||
| Expect(result["/dev/nvme1n1"]).To(Equal("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123")) | ||
| Expect(result["/dev/nvme2n1"]).To(Equal("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol456")) | ||
| }) | ||
|
|
||
| It("skips symlinks that cannot be resolved", func() { | ||
| err := fs.MkdirAll("/dev/disk/by-id", os.FileMode(0750)) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| // Create target device file for valid symlink | ||
| err = fs.WriteFileString("/dev/nvme1n1", "") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| fs.SetGlob("/dev/disk/by-id/nvme-*", []string{ | ||
| "/dev/disk/by-id/nvme-valid", | ||
| "/dev/disk/by-id/nvme-invalid", | ||
| }) | ||
| err = fs.Symlink("/dev/nvme1n1", "/dev/disk/by-id/nvme-valid") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| // nvme-invalid has no symlink target | ||
|
|
||
| result, err := resolver.ResolveSymlinksToDevices("/dev/disk/by-id/nvme-*") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(result).To(HaveLen(1)) | ||
| Expect(result["/dev/nvme1n1"]).To(Equal("/dev/disk/by-id/nvme-valid")) | ||
| }) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| It("returns error when glob fails", func() { | ||
| fs.GlobErr = errors.New("glob error") | ||
|
|
||
| _, err := resolver.ResolveSymlinksToDevices("/dev/disk/by-id/nvme-*") | ||
| Expect(err).To(HaveOccurred()) | ||
| Expect(err.Error()).To(ContainSubstring("glob error")) | ||
| }) | ||
| }) | ||
|
|
||
| Describe("GetDevicesByPattern", func() { | ||
| It("returns devices matching the pattern", func() { | ||
| fs.SetGlob("/dev/nvme*n1", []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}) | ||
|
|
||
| devices, err := resolver.GetDevicesByPattern("/dev/nvme*n1") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(devices).To(ConsistOf("/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1")) | ||
| }) | ||
|
|
||
| It("returns empty slice when no devices match", func() { | ||
| fs.SetGlob("/dev/nvme*n1", []string{}) | ||
|
|
||
| devices, err := resolver.GetDevicesByPattern("/dev/nvme*n1") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(devices).To(BeEmpty()) | ||
| }) | ||
|
|
||
| It("returns error when glob fails", func() { | ||
| fs.GlobErr = errors.New("glob error") | ||
|
|
||
| _, err := resolver.GetDevicesByPattern("/dev/nvme*n1") | ||
| Expect(err).To(HaveOccurred()) | ||
| }) | ||
| }) | ||
|
|
||
| Describe("FilterDevices", func() { | ||
| It("returns devices not in the exclusion map", func() { | ||
| allDevices := []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1", "/dev/nvme3n1"} | ||
| excludeDevices := map[string]string{ | ||
| "/dev/nvme1n1": "/dev/disk/by-id/ebs-vol1", | ||
| "/dev/nvme2n1": "/dev/disk/by-id/ebs-vol2", | ||
| } | ||
|
|
||
| filtered := resolver.FilterDevices(allDevices, excludeDevices) | ||
| Expect(filtered).To(ConsistOf("/dev/nvme0n1", "/dev/nvme3n1")) | ||
| }) | ||
|
|
||
| It("returns all devices when exclusion map is empty", func() { | ||
| allDevices := []string{"/dev/nvme0n1", "/dev/nvme1n1"} | ||
| excludeDevices := map[string]string{} | ||
|
|
||
| filtered := resolver.FilterDevices(allDevices, excludeDevices) | ||
| Expect(filtered).To(ConsistOf("/dev/nvme0n1", "/dev/nvme1n1")) | ||
| }) | ||
|
|
||
| It("returns empty slice when all devices are excluded", func() { | ||
| allDevices := []string{"/dev/nvme0n1"} | ||
| excludeDevices := map[string]string{ | ||
| "/dev/nvme0n1": "/dev/disk/by-id/ebs-vol1", | ||
| } | ||
|
|
||
| filtered := resolver.FilterDevices(allDevices, excludeDevices) | ||
| Expect(filtered).To(BeEmpty()) | ||
| }) | ||
| }) | ||
| }) | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.