diff --git a/agent/bootstrap_test.go b/agent/bootstrap_test.go index e526b9959..0bdaa6940 100644 --- a/agent/bootstrap_test.go +++ b/agent/bootstrap_test.go @@ -10,6 +10,7 @@ import ( "time" boshlogstarprovider "github.com/cloudfoundry/bosh-agent/v2/agent/logstarprovider" + "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -1132,16 +1133,7 @@ var _ = Describe("bootstrap", func() { monitRetryStrategy := boshretry.NewAttemptRetryStrategy(10, 1*time.Second, monitRetryable, logger) devicePathResolver := fakedevicepathresolver.NewFakeDevicePathResolver() - instanceStorageResolver := fakedevicepathresolver.NewFakeInstanceStorageResolver() - - // Default stub: instance storage resolver returns device paths as-is - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - paths := make([]string, len(devices)) - for i, device := range devices { - paths[i] = device.Path - } - return paths, nil - } + symlinkDeviceResolver := devicepathresolver.NewSymlinkDeviceResolver(fs, logger) fakeUUIDGenerator := boshuuid.NewGenerator() routesSearcher := boshnet.NewRoutesSearcher(logger, runner, nil) @@ -1163,7 +1155,7 @@ var _ = Describe("bootstrap", func() { ubuntuCertManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, state, linuxOptions, logger, diff --git a/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver.go b/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver.go deleted file mode 100644 index 0c366990e..000000000 --- a/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver.go +++ /dev/null @@ -1,99 +0,0 @@ -package devicepathresolver - -import ( - "strings" - - boshlog "github.com/cloudfoundry/bosh-utils/logger" - boshsys "github.com/cloudfoundry/bosh-utils/system" - - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -// autoDetectingInstanceStorageResolver automatically detects whether to use -// NVMe-specific logic or identity resolution based on device paths from the CPI. -// If any device path starts with "/dev/nvme", it uses AWS NVMe discovery logic. -// Otherwise, it uses the CPI-provided paths directly (identity resolution). -type autoDetectingInstanceStorageResolver struct { - fs boshsys.FileSystem - devicePathResolver DevicePathResolver - logger boshlog.Logger - ebsSymlinkPattern string - nvmeDevicePattern string - awsNVMeResolver InstanceStorageResolver - identityResolver InstanceStorageResolver - resolverInitialized bool - useNVMeResolver bool -} - -// NewAutoDetectingInstanceStorageResolver creates a resolver that automatically -// detects NVMe instances based on device paths from the CPI. -func NewAutoDetectingInstanceStorageResolver( - fs boshsys.FileSystem, - devicePathResolver DevicePathResolver, - logger boshlog.Logger, - ebsSymlinkPattern string, - nvmeDevicePattern string, -) InstanceStorageResolver { - if ebsSymlinkPattern == "" { - ebsSymlinkPattern = "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" - } - if nvmeDevicePattern == "" { - nvmeDevicePattern = "/dev/nvme*n1" - } - - return &autoDetectingInstanceStorageResolver{ - fs: fs, - devicePathResolver: devicePathResolver, - logger: logger, - ebsSymlinkPattern: ebsSymlinkPattern, - nvmeDevicePattern: nvmeDevicePattern, - resolverInitialized: false, - } -} - -func (r *autoDetectingInstanceStorageResolver) DiscoverInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { - if len(devices) == 0 { - return []string{}, nil - } - - // Auto-detect on first call by checking if any device path starts with /dev/nvme - if !r.resolverInitialized { - r.useNVMeResolver = r.detectNVMeDevices(devices) - - if r.useNVMeResolver { - r.logger.Info("AutoDetectingInstanceStorageResolver", - "Detected NVMe device paths from CPI - using AWS NVMe instance storage discovery") - r.awsNVMeResolver = NewAWSNVMeInstanceStorageResolver( - r.fs, - r.devicePathResolver, - r.logger, - r.ebsSymlinkPattern, - r.nvmeDevicePattern, - ) - } else { - r.logger.Info("AutoDetectingInstanceStorageResolver", - "Detected non-NVMe device paths from CPI - using identity resolution") - r.identityResolver = NewIdentityInstanceStorageResolver(r.devicePathResolver) - } - - r.resolverInitialized = true - } - - if r.useNVMeResolver { - return r.awsNVMeResolver.DiscoverInstanceStorage(devices) - } - return r.identityResolver.DiscoverInstanceStorage(devices) -} - -// detectNVMeDevices checks if any device path from the CPI starts with /dev/nvme -// This matches the CPI's logic: if current_disk =~ /^\/dev\/nvme/ -func (r *autoDetectingInstanceStorageResolver) detectNVMeDevices(devices []boshsettings.DiskSettings) bool { - for _, device := range devices { - if strings.HasPrefix(device.Path, "/dev/nvme") { - r.logger.Debug("AutoDetectingInstanceStorageResolver", - "Detected NVMe from CPI-provided path: %s", device.Path) - return true - } - } - return false -} diff --git a/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver_test.go b/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver_test.go deleted file mode 100644 index ea59fdef0..000000000 --- a/infrastructure/devicepathresolver/auto_detecting_instance_storage_resolver_test.go +++ /dev/null @@ -1,115 +0,0 @@ -package devicepathresolver_test - -import ( - "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" - fakedpresolv "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver/fakes" - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -var _ = Describe("AutoDetectingInstanceStorageResolver", func() { - var ( - resolver InstanceStorageResolver - fakeFS *fakesys.FakeFileSystem - fakeDevicePathResolver *fakedpresolv.FakeDevicePathResolver - logger boshlog.Logger - ) - - BeforeEach(func() { - if runtime.GOOS != "linux" { - Skip("Only supported on Linux") - } - fakeFS = fakesys.NewFakeFileSystem() - fakeDevicePathResolver = fakedpresolv.NewFakeDevicePathResolver() - logger = boshlog.NewLogger(boshlog.LevelNone) - }) - - Context("when CPI provides NVMe device paths", func() { - It("automatically uses AWS NVMe instance storage discovery", func() { - resolver = NewAutoDetectingInstanceStorageResolver( - fakeFS, - fakeDevicePathResolver, - logger, - "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*", - "/dev/nvme*n1", - ) - - devices := []boshsettings.DiskSettings{ - {Path: "/dev/nvme0n1"}, - {Path: "/dev/nvme1n1"}, - } - - err := fakeFS.WriteFileString("/dev/nvme0n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme1n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme2n1", "") - Expect(err).NotTo(HaveOccurred()) - - fakeFS.GlobStub = func(pattern string) ([]string, error) { - if pattern == "/dev/nvme*n1" { - return []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}, nil - } - if pattern == "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" { - return []string{"/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root"}, nil - } - return []string{}, nil - } - - err = fakeFS.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root") - Expect(err).NotTo(HaveOccurred()) - - paths, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(Equal([]string{"/dev/nvme1n1", "/dev/nvme2n1"})) - }) - }) - - Context("when CPI provides non-NVMe device paths", func() { - It("automatically uses identity resolution", func() { - resolver = NewAutoDetectingInstanceStorageResolver( - fakeFS, - fakeDevicePathResolver, - logger, - "", - "", - ) - - devices := []boshsettings.DiskSettings{ - {Path: "/dev/xvdba"}, - {Path: "/dev/xvdbb"}, - } - - fakeDevicePathResolver.GetRealDevicePathStub = func(diskSettings boshsettings.DiskSettings) (string, bool, error) { - return diskSettings.Path, false, nil - } - - paths, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(Equal([]string{"/dev/xvdba", "/dev/xvdbb"})) - }) - }) - - Context("when device list is empty", func() { - It("returns empty list", func() { - resolver = NewAutoDetectingInstanceStorageResolver( - fakeFS, - fakeDevicePathResolver, - logger, - "", - "", - ) - - paths, err := resolver.DiscoverInstanceStorage([]boshsettings.DiskSettings{}) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(BeEmpty()) - }) - }) -}) diff --git a/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver.go b/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver.go deleted file mode 100644 index fd73a5610..000000000 --- a/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver.go +++ /dev/null @@ -1,93 +0,0 @@ -package devicepathresolver - -import ( - "sort" - - bosherr "github.com/cloudfoundry/bosh-utils/errors" - boshlog "github.com/cloudfoundry/bosh-utils/logger" - boshsys "github.com/cloudfoundry/bosh-utils/system" - - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -type awsNVMeInstanceStorageResolver struct { - fs boshsys.FileSystem - devicePathResolver DevicePathResolver - logger boshlog.Logger - logTag string - ebsSymlinkPattern string - nvmeDevicePattern string -} - -func NewAWSNVMeInstanceStorageResolver( - fs boshsys.FileSystem, - devicePathResolver DevicePathResolver, - logger boshlog.Logger, - ebsSymlinkPattern string, - nvmeDevicePattern string, -) InstanceStorageResolver { - if ebsSymlinkPattern == "" { - ebsSymlinkPattern = "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" - } - if nvmeDevicePattern == "" { - nvmeDevicePattern = "/dev/nvme*n1" - } - - return &awsNVMeInstanceStorageResolver{ - fs: fs, - devicePathResolver: devicePathResolver, - logger: logger, - logTag: "AWSNVMeInstanceStorageResolver", - ebsSymlinkPattern: ebsSymlinkPattern, - nvmeDevicePattern: nvmeDevicePattern, - } -} - -func (r *awsNVMeInstanceStorageResolver) DiscoverInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { - if len(devices) == 0 { - return []string{}, nil - } - - allNvmeDevices, err := r.fs.Glob(r.nvmeDevicePattern) - if err != nil { - return nil, bosherr.WrapError(err, "Globbing NVMe devices") - } - - r.logger.Debug(r.logTag, "Found NVMe devices: %v", allNvmeDevices) - - ebsSymlinks, err := r.fs.Glob(r.ebsSymlinkPattern) - if err != nil { - return nil, bosherr.WrapError(err, "Globbing EBS symlinks") - } - - ebsDevices := make(map[string]bool) - for _, symlink := range ebsSymlinks { - absPath, err := r.fs.ReadAndFollowLink(symlink) - if err != nil { - r.logger.Debug(r.logTag, "Could not resolve symlink %s: %s", symlink, err.Error()) - continue - } - - r.logger.Debug(r.logTag, "EBS volume: %s -> %s", symlink, absPath) - ebsDevices[absPath] = true - } - - var instanceStorage []string - for _, devicePath := range allNvmeDevices { - if !ebsDevices[devicePath] { - instanceStorage = append(instanceStorage, devicePath) - r.logger.Info(r.logTag, "Discovered instance storage: %s", devicePath) - } else { - r.logger.Debug(r.logTag, "Excluding EBS volume: %s", devicePath) - } - } - - sort.Strings(instanceStorage) - - if len(instanceStorage) != len(devices) { - return nil, bosherr.Errorf("Expected %d instance storage devices but discovered %d: %v", - len(devices), len(instanceStorage), instanceStorage) - } - - return instanceStorage, nil -} diff --git a/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver_test.go b/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver_test.go deleted file mode 100644 index 047ab0f4f..000000000 --- a/infrastructure/devicepathresolver/aws_nvme_instance_storage_resolver_test.go +++ /dev/null @@ -1,124 +0,0 @@ -package devicepathresolver_test - -import ( - "runtime" - - boshlog "github.com/cloudfoundry/bosh-utils/logger" - fakesys "github.com/cloudfoundry/bosh-utils/system/fakes" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - . "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver" - fakedpresolv "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver/fakes" - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -var _ = Describe("AWSNVMeInstanceStorageResolver", func() { - var ( - resolver InstanceStorageResolver - fakeFS *fakesys.FakeFileSystem - fakeDevicePathResolver *fakedpresolv.FakeDevicePathResolver - logger boshlog.Logger - ) - BeforeEach(func() { - if runtime.GOOS != "linux" { - Skip("Only supported on Linux") - } - fakeFS = fakesys.NewFakeFileSystem() - fakeDevicePathResolver = fakedpresolv.NewFakeDevicePathResolver() - logger = boshlog.NewLogger(boshlog.LevelNone) - resolver = NewAWSNVMeInstanceStorageResolver(fakeFS, fakeDevicePathResolver, logger, "", "") - }) - Describe("DiscoverInstanceStorage", func() { - Context("when devices are NVMe", func() { - It("discovers instance storage by filtering out EBS volumes", func() { - devices := []boshsettings.DiskSettings{ - {Path: "/dev/nvme0n1"}, - {Path: "/dev/nvme1n1"}, - } - - // Create device files - err := fakeFS.WriteFileString("/dev/nvme0n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme1n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme2n1", "") - Expect(err).NotTo(HaveOccurred()) - - fakeFS.GlobStub = func(pattern string) ([]string, error) { - if pattern == "/dev/nvme*n1" { - return []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}, nil - } - if pattern == "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" { - return []string{"/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root"}, nil - } - return nil, nil - } - - err = fakeFS.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root") - Expect(err).NotTo(HaveOccurred()) - - paths, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(Equal([]string{"/dev/nvme1n1", "/dev/nvme2n1"})) - }) - It("returns error if not enough instance storage devices found", func() { - devices := []boshsettings.DiskSettings{ - {Path: "/dev/nvme0n1"}, - {Path: "/dev/nvme1n1"}, - {Path: "/dev/nvme2n1"}, - } - - // Create device files - err := fakeFS.WriteFileString("/dev/nvme0n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme1n1", "") - Expect(err).NotTo(HaveOccurred()) - - fakeFS.GlobStub = func(pattern string) ([]string, error) { - if pattern == "/dev/nvme*n1" { - return []string{"/dev/nvme0n1", "/dev/nvme1n1"}, nil - } - if pattern == "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" { - return []string{"/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root"}, nil - } - return nil, nil - } - - err = fakeFS.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol-root") - Expect(err).NotTo(HaveOccurred()) - - _, err = resolver.DiscoverInstanceStorage(devices) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Expected 3 instance storage devices but discovered 1")) - }) - It("returns error if too many instance storage devices found", func() { - devices := []boshsettings.DiskSettings{ - {Path: "/dev/nvme0n1"}, - {Path: "/dev/nvme1n1"}, - } - - // Create device files - err := fakeFS.WriteFileString("/dev/nvme0n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme1n1", "") - Expect(err).NotTo(HaveOccurred()) - err = fakeFS.WriteFileString("/dev/nvme2n1", "") - Expect(err).NotTo(HaveOccurred()) - - fakeFS.GlobStub = func(pattern string) ([]string, error) { - if pattern == "/dev/nvme*n1" { - return []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}, nil - } - // No EBS symlinks - all devices are instance storage - return nil, nil - } - - // No symlinks to filter out - all 3 devices will be returned as instance storage - _, err = resolver.DiscoverInstanceStorage(devices) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Expected 2 instance storage devices but discovered 3")) - }) - }) - }) -}) diff --git a/infrastructure/devicepathresolver/fakes/fake_instance_storage_resolver.go b/infrastructure/devicepathresolver/fakes/fake_instance_storage_resolver.go deleted file mode 100644 index a816e0bdd..000000000 --- a/infrastructure/devicepathresolver/fakes/fake_instance_storage_resolver.go +++ /dev/null @@ -1,32 +0,0 @@ -package fakes - -import ( - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -type FakeInstanceStorageResolver struct { - DiscoverInstanceStorageDevices []boshsettings.DiskSettings - DiscoverInstanceStoragePaths []string - DiscoverInstanceStorageErr error - DiscoverInstanceStorageCallCount int - DiscoverInstanceStorageStub func([]boshsettings.DiskSettings) ([]string, error) -} - -func NewFakeInstanceStorageResolver() *FakeInstanceStorageResolver { - return &FakeInstanceStorageResolver{} -} - -func (r *FakeInstanceStorageResolver) DiscoverInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { - r.DiscoverInstanceStorageDevices = devices - r.DiscoverInstanceStorageCallCount++ - - if r.DiscoverInstanceStorageStub != nil { - return r.DiscoverInstanceStorageStub(devices) - } - - if r.DiscoverInstanceStorageErr != nil { - return nil, r.DiscoverInstanceStorageErr - } - - return r.DiscoverInstanceStoragePaths, nil -} diff --git a/infrastructure/devicepathresolver/fallback_device_path_resolver_test.go b/infrastructure/devicepathresolver/fallback_device_path_resolver_test.go index f26db30b8..1a588da6a 100644 --- a/infrastructure/devicepathresolver/fallback_device_path_resolver_test.go +++ b/infrastructure/devicepathresolver/fallback_device_path_resolver_test.go @@ -51,7 +51,7 @@ var _ = Describe("FallbackDevicePathResolver", func() { It("does not call the secondary resolver", func() { _, _, err := pathResolver.GetRealDevicePath(diskSettings) Expect(err).ToNot(HaveOccurred()) - Expect(secondaryResolver.GetRealDevicePathDiskSettings).To(Equal(boshsettings.DiskSettings{})) + Expect(secondaryResolver.GetRealDevicePathDiskSettings).To(BeEmpty()) }) }) @@ -68,7 +68,7 @@ var _ = Describe("FallbackDevicePathResolver", func() { Expect(timeout).To(BeFalse()) Expect(realPath).To(Equal("/dev/sdc")) - Expect(secondaryResolver.GetRealDevicePathDiskSettings).To(Equal(diskSettings)) + Expect(secondaryResolver.GetRealDevicePathDiskSettings).To(ContainElement(diskSettings)) }) Context("when secondary resolver also errors", func() { diff --git a/infrastructure/devicepathresolver/identity_instance_storage_resolver.go b/infrastructure/devicepathresolver/identity_instance_storage_resolver.go deleted file mode 100644 index 15bc1f4a5..000000000 --- a/infrastructure/devicepathresolver/identity_instance_storage_resolver.go +++ /dev/null @@ -1,31 +0,0 @@ -package devicepathresolver - -import ( - bosherr "github.com/cloudfoundry/bosh-utils/errors" - - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -// identityInstanceStorageResolver returns device paths as-is from the CPI -type identityInstanceStorageResolver struct { - devicePathResolver DevicePathResolver -} - -// NewIdentityInstanceStorageResolver creates a resolver that uses CPI-provided paths directly -func NewIdentityInstanceStorageResolver(devicePathResolver DevicePathResolver) InstanceStorageResolver { - return &identityInstanceStorageResolver{ - devicePathResolver: devicePathResolver, - } -} - -func (r *identityInstanceStorageResolver) DiscoverInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { - paths := make([]string, len(devices)) - for i, device := range devices { - realPath, _, err := r.devicePathResolver.GetRealDevicePath(device) - if err != nil { - return nil, bosherr.WrapErrorf(err, "Getting device %s path", device) - } - paths[i] = realPath - } - return paths, nil -} diff --git a/infrastructure/devicepathresolver/identity_instance_storage_resolver_test.go b/infrastructure/devicepathresolver/identity_instance_storage_resolver_test.go deleted file mode 100644 index ba050543c..000000000 --- a/infrastructure/devicepathresolver/identity_instance_storage_resolver_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package devicepathresolver_test - -import ( - "errors" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - . "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver" - fakedpresolv "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver/fakes" - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -var _ = Describe("IdentityInstanceStorageResolver", func() { - var ( - resolver InstanceStorageResolver - fakeDevicePathResolver *fakedpresolv.FakeDevicePathResolver - ) - - BeforeEach(func() { - fakeDevicePathResolver = fakedpresolv.NewFakeDevicePathResolver() - resolver = NewIdentityInstanceStorageResolver(fakeDevicePathResolver) - }) - - Describe("DiscoverInstanceStorage", func() { - It("returns device paths resolved by the underlying device path resolver", func() { - devices := []boshsettings.DiskSettings{ - {Path: "/dev/xvdb"}, - {Path: "/dev/xvdc"}, - } - - fakeDevicePathResolver.GetRealDevicePathStub = func(diskSettings boshsettings.DiskSettings) (string, bool, error) { - return diskSettings.Path, false, nil - } - - paths, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(Equal([]string{"/dev/xvdb", "/dev/xvdc"})) - Expect(fakeDevicePathResolver.GetRealDevicePathCallCount()).To(Equal(2)) - }) - - It("returns error if device path resolver fails", func() { - devices := []boshsettings.DiskSettings{ - {Path: "/dev/xvdb"}, - } - - fakeDevicePathResolver.GetRealDevicePathReturns("", false, errors.New("fake-error")) - - _, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("fake-error")) - }) - - It("returns empty slice for empty input", func() { - devices := []boshsettings.DiskSettings{} - - paths, err := resolver.DiscoverInstanceStorage(devices) - Expect(err).NotTo(HaveOccurred()) - Expect(paths).To(Equal([]string{})) - }) - }) -}) diff --git a/infrastructure/devicepathresolver/instance_storage_resolver.go b/infrastructure/devicepathresolver/instance_storage_resolver.go deleted file mode 100644 index b33888df5..000000000 --- a/infrastructure/devicepathresolver/instance_storage_resolver.go +++ /dev/null @@ -1,13 +0,0 @@ -package devicepathresolver - -import ( - boshsettings "github.com/cloudfoundry/bosh-agent/v2/settings" -) - -// InstanceStorageResolver discovers instance storage devices, filtering out -// IaaS-managed volumes like EBS, persistent disks, etc. -type InstanceStorageResolver interface { - // DiscoverInstanceStorage takes a list of expected ephemeral disks and returns - // the actual device paths for instance storage, excluding IaaS-managed volumes - DiscoverInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) -} diff --git a/infrastructure/devicepathresolver/symlink_device_resolver.go b/infrastructure/devicepathresolver/symlink_device_resolver.go new file mode 100644 index 000000000..3ae8f0a2a --- /dev/null +++ b/infrastructure/devicepathresolver/symlink_device_resolver.go @@ -0,0 +1,84 @@ +package devicepathresolver + +import ( + bosherr "github.com/cloudfoundry/bosh-utils/errors" + boshlog "github.com/cloudfoundry/bosh-utils/logger" + boshsys "github.com/cloudfoundry/bosh-utils/system" +) + +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 + logger boshlog.Logger + logTag string +} + +// NewSymlinkDeviceResolver creates a new symlink device resolver. +func NewSymlinkDeviceResolver( + fs boshsys.FileSystem, + logger boshlog.Logger, +) *SymlinkDeviceResolver { + return &SymlinkDeviceResolver{ + fs: fs, + logger: logger, + logTag: "SymlinkDeviceResolver", + } +} + +// ResolveSymlinksToDevices resolves all symlinks matching the given pattern +// and returns a map of resolved device paths -> symlink paths. +func (r *SymlinkDeviceResolver) ResolveSymlinksToDevices(symlinkPattern string) (map[string]string, error) { + 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 { + r.logger.Debug(r.logTag, "Could not resolve symlink %s: %s", symlink, err.Error()) + continue + } + + 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 +} diff --git a/infrastructure/devicepathresolver/symlink_device_resolver_test.go b/infrastructure/devicepathresolver/symlink_device_resolver_test.go new file mode 100644 index 000000000..39bab0e3f --- /dev/null +++ b/infrastructure/devicepathresolver/symlink_device_resolver_test.go @@ -0,0 +1,150 @@ +package devicepathresolver_test + +import ( + "errors" + "os" + + . "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" +) + +var _ = Describe("SymlinkDeviceResolver", func() { + var ( + fs *fakesys.FakeFileSystem + logger boshlog.Logger + resolver *SymlinkDeviceResolver + ) + + BeforeEach(func() { + fs = fakesys.NewFakeFileSystem() + logger = boshlog.NewLogger(boshlog.LevelNone) + resolver = NewSymlinkDeviceResolver(fs, 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")) + }) + + 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()) + }) + }) +}) diff --git a/platform/linux_platform.go b/platform/linux_platform.go index 6f0ff917c..ed0bc6390 100644 --- a/platform/linux_platform.go +++ b/platform/linux_platform.go @@ -7,6 +7,7 @@ import ( "path" "path/filepath" "regexp" + "sort" "strconv" "strings" "text/template" @@ -77,16 +78,12 @@ type LinuxOptions struct { // possible values: virtio, scsi, iscsi, "" DevicePathResolutionType string - // Strategy for discovering instance storage devices; - // possible values: aws-nvme, "" - InstanceStorageResolutionType string - // Pattern for identifying IaaS-managed volumes (e.g., EBS on AWS) - // Used with InstanceStorageResolutionType to filter out non-instance storage + // Used to filter out non-instance storage on NVMe systems InstanceStorageManagedVolumePattern string // Pattern for discovering all potential instance storage devices - // Used with InstanceStorageResolutionType for device enumeration + // Used for device enumeration on NVMe systems InstanceStorageDevicePattern string // Strategy for resolving ephemeral & persistent disk partitioners; @@ -107,28 +104,28 @@ type LinuxOptions struct { } type linux struct { - fs boshsys.FileSystem - cmdRunner boshsys.CmdRunner - collector boshstats.Collector - compressor boshcmd.Compressor - copier boshcmd.Copier - dirProvider boshdirs.Provider - vitalsService boshvitals.Service - cdutil cdrom.CDUtil - diskManager boshdisk.Manager - netManager boshnet.Manager - certManager boshcert.Manager - monitRetryStrategy boshretry.RetryStrategy - devicePathResolver boshdpresolv.DevicePathResolver - instanceStorageResolver boshdpresolv.InstanceStorageResolver - options LinuxOptions - state *BootstrapState - logger boshlog.Logger - defaultNetworkResolver boshsettings.DefaultNetworkResolver - uuidGenerator boshuuid.Generator - auditLogger AuditLogger - logsTarProvider boshlogstarprovider.LogsTarProvider - serviceManager servicemanager.ServiceManager + fs boshsys.FileSystem + cmdRunner boshsys.CmdRunner + collector boshstats.Collector + compressor boshcmd.Compressor + copier boshcmd.Copier + dirProvider boshdirs.Provider + vitalsService boshvitals.Service + cdutil cdrom.CDUtil + diskManager boshdisk.Manager + netManager boshnet.Manager + certManager boshcert.Manager + monitRetryStrategy boshretry.RetryStrategy + devicePathResolver boshdpresolv.DevicePathResolver + symlinkDeviceResolver *boshdpresolv.SymlinkDeviceResolver + options LinuxOptions + state *BootstrapState + logger boshlog.Logger + defaultNetworkResolver boshsettings.DefaultNetworkResolver + uuidGenerator boshuuid.Generator + auditLogger AuditLogger + logsTarProvider boshlogstarprovider.LogsTarProvider + serviceManager servicemanager.ServiceManager } func NewLinuxPlatform( @@ -145,7 +142,7 @@ func NewLinuxPlatform( certManager boshcert.Manager, monitRetryStrategy boshretry.RetryStrategy, devicePathResolver boshdpresolv.DevicePathResolver, - instanceStorageResolver boshdpresolv.InstanceStorageResolver, + symlinkDeviceResolver *boshdpresolv.SymlinkDeviceResolver, state *BootstrapState, options LinuxOptions, logger boshlog.Logger, @@ -156,28 +153,28 @@ func NewLinuxPlatform( serviceManager servicemanager.ServiceManager, ) Platform { return &linux{ - fs: fs, - cmdRunner: cmdRunner, - collector: collector, - compressor: compressor, - copier: copier, - dirProvider: dirProvider, - vitalsService: vitalsService, - cdutil: cdutil, - diskManager: diskManager, - netManager: netManager, - certManager: certManager, - monitRetryStrategy: monitRetryStrategy, - devicePathResolver: devicePathResolver, - instanceStorageResolver: instanceStorageResolver, - state: state, - options: options, - logger: logger, - defaultNetworkResolver: defaultNetworkResolver, - uuidGenerator: uuidGenerator, - auditLogger: auditLogger, - logsTarProvider: logsTarProvider, - serviceManager: serviceManager, + fs: fs, + cmdRunner: cmdRunner, + collector: collector, + compressor: compressor, + copier: copier, + dirProvider: dirProvider, + vitalsService: vitalsService, + cdutil: cdutil, + diskManager: diskManager, + netManager: netManager, + certManager: certManager, + monitRetryStrategy: monitRetryStrategy, + devicePathResolver: devicePathResolver, + symlinkDeviceResolver: symlinkDeviceResolver, + state: state, + options: options, + logger: logger, + defaultNetworkResolver: defaultNetworkResolver, + uuidGenerator: uuidGenerator, + auditLogger: auditLogger, + logsTarProvider: logsTarProvider, + serviceManager: serviceManager, } } @@ -749,7 +746,7 @@ func (p linux) SetupRawEphemeralDisks(devices []boshsettings.DiskSettings) (err p.logger.Info(logTag, "Setting up %d raw ephemeral disk(s)", len(devices)) - instanceStorageDevices, err := p.instanceStorageResolver.DiscoverInstanceStorage(devices) + instanceStorageDevices, err := p.discoverInstanceStorageDevices(devices) if err != nil { return bosherr.WrapError(err, "Discovering instance storage devices") } @@ -802,6 +799,85 @@ func (p linux) SetupRawEphemeralDisks(devices []boshsettings.DiskSettings) (err return nil } +// discoverInstanceStorageDevices finds the actual device paths for instance storage. +// For NVMe devices with a configured managed volume pattern, it uses symlink-based +// filtering to exclude IaaS-managed volumes (e.g., EBS on AWS). +// Otherwise, it uses the DevicePathResolver to resolve each device directly. +func (p linux) discoverInstanceStorageDevices(devices []boshsettings.DiskSettings) ([]string, error) { + if len(devices) == 0 { + return []string{}, nil + } + + // Use NVMe symlink filtering if: + // 1. A managed volume pattern is configured (tells us what to exclude) + // 2. The CPI reports NVMe device paths + // 3. The symlink resolver is available + if p.options.InstanceStorageManagedVolumePattern != "" && + p.symlinkDeviceResolver != nil && + p.hasNVMeDevices(devices) { + return p.discoverNVMeInstanceStorage(devices) + } + + return p.discoverIdentityInstanceStorage(devices) +} + +// hasNVMeDevices checks if any device path from the CPI is an NVMe device. +func (p linux) hasNVMeDevices(devices []boshsettings.DiskSettings) bool { + for _, device := range devices { + if strings.HasPrefix(device.Path, boshdpresolv.NVMeDevicePathPrefix) { + return true + } + } + return false +} + +// discoverNVMeInstanceStorage discovers NVMe instance storage by filtering out +// IaaS-managed volumes (e.g., EBS on AWS, Managed Disks on Azure) using symlinks. +func (p linux) discoverNVMeInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { + nvmePattern := p.options.InstanceStorageDevicePattern + if nvmePattern == "" { + nvmePattern = boshdpresolv.NVMeDevicePattern + } + + allNvmeDevices, err := p.symlinkDeviceResolver.GetDevicesByPattern(nvmePattern) + if err != nil { + return nil, bosherr.WrapError(err, "Globbing NVMe devices") + } + p.logger.Debug(logTag, "Found NVMe devices: %v", allNvmeDevices) + + managedDevices, err := p.symlinkDeviceResolver.ResolveSymlinksToDevices(p.options.InstanceStorageManagedVolumePattern) + if err != nil { + return nil, bosherr.WrapError(err, "Resolving managed disk symlinks") + } + + instanceStorage := p.symlinkDeviceResolver.FilterDevices(allNvmeDevices, managedDevices) + sort.Strings(instanceStorage) + + for _, devicePath := range instanceStorage { + p.logger.Info(logTag, "Discovered instance storage: %s", devicePath) + } + + if len(instanceStorage) != len(devices) { + return nil, bosherr.Errorf("Expected %d instance storage devices but discovered %d: %v", + len(devices), len(instanceStorage), instanceStorage) + } + + return instanceStorage, nil +} + +// discoverIdentityInstanceStorage uses the DevicePathResolver for each device. +func (p linux) discoverIdentityInstanceStorage(devices []boshsettings.DiskSettings) ([]string, error) { + paths := make([]string, len(devices)) + for i, device := range devices { + realPath, _, err := p.devicePathResolver.GetRealDevicePath(device) + if err != nil { + return nil, bosherr.WrapErrorf(err, "Getting device %s path", device) + } + paths[i] = realPath + } + return paths, nil +} + func (p linux) SetupDataDir(jobConfig boshsettings.JobDir, runConfig boshsettings.RunDir) error { dataDir := p.dirProvider.DataDir() diff --git a/platform/linux_platform_test.go b/platform/linux_platform_test.go index b72cb0268..25199c077 100644 --- a/platform/linux_platform_test.go +++ b/platform/linux_platform_test.go @@ -22,6 +22,7 @@ import ( fakeuuidgen "github.com/cloudfoundry/bosh-utils/uuid/fakes" fakelogstarprovider "github.com/cloudfoundry/bosh-agent/v2/agent/logstarprovider/logstarproviderfakes" + boshdpresolv "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver" fakedpresolv "github.com/cloudfoundry/bosh-agent/v2/infrastructure/devicepathresolver/fakes" . "github.com/cloudfoundry/bosh-agent/v2/platform" fakecdrom "github.com/cloudfoundry/bosh-agent/v2/platform/cdrom/fakes" @@ -47,7 +48,7 @@ var _ = Describe("LinuxPlatform", func() { diskManager *diskfakes.FakeManager dirProvider boshdirs.Provider devicePathResolver *fakedpresolv.FakeDevicePathResolver - instanceStorageResolver *fakedpresolv.FakeInstanceStorageResolver + symlinkDeviceResolver *boshdpresolv.SymlinkDeviceResolver platform Platform cdutil *fakecdrom.FakeCDUtil compressor boshcmd.Compressor @@ -90,16 +91,7 @@ var _ = Describe("LinuxPlatform", func() { certManager = new(certfakes.FakeManager) monitRetryStrategy = fakeretry.NewFakeRetryStrategy() devicePathResolver = fakedpresolv.NewFakeDevicePathResolver() - instanceStorageResolver = fakedpresolv.NewFakeInstanceStorageResolver() - - // Default: instance storage resolver returns device paths as-is (identity resolution) - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - paths := make([]string, len(devices)) - for i, device := range devices { - paths[i] = device.Path - } - return paths, nil - } + symlinkDeviceResolver = boshdpresolv.NewSymlinkDeviceResolver(fs, logger) fakeDefaultNetworkResolver = &fakenet.FakeDefaultNetworkResolver{} serviceManager = &servicemanagerfakes.FakeServiceManager{} @@ -160,7 +152,7 @@ var _ = Describe("LinuxPlatform", func() { certManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, state, options, logger, @@ -479,7 +471,7 @@ bosh_foobar:...` certManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, state, options, logger, @@ -717,7 +709,7 @@ bosh_foobar:...` certManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, state, options, logger, @@ -1867,6 +1859,9 @@ Number Start End Size File system Name Flags Context("NVMe instance storage discovery", func() { BeforeEach(func() { + // Enable NVMe instance storage discovery by setting the managed volume pattern + options.InstanceStorageManagedVolumePattern = "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*" + devicePathResolver.GetRealDevicePathStub = func(diskSettings boshsettings.DiskSettings) (string, bool, error) { return diskSettings.Path, false, nil } @@ -1874,19 +1869,6 @@ Number Start End Size File system Name Flags It("discovers instance storage by excluding EBS volumes via symlinks", func() { // Setup: 3 NVMe devices, 2 are EBS (nvme0n1, nvme1n1), 1 is instance storage (nvme2n1) - fs.GlobStub = func(pattern string) ([]string, error) { - switch pattern { - case "/dev/nvme*n1": - return []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}, nil - case "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*": - return []string{ - "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123", - "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol456", - }, nil - default: - return nil, nil - } - } // Create the NVMe device files err := fs.WriteFileString("/dev/nvme0n1", "") @@ -1896,17 +1878,22 @@ Number Start End Size File system Name Flags err = fs.WriteFileString("/dev/nvme2n1", "") Expect(err).ToNot(HaveOccurred()) + // Create symlink directory + err = fs.MkdirAll("/dev/disk/by-id", os.FileMode(0750)) + Expect(err).ToNot(HaveOccurred()) + // Create symlinks for EBS volumes err = fs.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123") Expect(err).ToNot(HaveOccurred()) err = fs.Symlink("/dev/nvme1n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol456") Expect(err).ToNot(HaveOccurred()) - // Configure instance storage resolver to simulate NVMe filtering - // Returns only nvme2n1 (the instance storage device, after filtering EBS) - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - return []string{"/dev/nvme2n1"}, nil - } + // Set up glob patterns for the symlinkDeviceResolver + fs.SetGlob("/dev/nvme*n1", []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}) + 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", + }) // Mock parted output for nvme2n1 (instance storage - needs partitioning) cmdRunner.AddCmdResult("parted -s /dev/nvme2n1 p", fakesys.FakeCmdResult{ @@ -1923,22 +1910,30 @@ Number Start End Size File system Name Flags }) It("returns error when no instance storage devices found but CPI expects some", func() { - // Configure instance storage resolver to return error - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - return nil, errors.New("Expected 1 instance storage devices but discovered 0") - } + // Create the NVMe device file + err := fs.WriteFileString("/dev/nvme0n1", "") + Expect(err).ToNot(HaveOccurred()) + + // Create symlink directory + err = fs.MkdirAll("/dev/disk/by-id", os.FileMode(0750)) + Expect(err).ToNot(HaveOccurred()) - err := platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme2n1"}}) + // All NVMe devices are EBS - no instance storage available + fs.SetGlob("/dev/nvme*n1", []string{"/dev/nvme0n1"}) + fs.SetGlob("/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_*", []string{ + "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123", + }) + err = fs.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123") + Expect(err).ToNot(HaveOccurred()) + + err = platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme2n1"}}) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("Expected 1 instance storage devices but discovered 0")) }) It("returns error when globbing NVMe devices fails", func() { - // Configure instance storage resolver to return error - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - return nil, errors.New("Globbing NVMe devices: permission denied reading /dev") - } + fs.GlobErr = errors.New("permission denied reading /dev") err := platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme1n1"}}) @@ -1946,24 +1941,28 @@ Number Start End Size File system Name Flags Expect(err.Error()).To(ContainSubstring("Globbing NVMe devices")) }) - It("returns error when globbing EBS symlinks fails", func() { - // Configure instance storage resolver to return error - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - return nil, errors.New("Globbing EBS symlinks: permission denied") - } - - err := platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme1n1"}}) + It("skips symlinks that fail to resolve and continues", func() { + // Create the NVMe device files + err := fs.WriteFileString("/dev/nvme0n1", "") + Expect(err).ToNot(HaveOccurred()) + err = fs.WriteFileString("/dev/nvme1n1", "") + Expect(err).ToNot(HaveOccurred()) + err = fs.WriteFileString("/dev/nvme2n1", "") + Expect(err).ToNot(HaveOccurred()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Globbing EBS symlinks")) - }) + // Create symlink directory + err = fs.MkdirAll("/dev/disk/by-id", os.FileMode(0750)) + Expect(err).ToNot(HaveOccurred()) - It("skips symlinks that fail to resolve and continues", func() { - // Configure instance storage resolver to return nvme1n1 and nvme2n1 - // (simulating that broken symlinks are skipped and only nvme0n1 is EBS) - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - return []string{"/dev/nvme1n1", "/dev/nvme2n1"}, nil - } + // Set up NVMe devices: nvme0n1 (EBS), nvme1n1 and nvme2n1 (instance storage) + fs.SetGlob("/dev/nvme*n1", []string{"/dev/nvme0n1", "/dev/nvme1n1", "/dev/nvme2n1"}) + 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_broken", // broken symlink + }) + err = fs.Symlink("/dev/nvme0n1", "/dev/disk/by-id/nvme-Amazon_Elastic_Block_Store_vol123") + Expect(err).ToNot(HaveOccurred()) + // Note: nvme-Amazon_Elastic_Block_Store_broken has no symlink target - it will be skipped // Mock parted for nvme1n1 and nvme2n1 (instance storage) cmdRunner.AddCmdResult("parted -s /dev/nvme1n1 p", fakesys.FakeCmdResult{ @@ -1975,7 +1974,7 @@ Number Start End Size File system Name Flags Stdout: "Error: /dev/nvme2n1: unrecognised disk label", }) - err := platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme0n1"}, {Path: "/dev/nvme1n1"}}) + err = platform.SetupRawEphemeralDisks([]boshsettings.DiskSettings{{Path: "/dev/nvme0n1"}, {Path: "/dev/nvme1n1"}}) Expect(err).ToNot(HaveOccurred()) // Should partition nvme1n1 and nvme2n1 (only nvme0n1 was identified as EBS) @@ -1983,15 +1982,8 @@ Number Start End Size File system Name Flags }) It("uses CPI paths directly for non-NVMe devices", func() { - // For non-NVMe devices, the instance storage resolver returns paths as-is - // (this is the default stub behavior, but be explicit) - instanceStorageResolver.DiscoverInstanceStorageStub = func(devices []boshsettings.DiskSettings) ([]string, error) { - paths := make([]string, len(devices)) - for i, d := range devices { - paths[i] = d.Path - } - return paths, nil - } + // For non-NVMe devices, the device path resolver is used directly + devicePathResolver.RealDevicePath = "/dev/xvdb" cmdRunner.AddCmdResult("parted -s /dev/xvdb p", fakesys.FakeCmdResult{ Error: errors.New("unrecognised disk label"), @@ -3888,7 +3880,7 @@ from-device-path dm-0 NETAPP ,LUN C-Mode certManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, state, options, logger, diff --git a/platform/provider.go b/platform/provider.go index aa1fe1363..2c22ffba8 100644 --- a/platform/provider.go +++ b/platform/provider.go @@ -158,15 +158,8 @@ func NewProvider(logger boshlog.Logger, dirProvider boshdirs.Provider, statsColl devicePathResolver = devicepathresolver.NewFallbackDevicePathResolver(symlinkLunResolver, devicePathResolver, logger) } - // Use auto-detecting instance storage resolver that determines NVMe vs non-NVMe - // based on device paths from the CPI (e.g., /dev/nvme* vs /dev/xvd* or /dev/sd*) - instanceStorageResolver := devicepathresolver.NewAutoDetectingInstanceStorageResolver( - fs, - devicePathResolver, - logger, - options.Linux.InstanceStorageManagedVolumePattern, - options.Linux.InstanceStorageDevicePattern, - ) + // Symlink device resolver for NVMe instance storage discovery (filtering out EBS/managed disks) + symlinkDeviceResolver := devicepathresolver.NewSymlinkDeviceResolver(fs, logger) uuidGenerator := boshuuid.NewGenerator() logsTarProvider := boshlogstarprovider.NewLogsTarProvider(compressor, copier, dirProvider) @@ -186,7 +179,7 @@ func NewProvider(logger boshlog.Logger, dirProvider boshdirs.Provider, statsColl centosCertManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, bootstrapState, options.Linux, logger, @@ -213,7 +206,7 @@ func NewProvider(logger boshlog.Logger, dirProvider boshdirs.Provider, statsColl ubuntuCertManager, monitRetryStrategy, devicePathResolver, - instanceStorageResolver, + symlinkDeviceResolver, bootstrapState, options.Linux, logger,