diff --git a/pkg/implementation/logicalvolumegetter/mdadm.go b/pkg/implementation/logicalvolumegetter/mdadm.go index a3ec331..29c3430 100644 --- a/pkg/implementation/logicalvolumegetter/mdadm.go +++ b/pkg/implementation/logicalvolumegetter/mdadm.go @@ -295,9 +295,10 @@ func ParseMDADMExportOutput(output []byte) ([]*MDADMExportDetails, error) { device = currentDetails.Devices[deviceName] } - if matches[2] == "ROLE" { + switch matches[2] { + case "ROLE": device.Role = matches[3] - } else if matches[2] == "DEV" { + case "DEV": device.Path = matches[3] } diff --git a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go index a9e3e8b..bc7a1ac 100644 --- a/pkg/implementation/raidcontroller/megaraid/logicalvolume.go +++ b/pkg/implementation/raidcontroller/megaraid/logicalvolume.go @@ -166,7 +166,7 @@ func selectorLV(m *logicalvolume.Metadata) (string, error) { // logicalVolume returns a logical volume for a given logical volume metadata. // -//nolint:funlen // The function is not too complex +//nolint:funlen,cyclop // The function is not too complex func (a *Adapter) logicalVolume( metadata *logicalvolume.Metadata) ( *logicalvolume.LogicalVolume, error, @@ -224,14 +224,21 @@ func (a *Adapter) logicalVolume( return nil, errors.Wrap(err, "failed to unmarshal VD properties") } - permanentPath, err := vdProperties.permanentPath() + // Get the physical drives from the metadata + // NOTE: This is needed as fallback to get the device path + pds, err := a.fillPhysicalDrives(pdsMetadata) + if err != nil { + return nil, errors.Wrap(err, "failed to fill physical drives") + } + + devicePath, permanentPath, err := getPaths(vdProperties, pds) if err != nil { - return nil, errors.Wrap(err, "failed to get permanent path") + return nil, errors.Wrap(err, "failed to get paths") } logicalVolume := &logicalvolume.LogicalVolume{ Metadata: metadata, - DevicePath: vdProperties.OSDriveName, + DevicePath: devicePath, RAIDLevel: logicalvolume.RAIDLevelMap(vd.Type), PDrivesMetadata: pdsMetadata, CacheOptions: cacheOptions, @@ -542,19 +549,43 @@ func hasMatchingPDs(lvPDs []*physicaldrive.Metadata, pdSlots map[string]struct{} return false } -// CustomEvalSymlinks is a variable that holds a function that evaluates symlinks. -// It is used to mock the filepath.EvalSymlinks function in tests. +// It is used to mock the functions in tests. // nolint: gochecknoglobals // This is a variable that is used to mock a function in tests. -var CustomEvalSymlinks = filepath.EvalSymlinks +var ( + CustomEvalSymlinks = filepath.EvalSymlinks + CustomFileExists = utils.FileExists +) + +// getPaths returns the device path and a permanent paths for the logical volumes. +func getPaths(vdp *VDProperties, pdrives []*physicaldrive.PhysicalDrive) ( + devicePath, permanentPath string, err error, +) { + devicePath = vdp.OSDriveName + + permanentPath = fmt.Sprintf("/dev/disk/by-id/wwn-0x%s", vdp.SCSINAAID) + if !CustomFileExists(permanentPath) { + // If the permanent path is not found and there is only one physical drive, + // we will try to get the path from the physical drive information + // otherwise let's error here + if len(pdrives) != 1 { + return devicePath, "", errors.New("failed to get permanent path") + } -// permanentPath returns the permanent path of a virtual drive. -func (vdp *VDProperties) permanentPath() (string, error) { - sysPath := fmt.Sprintf("/dev/disk/by-id/wwn-0x%s", vdp.SCSINAAID) + pd := pdrives[0] - permanentPath, err := CustomEvalSymlinks(sysPath) - if err != nil { - return "", errors.Wrap(err, "failed to evaluate symlink") + permanentPath = computePermanentPath(pd) + if !CustomFileExists(permanentPath) { + return devicePath, "", errors.New("failed to get permanent path from physical drive") + } + } + + // If the devicePath is empty let's retrieve it from the permanent path + if devicePath == "" { + devicePath, err = CustomEvalSymlinks(permanentPath) + if err != nil { + return "", "", errors.Wrap(err, "failed to evaluate symlink") + } } - return permanentPath, nil + return devicePath, permanentPath, nil } diff --git a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go index f6801b9..ec39ce4 100644 --- a/pkg/implementation/raidcontroller/megaraid/megaraid_test.go +++ b/pkg/implementation/raidcontroller/megaraid/megaraid_test.go @@ -30,6 +30,9 @@ type UnitTestSuite struct { wasCreateLVCalledOnce bool + // fileExistsFunc is the original FileExists function + // It's used to restore the original function after mocking it + fileExistsFunc func(string) bool // evalSymlinksFunc is the original EvalSymlinks function // It's used to restore the original function after mocking it evalSymlinksFunc func(string) (string, error) @@ -191,26 +194,19 @@ func (s *UnitTestSuite) generalMockCalls(args []string) (*megaraid2.CmdOutput, e return mockReturn(filename) } -// setupMockCallsEvalSymlinksList sets up the mock calls for EvalSymlinks. -func (s *UnitTestSuite) setupMockCallsEvalSymlinksList() { - ids := []string{ - "600062b212da5d402bd3b43351207e5f", - "600062b212da5d402bd3b47dc0433389", - "600062b212da5d402bd3b47fc3685d94", - "600062b212da5d402bd3b481c696852b", - "600062b212da5d402bd3b483c9c3e053", - "600062b212da5d402bd3b486ccf8622c", - "600062b212da5d402bd3b488d03a89ca", - "600062b212da5d402bd3b48ad38fb43e", - "600062b212da5d402bd3b48cd6db37db", - "600062b212da5d402bd3b48eda2832e2", - "600062b212da5d402bd3b491dd849969", - "600062b212da5d402bd3b493e1699377", - } - - for _, id := range ids { - s.mockPathResolver.On("EvalSymlinks", "/dev/disk/by-id/wwn-0x"+id).Return("/dev/sda", nil) - } +// restoreCustomFileExists restores the custom FileExists function +// to the original one. +// It's used to mock the FileExists function. +func (s *UnitTestSuite) restoreCustomFileExists() { + megaraid2.CustomFileExists = s.fileExistsFunc +} + +// setupCustomFileExists sets up the custom FileExists function +// to be used in the tests. +// It's used to mock the FileExists function. +func (s *UnitTestSuite) setupCustomFileExists() { + s.fileExistsFunc = megaraid2.CustomFileExists + megaraid2.CustomFileExists = s.mockPathResolver.FileExists } // restoreCustomEvalSymlinks restores the custom EvalSymlinks function @@ -393,7 +389,11 @@ func (s *UnitTestSuite) TestPhysicalDrive() { func (s *UnitTestSuite) TestLogicalVolumes() { s.setupMockCalls() - s.setupMockCallsEvalSymlinksList() + s.mockPathResolver.On("FileExists", mock.Anything).Return(true) + s.mockPathResolver.On("EvalSymlinks", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return("/dev/sda", nil) + + s.setupCustomFileExists() + defer s.restoreCustomFileExists() s.setupCustomEvalSymlinks() defer s.restoreCustomEvalSymlinks() @@ -450,8 +450,12 @@ func (s *UnitTestSuite) TestLogicalVolumes() { func (s *UnitTestSuite) TestLogicalVolume() { s.setupMockCalls() + s.mockPathResolver.On("FileExists", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return(true) s.mockPathResolver.On("EvalSymlinks", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return("/dev/sda", nil) + s.setupCustomFileExists() + defer s.restoreCustomFileExists() + s.setupCustomEvalSymlinks() defer s.restoreCustomEvalSymlinks() @@ -559,8 +563,12 @@ func (s *UnitTestSuite) TestDisableJBOD() { func (s *UnitTestSuite) TestSetLVCacheOptions() { s.setupMockCalls() + s.mockPathResolver.On("FileExists", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return(true) s.mockPathResolver.On("EvalSymlinks", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return("/dev/sda", nil) + s.setupCustomFileExists() + defer s.restoreCustomFileExists() + s.setupCustomEvalSymlinks() defer s.restoreCustomEvalSymlinks() @@ -614,7 +622,11 @@ func (s *UnitTestSuite) TestSetLVCacheOptions() { func (s *UnitTestSuite) TestCreateLV() { s.setupMockCallsCreateLV() - s.setupMockCallsEvalSymlinksList() + s.mockPathResolver.On("FileExists", mock.Anything).Return(true) + s.mockPathResolver.On("EvalSymlinks", "/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377").Return("/dev/sda", nil) + + s.setupCustomFileExists() + defer s.restoreCustomFileExists() s.setupCustomEvalSymlinks() defer s.restoreCustomEvalSymlinks() @@ -691,7 +703,7 @@ func (s *UnitTestSuite) TestCreateLV() { s.Equal(logicalvolume.ReadPolicyReadAhead, newLv.CacheOptions.ReadPolicy) s.Equal(logicalvolume.WritePolicyWriteThrough, newLv.CacheOptions.WritePolicy) s.Equal(logicalvolume.IOPolicyDirect, newLv.CacheOptions.IOPolicy) - s.Equal("/dev/sda", newLv.PermanentPath) + s.Equal("/dev/disk/by-id/wwn-0x600062b212da5d402bd3b493e1699377", newLv.PermanentPath) } } } diff --git a/pkg/implementation/raidcontroller/megaraid/mocks/PathResolver.go b/pkg/implementation/raidcontroller/megaraid/mocks/PathResolver.go index 2cbadfb..65891ff 100644 --- a/pkg/implementation/raidcontroller/megaraid/mocks/PathResolver.go +++ b/pkg/implementation/raidcontroller/megaraid/mocks/PathResolver.go @@ -1,8 +1,8 @@ -// Code generated by mockery v2.46.3. DO NOT EDIT. +// Code generated by mockery 2.53.3. DO NOT EDIT. package mocks -import "github.com/stretchr/testify/mock" +import mock "github.com/stretchr/testify/mock" // PathResolver is an autogenerated mock type for the PathResolver type type PathResolver struct { @@ -37,6 +37,24 @@ func (_m *PathResolver) EvalSymlinks(_a0 string) (string, error) { return r0, r1 } +// FileExists provides a mock function with given fields: _a0 +func (_m *PathResolver) FileExists(_a0 string) bool { + ret := _m.Called(_a0) + + if len(ret) == 0 { + panic("no return value specified for FileExists") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(_a0) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + // NewPathResolver creates a new instance of PathResolver. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewPathResolver(t interface { diff --git a/pkg/implementation/raidcontroller/megaraid/mocks/Runner.go b/pkg/implementation/raidcontroller/megaraid/mocks/Runner.go index 77bee6e..d133e8d 100644 --- a/pkg/implementation/raidcontroller/megaraid/mocks/Runner.go +++ b/pkg/implementation/raidcontroller/megaraid/mocks/Runner.go @@ -1,11 +1,10 @@ -// Code generated by mockery v2.46.3. DO NOT EDIT. +// Code generated by mockery 2.53.3. DO NOT EDIT. package mocks import ( - "github.com/stretchr/testify/mock" - - "github.com/scality/raidmgmt/pkg/implementation/raidcontroller/megaraid" + megaraid "github.com/scality/raidmgmt/pkg/implementation/raidcontroller/megaraid" + mock "github.com/stretchr/testify/mock" ) // Runner is an autogenerated mock type for the Runner type diff --git a/pkg/implementation/raidcontroller/megaraid/pathresolver.go b/pkg/implementation/raidcontroller/megaraid/pathresolver.go index f8fc149..170bcfa 100644 --- a/pkg/implementation/raidcontroller/megaraid/pathresolver.go +++ b/pkg/implementation/raidcontroller/megaraid/pathresolver.go @@ -2,8 +2,11 @@ package megaraid // PathResolver is an interface that defines the EvalSymlinks method. // +// FileExists checks if a file exists at the given path. +// It is used to mock the os package in tests. // EvalSymlinks returns the path name after the evaluation of any symbolic links. // It is used to mock the filepath.EvalSymlinks function in tests. type PathResolver interface { + FileExists(string) bool EvalSymlinks(string) (string, error) } diff --git a/pkg/implementation/raidcontroller/megaraid/physicaldrive.go b/pkg/implementation/raidcontroller/megaraid/physicaldrive.go index eb81825..a3e7a7c 100644 --- a/pkg/implementation/raidcontroller/megaraid/physicaldrive.go +++ b/pkg/implementation/raidcontroller/megaraid/physicaldrive.go @@ -2,6 +2,7 @@ package megaraid import ( "fmt" + "path/filepath" "sort" "strconv" "strings" @@ -66,7 +67,7 @@ func (a *Adapter) physicaldrives(metadata *raidcontroller.Metadata) ( // physicalDrive returns a physical drive for a given physical drive metadata. // -//nolint:funlen // no good reason to split it for now +//nolint:funlen,gocognit // no good reason to split it for now func (a *Adapter) physicalDrive( metadata *physicaldrive.Metadata) ( *physicaldrive.PhysicalDrive, error, @@ -123,11 +124,21 @@ func (a *Adapter) physicalDrive( Status: pd.PDStatus(), JBOD: jbod, // TODO : fill those fields - // PermanentPath: "", - // DevicePath: "", // Reason: "", } + if physicalDrive.JBOD { + physicalDrive.PermanentPath = computePermanentPath(physicalDrive) + if !utils.FileExists((physicalDrive.PermanentPath)) { + return nil, errors.New("permanent path does not exist") + } + + physicalDrive.DevicePath, err = filepath.EvalSymlinks(physicalDrive.PermanentPath) + if err != nil { + return nil, errors.Wrap(err, "failed to evaluate symlinks") + } + } + return physicalDrive, nil } @@ -247,3 +258,10 @@ func (a *Adapter) setJBOD( return nil } + +// computePermanentPath computes the permanent path for a given physical drive. +// The permanent path is a string that represents the physical drive +// this may not exists if the drive is not in JBOD. +func computePermanentPath(pd *physicaldrive.PhysicalDrive) string { + return fmt.Sprintf("/dev/disk/by-id/scsi-S%s_%s_%s", pd.Vendor, pd.Model, pd.Serial) +} diff --git a/pkg/implementation/raidcontroller/megaraid/structs.go b/pkg/implementation/raidcontroller/megaraid/structs.go index 2343b60..9bc9724 100644 --- a/pkg/implementation/raidcontroller/megaraid/structs.go +++ b/pkg/implementation/raidcontroller/megaraid/structs.go @@ -65,10 +65,12 @@ type ( // Physical Drive. PD struct { - EIDSlot string `json:"EID:Slt"` - DeviceID int `json:"DID"` - State string `json:"State"` - DeviceGroup int `json:"DG"` + EIDSlot string `json:"EID:Slt"` + DeviceID int `json:"DID"` + State string `json:"State"` + // DG can be either a "-" or a int, since we are + // not using it today let's just comment it out + // DeviceGroup int `json:"DG"` Size string `json:"Size"` // Size (humanized) Interface string `json:"Intf"` MediaType string `json:"Med"` // Media Type (HDD, SSD, NVMe) diff --git a/pkg/implementation/raidcontroller/megaraid/testdata/logicalvolumes/show/v228.json b/pkg/implementation/raidcontroller/megaraid/testdata/logicalvolumes/show/v228.json index 824f67e..353e7f2 100644 --- a/pkg/implementation/raidcontroller/megaraid/testdata/logicalvolumes/show/v228.json +++ b/pkg/implementation/raidcontroller/megaraid/testdata/logicalvolumes/show/v228.json @@ -1 +1 @@ -{"Controllers":[{"Command Status":{"CLI Version":"007.1616.0000.0000 Dec 24, 2020","Operating system":"Linux 4.18.0-553.22.1.el8_10.x86_64","Controller":0,"Status":"Success","Description":"None"},"Response Data":{"/c0/v228":[{"DG/VD":"11/228","TYPE":"RAID0","State":"Optl","Access":"RW","Consist":"Yes","Cache":"RWTD","Cac":"-","sCC":"ON","Size":"16.370 TB","Name":""}],"PDs for VD 228":[{"EID:Slt":"251:12","DID":1,"State":"Onln","DG":11,"Size":"16.370 TB","Intf":"SAS","Med":"HDD","SED":"N","PI":"N","SeSz":"512B","Model":"ST18000NM000D ","Sp":"U","Type":"-"}],"VD228 Properties":{"Strip Size":"256 KB","Number of Blocks":35155607552,"VD has Emulated PD":"Yes","Span Depth":1,"Number of Drives Per Span":1,"Write Cache(initial setting)":"WriteThrough","Disk Cache Policy":"Disk's Default","Encryption":"None","Data Protection":"None","Active Operations":"None","Exposed to OS":"Yes","OS Drive Name":"/dev/sda","Creation Date":"20-04-2023","Creation Time":"08:30:11 AM","Emulation type":"default","Cachebypass size":"Cachebypass-64k","Cachebypass Mode":"Cachebypass Intelligent","Is LD Ready for OS Requests":"Yes","SCSI NAA Id":"600062b212da5d402bd3b493e1699377","Unmap Enabled":"No"}}}]} \ No newline at end of file +{"Controllers":[{"Command Status":{"CLI Version":"007.1616.0000.0000 Dec 24, 2020","Operating system":"Linux 4.18.0-553.22.1.el8_10.x86_64","Controller":0,"Status":"Success","Description":"None"},"Response Data":{"/c0/v228":[{"DG/VD":"11/228","TYPE":"RAID0","State":"Optl","Access":"RW","Consist":"Yes","Cache":"RWTD","Cac":"-","sCC":"ON","Size":"16.370 TB","Name":""}],"PDs for VD 228":[{"EID:Slt":"251:12","DID":1,"State":"Onln","DG":11,"Size":"16.370 TB","Intf":"SAS","Med":"HDD","SED":"N","PI":"N","SeSz":"512B","Model":"ST18000NM000D ","Sp":"U","Type":"-"}],"VD228 Properties":{"Strip Size":"256 KB","Number of Blocks":35155607552,"VD has Emulated PD":"Yes","Span Depth":1,"Number of Drives Per Span":1,"Write Cache(initial setting)":"WriteThrough","Disk Cache Policy":"Disk's Default","Encryption":"None","Data Protection":"None","Active Operations":"None","Exposed to OS":"Yes","Creation Date":"20-04-2023","Creation Time":"08:30:11 AM","Emulation type":"default","Cachebypass size":"Cachebypass-64k","Cachebypass Mode":"Cachebypass Intelligent","Is LD Ready for OS Requests":"Yes","SCSI NAA Id":"600062b212da5d402bd3b493e1699377","Unmap Enabled":"No"}}}]} diff --git a/pkg/utils/path.go b/pkg/utils/path.go new file mode 100644 index 0000000..7a498c8 --- /dev/null +++ b/pkg/utils/path.go @@ -0,0 +1,9 @@ +package utils + +import "os" + +// FileExists checks if a file exists at the given path. +func FileExists(path string) bool { + _, err := os.Stat(path) + return !os.IsNotExist(err) +}