From a4504f2ad6c75887c1f3780a0df3bf3d84540091 Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Tue, 17 Mar 2026 11:08:24 +0100 Subject: [PATCH 1/4] rebased branch Signed-off-by: Valentin Daviot --- .../discoveredphysicaldisk_controller.go | 2 + internal/controller/discovery_ticker_test.go | 6 +- pkg/domain/logicalvolume.go | 29 +++++ pkg/infrastructure/di/container.go | 5 + .../di/logical_volume_discoverer.go | 53 ++++++++ pkg/infrastructure/di/usecase.go | 11 +- .../logicalvolumediscoverer/megaraid.go | 65 ++++++++++ .../logicalvolumediscoverer/smartarray.go | 65 ++++++++++ pkg/service/logical_volume_discoverer.go | 26 ++++ pkg/usecase/discover_physical_drives.go | 116 +++++++++++++++--- pkg/usecase/discover_physical_drives_test.go | 9 +- 11 files changed, 366 insertions(+), 21 deletions(-) create mode 100644 pkg/domain/logicalvolume.go create mode 100644 pkg/infrastructure/di/logical_volume_discoverer.go create mode 100644 pkg/infrastructure/logicalvolumediscoverer/megaraid.go create mode 100644 pkg/infrastructure/logicalvolumediscoverer/smartarray.go create mode 100644 pkg/service/logical_volume_discoverer.go diff --git a/internal/controller/discoveredphysicaldisk_controller.go b/internal/controller/discoveredphysicaldisk_controller.go index ee31c83..3766183 100644 --- a/internal/controller/discoveredphysicaldisk_controller.go +++ b/internal/controller/discoveredphysicaldisk_controller.go @@ -101,6 +101,8 @@ func mapDriveToStatus(status *metalk8sv1alpha1.DiscoveredPhysicalDiskStatus, res status.JBOD = &drive.JBOD status.Status = ptr(mapPDStatus(drive.Status)) status.Reason = &drive.Reason + status.DevicePath = &drive.DevicePath + status.PermanentPath = &drive.PermanentPath } func mapPDStatus(status physicaldrive.PDStatus) string { diff --git a/internal/controller/discovery_ticker_test.go b/internal/controller/discovery_ticker_test.go index bf3fc59..4713da2 100644 --- a/internal/controller/discovery_ticker_test.go +++ b/internal/controller/discovery_ticker_test.go @@ -46,7 +46,7 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: 100 * time.Millisecond, EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node", "default"), + UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, nil, &noopCacheWriter{}, "test-node", "default"), } done := make(chan error, 1) @@ -70,7 +70,7 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: 50 * time.Millisecond, EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node", "default"), + UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, nil, &noopCacheWriter{}, "test-node", "default"), } tickCount := 0 @@ -110,7 +110,7 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: time.Hour, // Long interval; should not matter. EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node", "default"), + UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, nil, &noopCacheWriter{}, "test-node", "default"), } done := make(chan error, 1) diff --git a/pkg/domain/logicalvolume.go b/pkg/domain/logicalvolume.go new file mode 100644 index 0000000..de28930 --- /dev/null +++ b/pkg/domain/logicalvolume.go @@ -0,0 +1,29 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package domain + +import ( + "github.com/scality/raidmgmt/pkg/domain/entities/logicalvolume" +) + +// DiscoveredLogicalVolume extends the raidmgmt LogicalVolume with RAID +// controller type context, mirroring the pattern used by DiscoveredPhysicalDrive. +type DiscoveredLogicalVolume struct { + ControllerType string + ControllerID int + *logicalvolume.LogicalVolume +} diff --git a/pkg/infrastructure/di/container.go b/pkg/infrastructure/di/container.go index edc394b..447152a 100644 --- a/pkg/infrastructure/di/container.go +++ b/pkg/infrastructure/di/container.go @@ -25,6 +25,7 @@ import ( "disk-management-agent/pkg/infrastructure/discovereddrivecache" "disk-management-agent/pkg/infrastructure/discoveredphysicaldiskstore" + "disk-management-agent/pkg/infrastructure/logicalvolumediscoverer" "disk-management-agent/pkg/infrastructure/physicaldrivediscoverer" "disk-management-agent/pkg/usecase" ) @@ -48,6 +49,10 @@ type Container struct { megaraidStorcliDiscoverer *physicaldrivediscoverer.MegaRAID smartArrayDiscoverer *physicaldrivediscoverer.SmartArray + megaraidPerccliLVDiscoverer *logicalvolumediscoverer.MegaRAID + megaraidStorcliLVDiscoverer *logicalvolumediscoverer.MegaRAID + smartArrayLVDiscoverer *logicalvolumediscoverer.SmartArray + discoveredPhysicalDiskStore *discoveredphysicaldiskstore.Kubernetes discoveredDriveCache *discovereddrivecache.InMemory diff --git a/pkg/infrastructure/di/logical_volume_discoverer.go b/pkg/infrastructure/di/logical_volume_discoverer.go new file mode 100644 index 0000000..a3cd1cb --- /dev/null +++ b/pkg/infrastructure/di/logical_volume_discoverer.go @@ -0,0 +1,53 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package di + +import ( + "github.com/scality/raidmgmt/pkg/core" + + "disk-management-agent/pkg/infrastructure/logicalvolumediscoverer" +) + +func (c *Container) getMegaRAIDPerccliLVDiscoverer() *logicalvolumediscoverer.MegaRAID { + if c.megaraidPerccliLVDiscoverer == nil { + c.megaraidPerccliLVDiscoverer = logicalvolumediscoverer.NewMegaRAID( + core.NewRAIDController(c.getMegaRAIDPerccliRAIDController()), + ) + } + + return c.megaraidPerccliLVDiscoverer +} + +func (c *Container) getMegaRAIDStorcliLVDiscoverer() *logicalvolumediscoverer.MegaRAID { + if c.megaraidStorcliLVDiscoverer == nil { + c.megaraidStorcliLVDiscoverer = logicalvolumediscoverer.NewMegaRAID( + core.NewRAIDController(c.getMegaRAIDStorcliRAIDController()), + ) + } + + return c.megaraidStorcliLVDiscoverer +} + +func (c *Container) getSmartArrayLVDiscoverer() *logicalvolumediscoverer.SmartArray { + if c.smartArrayLVDiscoverer == nil { + c.smartArrayLVDiscoverer = logicalvolumediscoverer.NewSmartArray( + core.NewRAIDController(c.getSmartArrayRAIDController()), + ) + } + + return c.smartArrayLVDiscoverer +} diff --git a/pkg/infrastructure/di/usecase.go b/pkg/infrastructure/di/usecase.go index 5b59218..b05c341 100644 --- a/pkg/infrastructure/di/usecase.go +++ b/pkg/infrastructure/di/usecase.go @@ -42,15 +42,22 @@ func (c *Container) getDiscoveredDriveCache() *discovereddrivecache.InMemory { // GetDiscoverPhysicalDrivesUseCase returns the singleton use case instance. func (c *Container) GetDiscoverPhysicalDrivesUseCase() *usecase.DiscoverPhysicalDrives { if c.discoverPhysicalDrivesUseCase == nil { - discoverers := []service.PhysicalDriveDiscoverer{ + pdDiscoverers := []service.PhysicalDriveDiscoverer{ c.getMegaRAIDPerccliDiscoverer(), c.getMegaRAIDStorcliDiscoverer(), c.getSmartArrayDiscoverer(), } + lvDiscoverers := []service.LogicalVolumeDiscoverer{ + c.getMegaRAIDPerccliLVDiscoverer(), + c.getMegaRAIDStorcliLVDiscoverer(), + c.getSmartArrayLVDiscoverer(), + } + c.discoverPhysicalDrivesUseCase = usecase.NewDiscoverPhysicalDrives( c.logger, - discoverers, + pdDiscoverers, + lvDiscoverers, c.getDiscoveredPhysicalDiskStore(), c.getDiscoveredDriveCache(), c.nodeName, diff --git a/pkg/infrastructure/logicalvolumediscoverer/megaraid.go b/pkg/infrastructure/logicalvolumediscoverer/megaraid.go new file mode 100644 index 0000000..d124b80 --- /dev/null +++ b/pkg/infrastructure/logicalvolumediscoverer/megaraid.go @@ -0,0 +1,65 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package logicalvolumediscoverer + +import ( + "github.com/pkg/errors" + "github.com/scality/raidmgmt/pkg/domain/ports" + + "disk-management-agent/pkg/domain" + "disk-management-agent/pkg/service" +) + +const megaraidControllerType = "MegaRAID" + +// MegaRAID discovers logical volumes behind MegaRAID/PERC controllers +// using storcli or perccli. +type MegaRAID struct { + rc ports.RAIDController +} + +var _ service.LogicalVolumeDiscoverer = &MegaRAID{} + +func NewMegaRAID(rc ports.RAIDController) *MegaRAID { + return &MegaRAID{rc: rc} +} + +func (d *MegaRAID) DiscoverLogicalVolumes() ([]*domain.DiscoveredLogicalVolume, error) { + controllers, err := d.rc.Controllers() + if err != nil { + return nil, errors.Wrap(err, "failed to list MegaRAID controllers") + } + + var volumes []*domain.DiscoveredLogicalVolume + + for _, ctrl := range controllers { + lvs, err := d.rc.LogicalVolumes(ctrl.Metadata) + if err != nil { + return nil, errors.Wrapf(err, "failed to list logical volumes for MegaRAID controller %d", ctrl.Metadata.ID) + } + + for _, lv := range lvs { + volumes = append(volumes, &domain.DiscoveredLogicalVolume{ + ControllerType: megaraidControllerType, + ControllerID: ctrl.Metadata.ID, + LogicalVolume: lv, + }) + } + } + + return volumes, nil +} diff --git a/pkg/infrastructure/logicalvolumediscoverer/smartarray.go b/pkg/infrastructure/logicalvolumediscoverer/smartarray.go new file mode 100644 index 0000000..9b5a2b6 --- /dev/null +++ b/pkg/infrastructure/logicalvolumediscoverer/smartarray.go @@ -0,0 +1,65 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package logicalvolumediscoverer + +import ( + "github.com/pkg/errors" + "github.com/scality/raidmgmt/pkg/domain/ports" + + "disk-management-agent/pkg/domain" + "disk-management-agent/pkg/service" +) + +const smartArrayControllerType = "SmartArray" + +// SmartArray discovers logical volumes behind HPE Smart Array controllers +// using ssacli. +type SmartArray struct { + rc ports.RAIDController +} + +var _ service.LogicalVolumeDiscoverer = &SmartArray{} + +func NewSmartArray(rc ports.RAIDController) *SmartArray { + return &SmartArray{rc: rc} +} + +func (d *SmartArray) DiscoverLogicalVolumes() ([]*domain.DiscoveredLogicalVolume, error) { + controllers, err := d.rc.Controllers() + if err != nil { + return nil, errors.Wrap(err, "failed to list SmartArray controllers") + } + + var volumes []*domain.DiscoveredLogicalVolume + + for _, ctrl := range controllers { + lvs, err := d.rc.LogicalVolumes(ctrl.Metadata) + if err != nil { + return nil, errors.Wrapf(err, "failed to list logical volumes for SmartArray controller %d", ctrl.Metadata.ID) + } + + for _, lv := range lvs { + volumes = append(volumes, &domain.DiscoveredLogicalVolume{ + ControllerType: smartArrayControllerType, + ControllerID: ctrl.Metadata.ID, + LogicalVolume: lv, + }) + } + } + + return volumes, nil +} diff --git a/pkg/service/logical_volume_discoverer.go b/pkg/service/logical_volume_discoverer.go new file mode 100644 index 0000000..a1e8f8a --- /dev/null +++ b/pkg/service/logical_volume_discoverer.go @@ -0,0 +1,26 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import "disk-management-agent/pkg/domain" + +// LogicalVolumeDiscoverer discovers logical volumes from RAID controllers. +// Each implementation corresponds to a specific RAID controller adapter +// (MegaRAID, SmartArray). +type LogicalVolumeDiscoverer interface { + DiscoverLogicalVolumes() ([]*domain.DiscoveredLogicalVolume, error) +} diff --git a/pkg/usecase/discover_physical_drives.go b/pkg/usecase/discover_physical_drives.go index c18ee79..bb29c16 100644 --- a/pkg/usecase/discover_physical_drives.go +++ b/pkg/usecase/discover_physical_drives.go @@ -34,29 +34,33 @@ import ( // It also populates the drive cache so that the reconciler can read the // latest discovered state. type DiscoverPhysicalDrives struct { - logger logr.Logger - discoverers []service.PhysicalDriveDiscoverer - store service.DiscoveredPhysicalDiskStore - cacheWriter service.DiscoveredDriveCacheWriter - nodeName string - namespace string + logger logr.Logger + pdDiscoverers []service.PhysicalDriveDiscoverer + lvDiscoverers []service.LogicalVolumeDiscoverer + store service.DiscoveredPhysicalDiskStore + cacheWriter service.DiscoveredDriveCacheWriter + nodeName string + namespace string } // NewDiscoverPhysicalDrives creates a new DiscoverPhysicalDrives use case. func NewDiscoverPhysicalDrives( logger logr.Logger, - discoverers []service.PhysicalDriveDiscoverer, + pdDiscoverers []service.PhysicalDriveDiscoverer, + lvDiscoverers []service.LogicalVolumeDiscoverer, store service.DiscoveredPhysicalDiskStore, cacheWriter service.DiscoveredDriveCacheWriter, - nodeName, namespace string, + nodeName string, + namespace string, ) *DiscoverPhysicalDrives { return &DiscoverPhysicalDrives{ - logger: logger.WithName("discover-physical-drives"), - discoverers: discoverers, - store: store, - cacheWriter: cacheWriter, - nodeName: nodeName, - namespace: namespace, + logger: logger.WithName("discover-physical-drives"), + pdDiscoverers: pdDiscoverers, + lvDiscoverers: lvDiscoverers, + store: store, + cacheWriter: cacheWriter, + nodeName: nodeName, + namespace: namespace, } } @@ -68,6 +72,9 @@ func NewDiscoverPhysicalDrives( // watch. func (u *DiscoverPhysicalDrives) Execute(ctx context.Context) ([]string, error) { allDrives := u.gatherDrives() + allLVs := u.gatherLogicalVolumes() + + u.enrichDrivePaths(allDrives, allLVs) drivesByName := u.buildCacheMap(allDrives) u.cacheWriter.Replace(drivesByName) @@ -141,7 +148,11 @@ func (u *DiscoverPhysicalDrives) buildCacheMap( func (u *DiscoverPhysicalDrives) gatherDrives() []*domain.DiscoveredPhysicalDrive { var allDrives []*domain.DiscoveredPhysicalDrive - for _, discoverer := range u.discoverers { + for _, discoverer := range u.pdDiscoverers { + if discoverer == nil { + continue + } + drives, err := discoverer.DiscoverPhysicalDrives() if err != nil { u.logger.V(1).Info("Discoverer returned an error, skipping", "error", err) @@ -155,6 +166,81 @@ func (u *DiscoverPhysicalDrives) gatherDrives() []*domain.DiscoveredPhysicalDriv return allDrives } +// gatherLogicalVolumes collects logical volumes from all registered LV +// discoverers. The same error-skipping policy as gatherDrives applies. +func (u *DiscoverPhysicalDrives) gatherLogicalVolumes() []*domain.DiscoveredLogicalVolume { + var allLVs []*domain.DiscoveredLogicalVolume + + for _, discoverer := range u.lvDiscoverers { + lvs, err := discoverer.DiscoverLogicalVolumes() + if err != nil { + u.logger.V(1).Info("LV discoverer returned an error, skipping", "error", err) + + continue + } + + allLVs = append(allLVs, lvs...) + } + + return allLVs +} + +// enrichDrivePaths populates DevicePath and PermanentPath on physical +// drives that don't already have them by finding the RAID logical volume +// that contains each drive and copying the LV's paths. +func (u *DiscoverPhysicalDrives) enrichDrivePaths( + drives []*domain.DiscoveredPhysicalDrive, + lvs []*domain.DiscoveredLogicalVolume, +) { + for _, drive := range drives { + if drive.DevicePath != "" && drive.PermanentPath != "" { + continue + } + + lv := findMatchingLogicalVolume(drive, lvs) + if lv == nil { + u.logger.V(1).Info( + "No matching logical volume found for drive, paths will be empty", + "controllerType", drive.ControllerType, + "controllerID", drive.ControllerID, + "driveID", drive.ID, + ) + + continue + } + + if drive.DevicePath == "" { + drive.DevicePath = lv.DevicePath + } + + if drive.PermanentPath == "" { + drive.PermanentPath = lv.PermanentPath + } + } +} + +// findMatchingLogicalVolume returns the logical volume that contains the +// given physical drive, or nil if none match. Matching is done on +// controller type, controller ID and physical drive ID. +func findMatchingLogicalVolume( + drive *domain.DiscoveredPhysicalDrive, + lvs []*domain.DiscoveredLogicalVolume, +) *domain.DiscoveredLogicalVolume { + for _, lv := range lvs { + if lv.ControllerType != drive.ControllerType || lv.ControllerID != drive.ControllerID { + continue + } + + for _, pdMeta := range lv.PDrivesMetadata { + if pdMeta.ID == drive.ID { + return lv + } + } + } + + return nil +} + func buildCR( name, namespace, nodeName string, drive *domain.DiscoveredPhysicalDrive, diff --git a/pkg/usecase/discover_physical_drives_test.go b/pkg/usecase/discover_physical_drives_test.go index ab39e30..0847eab 100644 --- a/pkg/usecase/discover_physical_drives_test.go +++ b/pkg/usecase/discover_physical_drives_test.go @@ -132,7 +132,7 @@ func newTestSSD(ctrlType string, ctrlID int, slotID string) *domain.DiscoveredPh func TestExecute_NoDiscoverers(t *testing.T) { store := &mockStore{} cache := &mockCacheWriter{} - uc := NewDiscoverPhysicalDrives(logr.Discard(), nil, store, cache, "node-1", "default") + uc := NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, store, cache, "node-1", "default") existing, err := uc.Execute(context.Background()) @@ -156,6 +156,7 @@ func TestExecute_SSDDrivesOnly(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, + nil, store, cache, "node-1", @@ -184,6 +185,7 @@ func TestExecute_MixOfHDDAndSSD(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, + nil, store, cache, "node-1", @@ -215,6 +217,7 @@ func TestExecute_ExistingCR(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, + nil, store, cache, "node-1", @@ -235,6 +238,7 @@ func TestExecute_DiscovererError(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{failingDiscoverer}, + nil, store, cache, "node-1", @@ -257,6 +261,7 @@ func TestExecute_StoreGetError(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, + nil, store, cache, "node-1", @@ -279,6 +284,7 @@ func TestExecute_StoreCreateError(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, + nil, store, cache, "node-1", @@ -304,6 +310,7 @@ func TestExecute_MultipleDiscoverers(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{megaraidDiscoverer, smartArrayDiscoverer}, + nil, store, cache, "node-1", From 8566dfd5cff46a456e98adf30a615713b2431699 Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Fri, 20 Mar 2026 11:26:27 +0100 Subject: [PATCH 2/4] rebasing Signed-off-by: Valentin Daviot --- internal/controller/discovery_ticker_test.go | 27 +++++++++++++++++--- pkg/usecase/discover_physical_drives.go | 22 ++++++++-------- pkg/usecase/discover_physical_drives_test.go | 6 ++--- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/internal/controller/discovery_ticker_test.go b/internal/controller/discovery_ticker_test.go index 14b1198..8c7742a 100644 --- a/internal/controller/discovery_ticker_test.go +++ b/internal/controller/discovery_ticker_test.go @@ -46,7 +46,14 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: 100 * time.Millisecond, EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node"), + UseCase: usecase.NewDiscoverPhysicalDrives( + logr.Discard(), + []service.PhysicalDriveDiscoverer{}, + []service.LogicalVolumeDiscoverer{}, + nil, + &noopCacheWriter{}, + "test-node", + ), } done := make(chan error, 1) @@ -70,7 +77,14 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: 50 * time.Millisecond, EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node"), + UseCase: usecase.NewDiscoverPhysicalDrives( + logr.Discard(), + []service.PhysicalDriveDiscoverer{}, + []service.LogicalVolumeDiscoverer{}, + nil, + &noopCacheWriter{}, + "test-node", + ), } tickCount := 0 @@ -110,7 +124,14 @@ var _ = Describe("DiscoveryTicker", func() { NodeName: "test-node", Interval: time.Hour, // Long interval; should not matter. EventChan: eventChan, - UseCase: usecase.NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, &noopCacheWriter{}, "test-node"), + UseCase: usecase.NewDiscoverPhysicalDrives( + logr.Discard(), + []service.PhysicalDriveDiscoverer{}, + []service.LogicalVolumeDiscoverer{}, + nil, + &noopCacheWriter{}, + "test-node", + ), } done := make(chan error, 1) diff --git a/pkg/usecase/discover_physical_drives.go b/pkg/usecase/discover_physical_drives.go index 76e8a4f..aa56d51 100644 --- a/pkg/usecase/discover_physical_drives.go +++ b/pkg/usecase/discover_physical_drives.go @@ -34,11 +34,12 @@ import ( // It also populates the drive cache so that the reconciler can read the // latest discovered state. type DiscoverPhysicalDrives struct { - logger logr.Logger - discoverers []service.PhysicalDriveDiscoverer - store service.DiscoveredPhysicalDiskStore - cacheWriter service.DiscoveredDriveCacheWriter - nodeName string + logger logr.Logger + pdDiscoverers []service.PhysicalDriveDiscoverer + lvDiscoverers []service.LogicalVolumeDiscoverer + store service.DiscoveredPhysicalDiskStore + cacheWriter service.DiscoveredDriveCacheWriter + nodeName string } // NewDiscoverPhysicalDrives creates a new DiscoverPhysicalDrives use case. @@ -51,11 +52,12 @@ func NewDiscoverPhysicalDrives( nodeName string, ) *DiscoverPhysicalDrives { return &DiscoverPhysicalDrives{ - logger: logger.WithName("discover-physical-drives"), - discoverers: discoverers, - store: store, - cacheWriter: cacheWriter, - nodeName: nodeName, + logger: logger.WithName("discover-physical-drives"), + pdDiscoverers: pdDiscoverers, + lvDiscoverers: lvDiscoverers, + store: store, + cacheWriter: cacheWriter, + nodeName: nodeName, } } diff --git a/pkg/usecase/discover_physical_drives_test.go b/pkg/usecase/discover_physical_drives_test.go index aad3f54..dabf93d 100644 --- a/pkg/usecase/discover_physical_drives_test.go +++ b/pkg/usecase/discover_physical_drives_test.go @@ -132,7 +132,7 @@ func newTestSSD(ctrlType string, ctrlID int, slotID string) *domain.DiscoveredPh func TestExecute_NoDiscoverers(t *testing.T) { store := &mockStore{} cacheWriter := &mockCacheWriter{} - uc := NewDiscoverPhysicalDrives(logr.Discard(), nil, store, cacheWriter, "node-1") + uc := NewDiscoverPhysicalDrives(logr.Discard(), nil, nil, store, cacheWriter, "node-1") existing, err := uc.Execute(context.Background()) @@ -156,7 +156,7 @@ func TestExecute_SSDDrivesOnly(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, - nil, + []service.LogicalVolumeDiscoverer{}, store, cache, "node-1", @@ -184,7 +184,7 @@ func TestExecute_MixOfHDDAndSSD(t *testing.T) { uc := NewDiscoverPhysicalDrives( logr.Discard(), []service.PhysicalDriveDiscoverer{discoverer}, - nil, + []service.LogicalVolumeDiscoverer{}, store, cache, "node-1", From a4bd5aafcec1f43b44399036df59a48b10692ad8 Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Fri, 20 Mar 2026 11:39:40 +0100 Subject: [PATCH 3/4] lint Signed-off-by: Valentin Daviot --- pkg/infrastructure/logicalvolumediscoverer/megaraid.go | 5 +++-- pkg/infrastructure/logicalvolumediscoverer/smartarray.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/infrastructure/logicalvolumediscoverer/megaraid.go b/pkg/infrastructure/logicalvolumediscoverer/megaraid.go index d124b80..f00c873 100644 --- a/pkg/infrastructure/logicalvolumediscoverer/megaraid.go +++ b/pkg/infrastructure/logicalvolumediscoverer/megaraid.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +//nolint:dupl // This function is similar to the one in the SmartArray implementation. package logicalvolumediscoverer import ( @@ -49,13 +50,13 @@ func (d *MegaRAID) DiscoverLogicalVolumes() ([]*domain.DiscoveredLogicalVolume, for _, ctrl := range controllers { lvs, err := d.rc.LogicalVolumes(ctrl.Metadata) if err != nil { - return nil, errors.Wrapf(err, "failed to list logical volumes for MegaRAID controller %d", ctrl.Metadata.ID) + return nil, errors.Wrapf(err, "failed to list logical volumes for MegaRAID controller %d", ctrl.ID) } for _, lv := range lvs { volumes = append(volumes, &domain.DiscoveredLogicalVolume{ ControllerType: megaraidControllerType, - ControllerID: ctrl.Metadata.ID, + ControllerID: ctrl.ID, LogicalVolume: lv, }) } diff --git a/pkg/infrastructure/logicalvolumediscoverer/smartarray.go b/pkg/infrastructure/logicalvolumediscoverer/smartarray.go index 9b5a2b6..4bc3d0a 100644 --- a/pkg/infrastructure/logicalvolumediscoverer/smartarray.go +++ b/pkg/infrastructure/logicalvolumediscoverer/smartarray.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +//nolint:dupl // This function is similar to the one in the MegaRAID implementation. package logicalvolumediscoverer import ( @@ -49,13 +50,13 @@ func (d *SmartArray) DiscoverLogicalVolumes() ([]*domain.DiscoveredLogicalVolume for _, ctrl := range controllers { lvs, err := d.rc.LogicalVolumes(ctrl.Metadata) if err != nil { - return nil, errors.Wrapf(err, "failed to list logical volumes for SmartArray controller %d", ctrl.Metadata.ID) + return nil, errors.Wrapf(err, "failed to list logical volumes for SmartArray controller %d", ctrl.ID) } for _, lv := range lvs { volumes = append(volumes, &domain.DiscoveredLogicalVolume{ ControllerType: smartArrayControllerType, - ControllerID: ctrl.Metadata.ID, + ControllerID: ctrl.ID, LogicalVolume: lv, }) } From eeb9deaf4eeef0263bd9009436c674202af628f6 Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Fri, 20 Mar 2026 14:30:08 +0100 Subject: [PATCH 4/4] pouet Signed-off-by: Valentin Daviot --- pkg/usecase/discover_physical_drives.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/usecase/discover_physical_drives.go b/pkg/usecase/discover_physical_drives.go index aa56d51..c7d61a1 100644 --- a/pkg/usecase/discover_physical_drives.go +++ b/pkg/usecase/discover_physical_drives.go @@ -169,6 +169,10 @@ func (u *DiscoverPhysicalDrives) gatherLogicalVolumes() []*domain.DiscoveredLogi var allLVs []*domain.DiscoveredLogicalVolume for _, discoverer := range u.lvDiscoverers { + if discoverer == nil { + continue + } + lvs, err := discoverer.DiscoverLogicalVolumes() if err != nil { u.logger.V(1).Info("LV discoverer returned an error, skipping", "error", err)