Skip to content

Commit

Permalink
[issue-1114] Reverting SerachDrivePath changes (#1115)
Browse files Browse the repository at this point in the history
  • Loading branch information
katarzyna-kulpa authored Mar 8, 2024
1 parent 9e4a104 commit 9738a24
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 75 deletions.
26 changes: 11 additions & 15 deletions pkg/base/linuxutils/lsblk/lsblk.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,16 @@ func (l *LSBLK) GetBlockDevices(device string) ([]BlockDevice, error) {
return res, nil
}

// SearchDrivePath searches for the drive path by comparing drive's sn, vid, pid with lsblk output
// SearchDrivePath if not defined returns drive path based on drive S/N, VID and PID.
// Receives an instance of drivecrd.Drive struct
// Returns drive's path based on provided drivecrd.Drive or error if something went wrong
func (l *LSBLK) SearchDrivePath(drive *api.Drive) (string, error) {
device := ""
// device path might be already set by hwmgr
device := drive.Path
if device != "" {
return device, nil
}

// try to find it with lsblk
lsblkOut, err := l.GetBlockDevices("")
if err != nil {
Expand All @@ -171,11 +178,8 @@ func (l *LSBLK) SearchDrivePath(drive *api.Drive) (string, error) {
vid := drive.VID
pid := drive.PID
for _, l := range lsblkOut {
lvid := strings.TrimSpace(l.Vendor)
// The hasPrefixIgnoreSpaces function is used to compare pid and lsblk model, accounting for drives that may have
// multiple spaces in their pid, and situations where pid is truncated or limited to 16 digits compared to lsblk model.
if strings.EqualFold(l.Serial, sn) && (lvid == "" || strings.EqualFold(lvid, vid)) &&
hasPrefixIgnoreSpaces(l.Model, pid) {
if strings.EqualFold(l.Serial, sn) && strings.EqualFold(l.Vendor, vid) &&
strings.EqualFold(l.Model, pid) {
device = l.Name
break
}
Expand All @@ -188,11 +192,3 @@ func (l *LSBLK) SearchDrivePath(drive *api.Drive) (string, error) {

return device, nil
}
func hasPrefixIgnoreSpaces(s, prefix string) bool {
empty := ""
emptySpace := " "
s = strings.ReplaceAll(s, emptySpace, empty)
prefix = strings.ReplaceAll(prefix, emptySpace, empty)

return strings.HasPrefix(s, prefix)
}
53 changes: 10 additions & 43 deletions pkg/base/linuxutils/lsblk/lsblk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,35 +85,23 @@ func TestLSBLK_SearchDrivePath_Success(t *testing.T) {
l := NewLSBLK(testLogger)
l.e = e

// path is in drive spec
dCR := testDriveCR
path := "/dev/sda"
dCR.Spec.Path = path

res, err := l.SearchDrivePath(&dCR.Spec)
assert.Nil(t, err)
assert.Equal(t, path, res)

// got from lsblk output
e.On("RunCmd", allDevicesCmd).Return(mocks.LsblkTwoDevicesStr, "", nil)
sn = "hdd1" // from mocks.LsblkTwoDevicesStr
expectedDevice := "/dev/sda" // from mocks.LsblkTwoDevicesStr
d2CR := testDriveCR
d2CR.Spec.SerialNumber = sn

res, err := l.SearchDrivePath(&d2CR.Spec)
assert.Nil(t, err)
assert.Equal(t, expectedDevice, res)
}

func TestLSBLK_SearchDrivePath_ModelWithEmptySpaces_Success(t *testing.T) {
l := NewLSBLK(testLogger)
e := &mocks.GoMockExecutor{}
e.On("RunCmd", allDevicesCmd).Return(mocks.LsblkDevV2, "", nil)
l.e = e

sn := "5000cca0bbce17ff"
//pid := "HGS THUS728T8TAL"
pid := "HGS THUS728T8TA"
vendor := "ATA"
expectedDevice := "/dev/sdc"
d2CR := testDriveCR
d2CR.Spec.SerialNumber = sn
d2CR.Spec.VID = vendor
d2CR.Spec.PID = pid

res, err := l.SearchDrivePath(&d2CR.Spec)
res, err = l.SearchDrivePath(&d2CR.Spec)
assert.Nil(t, err)
assert.Equal(t, expectedDevice, res)
}
Expand Down Expand Up @@ -150,27 +138,6 @@ func TestLSBLK_SearchDrivePath(t *testing.T) {
assert.NotNil(t, err)
}

func TestLSBLK_SearchDrivePath_EmptyVendor_Success(t *testing.T) {
l := NewLSBLK(testLogger)
e := &mocks.GoMockExecutor{}

e.On("RunCmd", allDevicesCmd).Return(mocks.LsblkDevNullVendor, "", nil)
l.e = e

sn := "PHLJ043300VY4P0DGN"
pid := "Dell Express Flash NVMe P4510 4TB SFF"
vendor := "0x8086"
expectedDevice := "/dev/sdc"
d2CR := testDriveCR
d2CR.Spec.SerialNumber = sn
d2CR.Spec.VID = vendor
d2CR.Spec.PID = pid

res, err := l.SearchDrivePath(&d2CR.Spec)
assert.Nil(t, err)
assert.Equal(t, expectedDevice, res)
}

func TestLSBLK_GetBlockDevicesV2_Success(t *testing.T) {
l := NewLSBLK(testLogger)
e := &mocks.GoMockExecutor{}
Expand Down
18 changes: 1 addition & 17 deletions pkg/mocks/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,23 +163,7 @@ var LsblkDevV2 = `{
"serial":"5000cca0bbce17ff",
"wwn":"0x5000cca0bbce17ff",
"vendor":"ATA ",
"model":"HGS THUS728T8TAL",
"rev":"RT04",
"mountpoint":null,
"fstype":null,
"partuuid":null
}]}`

// LsblkDevNullVendor provides output for lsblk with null vendor
var LsblkDevNullVendor = `{
"blockdevices": [{
"name":"/dev/sdc",
"type":"disk",
"size":8001563222016,
"rota":true,
"serial":"PHLJ043300VY4P0DGN",
"vendor":null,
"model":"Dell Express Flash NVMe P4510 4TB SFF",
"model":"HGST_HUS728T8TAL",
"rev":"RT04",
"mountpoint":null,
"fstype":null,
Expand Down

0 comments on commit 9738a24

Please sign in to comment.