From 8170b6a2ed591312dc9c55265012bfbde86717b9 Mon Sep 17 00:00:00 2001 From: Alex Rodriguez <131964409+ezekiel-alexrod@users.noreply.github.com> Date: Wed, 27 May 2026 15:44:57 +0200 Subject: [PATCH] physicaldrivegetter: tolerate predictive failure and unknown ssacli statuses ssacli reports a small open set of "Status:" values, but the parser previously aborted the whole physical-drive inventory as soon as it saw anything outside of OK/Failed/Offline. In particular, a drive flagged "Predictive Failure" - which is still online and serving its array - caused the install to fail. - Map "Predictive Failure" to PDStatusUsed so SMART warnings no longer block the install while the drive is still operational. - Soft-fail unknown statuses (e.g. "Rebuilding", future labels) to PDStatusUnknown instead of returning an error, and surface the raw ssacli label via PhysicalDrive.Reason so callers can still react. - Extract the status lookup into parseSSACLIStatus + a package-level map for clarity, and cover both new paths with fixture-based tests. Refs: ARTESCA-17608 Signed-off-by: Alex Rodriguez <131964409+ezekiel-alexrod@users.noreply.github.com> --- .../predictive_failure_detail.txt | 36 ++++++++ .../physicaldrives/unknown_status_detail.txt | 36 ++++++++ .../physicaldrivegetter/ssacli.go | 39 ++++++--- .../physicaldrivegetter/ssacli_test.go | 83 +++++++++++++++++++ 4 files changed, 182 insertions(+), 12 deletions(-) create mode 100644 pkg/implementation/physicaldrivegetter/physicaldrives/predictive_failure_detail.txt create mode 100644 pkg/implementation/physicaldrivegetter/physicaldrives/unknown_status_detail.txt diff --git a/pkg/implementation/physicaldrivegetter/physicaldrives/predictive_failure_detail.txt b/pkg/implementation/physicaldrivegetter/physicaldrives/predictive_failure_detail.txt new file mode 100644 index 0000000..fd46c98 --- /dev/null +++ b/pkg/implementation/physicaldrivegetter/physicaldrives/predictive_failure_detail.txt @@ -0,0 +1,36 @@ + +HPE Smart Array P816i-a SR Gen10 in Slot 0 (Embedded) + + Array K + + physicaldrive 3I:3:2 + Port: 3I + Box: 3 + Bay: 2 + Status: Predictive Failure + Drive Type: Data Drive + Interface Type: SAS + Size: 6 TB + Drive exposed to OS: False + Logical/Physical Block Size: 512/4096 + Rotational Speed: 7200 + Firmware Revision: HPD4 + Serial Number: ZADBBFGH + WWID: 5000C500CB811B99 + Model: HP MB6000JVYZD + Current Temperature (C): 33 + Maximum Temperature (C): 45 + PHY Count: 2 + PHY Transfer Rate: 12.0Gbps, Unknown + PHY Physical Link Rate: 12.0Gbps, Unknown + PHY Maximum Link Rate: 12.0Gbps, 12.0Gbps + Drive Authentication Status: OK + Carrier Application Version: 11 + Carrier Bootloader Version: 6 + Sanitize Erase Supported: True + Sanitize Estimated Max Erase Time: 11 hour(s), 40 minute(s) + Unrestricted Sanitize Supported: True + Shingled Magnetic Recording Support: None + Drive Unique ID: 5000C500CB811B9B + Multi-Actuator Drive: False + Write Cache Status: Disabled diff --git a/pkg/implementation/physicaldrivegetter/physicaldrives/unknown_status_detail.txt b/pkg/implementation/physicaldrivegetter/physicaldrives/unknown_status_detail.txt new file mode 100644 index 0000000..19a17a9 --- /dev/null +++ b/pkg/implementation/physicaldrivegetter/physicaldrives/unknown_status_detail.txt @@ -0,0 +1,36 @@ + +HPE Smart Array P816i-a SR Gen10 in Slot 0 (Embedded) + + Array K + + physicaldrive 3I:3:2 + Port: 3I + Box: 3 + Bay: 2 + Status: Rebuilding + Drive Type: Data Drive + Interface Type: SAS + Size: 6 TB + Drive exposed to OS: False + Logical/Physical Block Size: 512/4096 + Rotational Speed: 7200 + Firmware Revision: HPD4 + Serial Number: ZADBBFGH + WWID: 5000C500CB811B99 + Model: HP MB6000JVYZD + Current Temperature (C): 33 + Maximum Temperature (C): 45 + PHY Count: 2 + PHY Transfer Rate: 12.0Gbps, Unknown + PHY Physical Link Rate: 12.0Gbps, Unknown + PHY Maximum Link Rate: 12.0Gbps, 12.0Gbps + Drive Authentication Status: OK + Carrier Application Version: 11 + Carrier Bootloader Version: 6 + Sanitize Erase Supported: True + Sanitize Estimated Max Erase Time: 11 hour(s), 40 minute(s) + Unrestricted Sanitize Supported: True + Shingled Magnetic Recording Support: None + Drive Unique ID: 5000C500CB811B9B + Multi-Actuator Drive: False + Write Cache Status: Disabled diff --git a/pkg/implementation/physicaldrivegetter/ssacli.go b/pkg/implementation/physicaldrivegetter/ssacli.go index 6ac24ab..b012470 100644 --- a/pkg/implementation/physicaldrivegetter/ssacli.go +++ b/pkg/implementation/physicaldrivegetter/ssacli.go @@ -29,6 +29,20 @@ var ( ssacliSlotRegexp = regexp.MustCompile(ssacliSlotRegexpPattern) ssacliPhysicalDriveRegexp = regexp.MustCompile(ssacliPhysicalDriveRegexpPattern) + + // ssacliStatusMap maps known ssacli "Status:" values to a PDStatus. Any + // value not in the map is treated as PDStatusUnknown by parseSSACLIStatus + // so the parser keeps working when ssacli reports a status we don't model + // (e.g. "Predictive Failure", "Rebuilding", or a future label). A + // "Predictive Failure" drive is still online and serving its array, so it + // is mapped to PDStatusUsed and the install is allowed to continue. + //nolint:gochecknoglobals // small lookup table. + ssacliStatusMap = map[string]physicaldrive.PDStatus{ + "OK": physicaldrive.PDStatusUsed, + "Failed": physicaldrive.PDStatusFailed, + "Offline": physicaldrive.PDStatusFailed, + "Predictive Failure": physicaldrive.PDStatusUsed, + } ) // NewSSACLI creates a new SSACLI instance. @@ -202,18 +216,8 @@ func (s *SSACLI) parsePDLine( //nolint:funlen // This function is long and not c case "Status": if physicalDrive.Status == physicaldrive.PDStatusUnknown { - mapStatus := map[string]physicaldrive.PDStatus{ - "OK": physicaldrive.PDStatusUsed, - "Failed": physicaldrive.PDStatusFailed, - "Offline": physicaldrive.PDStatusFailed, - } - - status, ok := mapStatus[value] - if !ok { - return errors.Errorf("invalid status: %s", value) - } - - physicalDrive.Status = status + physicalDrive.Status = parseSSACLIStatus(value) + physicalDrive.Reason = value } case "Drive Type": @@ -300,3 +304,14 @@ func isBlockDeviceUsed(device *BlockDevice) bool { return false } + +// parseSSACLIStatus maps an ssacli "Status:" value to a PDStatus. Unknown +// values are mapped to PDStatusUnknown rather than aborting the whole inventory +// call; callers can read PhysicalDrive.Reason to recover the raw label. +func parseSSACLIStatus(value string) physicaldrive.PDStatus { + if status, ok := ssacliStatusMap[value]; ok { + return status + } + + return physicaldrive.PDStatusUnknown +} diff --git a/pkg/implementation/physicaldrivegetter/ssacli_test.go b/pkg/implementation/physicaldrivegetter/ssacli_test.go index 0a65674..e258eb9 100644 --- a/pkg/implementation/physicaldrivegetter/ssacli_test.go +++ b/pkg/implementation/physicaldrivegetter/ssacli_test.go @@ -188,3 +188,86 @@ func mockOutput(filename string) []byte { return output } + +func TestSSACLIPhysicalDriveStatus(t *testing.T) { + tests := []struct { + name string + mocking []byte + metadata *physicaldrive.Metadata + expected *physicaldrive.PhysicalDrive + expectedError bool + }{ + { + // "Predictive Failure" must not abort the inventory: the drive is + // still online and serving its array, so it is surfaced as used + // with the raw ssacli label kept in Reason. + name: "predictive failure", + mocking: mockOutput("physicaldrives/predictive_failure_detail"), + metadata: &physicaldrive.Metadata{ + CtrlMetadata: &raidcontroller.Metadata{ + ID: 0, + }, + ID: "3I:3:2", + }, + expected: &physicaldrive.PhysicalDrive{ + Status: physicaldrive.PDStatusUsed, + Reason: "Predictive Failure", + }, + expectedError: false, + }, + { + // An unmodeled status (here "Rebuilding") soft-fails to + // PDStatusUnknown rather than returning an error, so a future + // ssacli label can never again take down the whole inventory call. + name: "unknown status soft-fails", + mocking: mockOutput("physicaldrives/unknown_status_detail"), + metadata: &physicaldrive.Metadata{ + CtrlMetadata: &raidcontroller.Metadata{ + ID: 0, + }, + ID: "3I:3:2", + }, + expected: &physicaldrive.PhysicalDrive{ + Status: physicaldrive.PDStatusUnknown, + Reason: "Rebuilding", + }, + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockRunner := new(MockCommandRunner) + + s := &SSACLI{ + SSACLI: mockRunner, + LSBLK: mockRunner, + } + + mockRunner.On("Run", []string{ + "controller", + "slot=" + strconv.Itoa(tt.metadata.CtrlMetadata.ID), + "physicaldrive", + tt.metadata.ID, + "show", + "detail", + }).Return(tt.mocking, nil) + + lsblkOutput := []byte(`NAME ROTA SIZE TYPE TRAN MOUNTPOINT FSTYPE PARTTYPE +/dev/sda 0 858993459200 disk sata `) + mockRunner.On("Run", mock.AnythingOfType("[]string")).Return(lsblkOutput, nil) + + physicalDrive, err := s.PhysicalDrive(tt.metadata) + + if tt.expectedError { + assert.Error(t, err) + assert.Nil(t, physicalDrive) + } else { + assert.NoError(t, err) + assert.NotEmpty(t, physicalDrive) + assert.Equal(t, tt.expected.Status, physicalDrive.Status) + assert.Equal(t, tt.expected.Reason, physicalDrive.Reason) + } + }) + } +}