Skip to content

Commit f9cbe47

Browse files
ezekiel-alexrodezekiel-alexrod
authored
Merge pull request #18 from scality/bugfix/handle-pkname
[MINOR] Handle PKNAME colum of `lsblk` to set the status of the parent disk of partitions
2 parents c3f06b9 + 9b4f700 commit f9cbe47

3 files changed

Lines changed: 82 additions & 23 deletions

File tree

‎pkg/implementation/physicaldrivegetter/rhel8.go‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ type (
2222
}
2323

2424
BlockDevice struct {
25-
DevicePath string
26-
Size uint64
27-
Rotational string
28-
Type string
29-
Tran string
30-
MountPoint string
31-
PartitionType string
32-
FilesystemType string
25+
DevicePath string
26+
Size uint64
27+
Rotational string
28+
Type string
29+
Tran string
30+
MountPoint string
31+
PartitionType string
32+
FileSystemType string
33+
ParentKernelName string
3334
}
3435
)
3536

@@ -55,9 +56,17 @@ func (r *RHEL8) PhysicalDrives(
5556
return nil, errors.Wrap(err, "failed to list block devices")
5657
}
5758

59+
pkNames := make(map[string]struct{})
60+
5861
physicalDrives := make([]*physicaldrive.PhysicalDrive, 0, len(blockDevices))
5962

6063
for _, device := range blockDevices {
64+
// We need to find the parent kernel names of the block devices
65+
// to set the Used status correctly.
66+
if device.ParentKernelName != "" {
67+
pkNames[device.ParentKernelName] = struct{}{}
68+
}
69+
6170
physicalDrive, err := r.PhysicalDrive(&physicaldrive.Metadata{
6271
ID: device.DevicePath,
6372
})
@@ -68,6 +77,17 @@ func (r *RHEL8) PhysicalDrives(
6877
physicalDrives = append(physicalDrives, physicalDrive)
6978
}
7079

80+
// Set the Used status if the physical drive is a disk
81+
// and its parent kernel name is in the list of block devices
82+
// of the partitions.
83+
for pkName := range pkNames {
84+
for _, physicalDrive := range physicalDrives {
85+
if pkName == physicalDrive.ID {
86+
physicalDrive.Status = physicaldrive.PDStatusUsed
87+
}
88+
}
89+
}
90+
7191
return physicalDrives, nil
7292
}
7393

@@ -207,7 +227,10 @@ func (r *RHEL8) physicalDriveStatus(device *BlockDevice) (physicaldrive.PDStatus
207227
return physicaldrive.PDStatusFailed, "", nil
208228
}
209229

210-
if device.MountPoint != "" || device.FilesystemType != "" || device.PartitionType != "" {
230+
if device.MountPoint != "" ||
231+
device.FileSystemType != "" ||
232+
device.PartitionType != "" ||
233+
device.ParentKernelName != "" {
211234
return physicaldrive.PDStatusUsed, reason, nil
212235
}
213236

@@ -221,7 +244,7 @@ func (r *RHEL8) getBlockDevice(devicePath string) (*BlockDevice, error) {
221244
"--bytes",
222245
"--nodeps",
223246
"--output",
224-
"name,rota,size,type,tran,mountpoint,fstype,parttype",
247+
"name,rota,size,type,tran,mountpoint,fstype,parttype,pkname",
225248
})
226249
if err != nil {
227250
return nil, errors.Wrap(err, "failed to get block device using lsblk")
@@ -245,7 +268,7 @@ func (r *RHEL8) listBlockDevices() ([]BlockDevice, error) {
245268
"--paths",
246269
"--bytes",
247270
"--output",
248-
"name,rota,size,type,tran,mountpoint,fstype,parttype",
271+
"name,rota,size,type,tran,mountpoint,fstype,parttype,pkname",
249272
})
250273
if err != nil {
251274
return nil, errors.Wrap(err, "failed to run list block devices command")
@@ -327,9 +350,11 @@ func ParseLSBLKOutput(output []byte) ([]BlockDevice, error) {
327350
case "MOUNTPOINT":
328351
device.MountPoint = field
329352
case "FSTYPE":
330-
device.FilesystemType = field
353+
device.FileSystemType = field
331354
case "PARTTYPE":
332355
device.PartitionType = field
356+
case "PKNAME":
357+
device.ParentKernelName = field
333358
}
334359
}
335360
}

‎pkg/implementation/physicaldrivegetter/rhel8_test.go‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,15 @@ func TestRHEL8_PhysicalDrives_Success(t *testing.T) {
402402
// Setup mock for listing block devices
403403
mockLSBLK.On("Run", mock.MatchedBy(func(args []string) bool {
404404
return len(args) > 0 && args[0] == "--list"
405-
})).Return([]byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE
406-
/dev/nvme1n1 0 8589934592 disk nvme
407-
/dev/nvme2n1 0 8589934592 disk nvme`), nil)
405+
})).Return([]byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE PKNAME
406+
/dev/nvme1n1 0 8589934592 disk nvme
407+
/dev/nvme1n1p1 0 103809024 part nvme /boot/efi vfat c12a7328-f81f-11d2-ba4b-00a0c93ec93b /dev/nvme1n1
408+
/dev/nvme2n1 0 8589934592 disk nvme`), nil)
408409

409410
// Setup mocks for first device
410411
mockLSBLK.On("Run", mock.MatchedBy(func(args []string) bool {
411412
return len(args) > 0 && args[0] == "/dev/nvme1n1"
412-
})).Return([]byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE
413+
})).Return([]byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE PKNAME
413414
/dev/nvme1n1 0 8589934592 disk nvme`), nil)
414415

415416
mockUDevADM.On("Run", mock.MatchedBy(func(args []string) bool {
@@ -421,6 +422,26 @@ E: DEVNAME=/dev/nvme1n1
421422
E: DEVLINKS=/dev/disk/by-id/nvme-123`), nil)
422423

423424
mockSmartCTL.On("Run", []string{"-a", "/dev/nvme1n1"}).Return([]byte(`
425+
=== START OF SMART DATA SECTION ===
426+
SMART overall-health self-assessment test result: PASSED
427+
`), nil)
428+
429+
// Setup mock for partition of the first device
430+
mockLSBLK.On("Run", mock.MatchedBy(func(args []string) bool {
431+
return len(args) > 0 && args[0] == "/dev/nvme1n1p1"
432+
})).Return([]byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE PKNAME
433+
/dev/nvme1n1p1 0 103809024 part nvme /boot/efi vfat c12a7328-f81f-11d2-ba4b-00a0c93ec93b /dev/nvme1n1
434+
`), nil)
435+
436+
mockUDevADM.On("Run", mock.MatchedBy(func(args []string) bool {
437+
return len(args) > 1 && args[2] == "--name=/dev/nvme1n1p1"
438+
})).Return([]byte(`E: ID_MODEL=Amazon Elastic Block Store
439+
E: ID_SERIAL_SHORT=vol05ece746e40ff492f
440+
E: ID_WWN=nvme.1d0f-123456
441+
E: DEVNAME=/dev/nvme1n1p1
442+
E: DEVLINKS=/dev/disk/by-id/nvme-123`), nil)
443+
444+
mockSmartCTL.On("Run", []string{"-a", "/dev/nvme1n1p1"}).Return([]byte(`
424445
=== START OF SMART DATA SECTION ===
425446
SMART overall-health self-assessment test result: PASSED
426447
`), nil)
@@ -450,21 +471,31 @@ SMART overall-health self-assessment test result: PASSED
450471

451472
// Assertions
452473
assert.NoError(t, err)
453-
assert.Equal(t, 2, len(physicalDrives))
474+
assert.Equal(t, 3, len(physicalDrives))
454475

455476
// First drive assertions
456477
assert.Equal(t, "Amazon Elastic Block Store", physicalDrives[0].Model)
457478
assert.Equal(t, "vol05ece746e40ff492f", physicalDrives[0].Serial)
458479
assert.Equal(t, "/dev/nvme1n1", physicalDrives[0].ID)
459480
assert.Equal(t, "/dev/nvme1n1", physicalDrives[0].DevicePath)
460481
assert.Equal(t, physicaldrive.DiskTypeNVMe, physicalDrives[0].Type)
482+
assert.Equal(t, physicaldrive.PDStatusUsed, physicalDrives[0].Status)
461483

462-
// Second drive assertions
484+
// Partition of the first drive assertions
463485
assert.Equal(t, "Amazon Elastic Block Store", physicalDrives[1].Model)
464-
assert.Equal(t, "vol05ece746e40ff493g", physicalDrives[1].Serial)
465-
assert.Equal(t, "/dev/nvme2n1", physicalDrives[1].ID)
466-
assert.Equal(t, "/dev/nvme2n1", physicalDrives[1].DevicePath)
486+
assert.Equal(t, "vol05ece746e40ff492f", physicalDrives[1].Serial)
487+
assert.Equal(t, "/dev/nvme1n1p1", physicalDrives[1].ID)
488+
assert.Equal(t, "/dev/nvme1n1p1", physicalDrives[1].DevicePath)
467489
assert.Equal(t, physicaldrive.DiskTypeNVMe, physicalDrives[1].Type)
490+
assert.Equal(t, physicaldrive.PDStatusUsed, physicalDrives[1].Status)
491+
492+
// Second drive assertions
493+
assert.Equal(t, "Amazon Elastic Block Store", physicalDrives[2].Model)
494+
assert.Equal(t, "vol05ece746e40ff493g", physicalDrives[2].Serial)
495+
assert.Equal(t, "/dev/nvme2n1", physicalDrives[2].ID)
496+
assert.Equal(t, "/dev/nvme2n1", physicalDrives[2].DevicePath)
497+
assert.Equal(t, physicaldrive.DiskTypeNVMe, physicalDrives[2].Type)
498+
assert.Equal(t, physicaldrive.PDStatusUnassignedGood, physicalDrives[2].Status)
468499

469500
mockLSBLK.AssertExpectations(t)
470501
mockUDevADM.AssertExpectations(t)

‎pkg/implementation/physicaldrivegetter/ssacli.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func (s *SSACLI) getBlockDevice(devicePath string) (*BlockDevice, error) {
260260
"--bytes",
261261
"--nodeps",
262262
"--output",
263-
"name,rota,size,type,tran,mountpoint,fstype,parttype",
263+
"name,rota,size,type,tran,mountpoint,fstype,parttype,pkname",
264264
})
265265
if err != nil {
266266
return nil, errors.Wrap(err, "failed to get block device using lsblk")
@@ -294,7 +294,10 @@ func parseSlotInfo(pd *physicaldrive.PhysicalDrive, key, value string) {
294294
// If the device is mounted or has a filesystem type, it is considered used.
295295
// Otherwise, it is considered unassigned good.
296296
func isBlockDeviceUsed(device *BlockDevice) bool {
297-
if device.MountPoint != "" || device.FilesystemType != "" || device.PartitionType != "" {
297+
if device.MountPoint != "" ||
298+
device.FileSystemType != "" ||
299+
device.PartitionType != "" ||
300+
device.ParentKernelName != "" {
298301
return true
299302
}
300303

0 commit comments

Comments
 (0)