From d607bc13424285ca463ff6be812e8a19be251d28 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Wed, 9 Oct 2024 14:01:37 -0300 Subject: [PATCH 1/7] redfish/drive: expose oem data and actions Signed-off-by: Matt Vandermeulen --- redfish/drive.go | 21 ++++++++++++--------- redfish/drive_test.go | 4 ++-- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/redfish/drive.go b/redfish/drive.go index 79eace6a..c6e248bb 100644 --- a/redfish/drive.go +++ b/redfish/drive.go @@ -299,6 +299,10 @@ type Drive struct { // WriteCacheEnabled shall indicate whether the drive // write cache is enabled. WriteCacheEnabled bool + // Actions are all the listed actions under the drive + Actions DriveActions + // Oem is all the available OEM information for the drive + Oem json.RawMessage // ActiveSoftwareImage shall contain a link a resource of type SoftwareInventory that represents the active drive // firmware image. @@ -329,12 +333,16 @@ type Drive struct { storagePools []string // storagePools []string StoragePoolsCount int - // secureEraseTarget is the URL for SecureErase actions. - secureEraseTarget string // rawData holds the original serialized JSON so we can compare updates. rawData []byte } +// DriveActions are a set of actions available under a drive +type DriveActions struct { + SecureErase common.ActionTarget `json:"#Drive.SecureErase"` + Oem json.RawMessage +} + // UnmarshalJSON unmarshals a Drive object from the raw JSON. func (drive *Drive) UnmarshalJSON(b []byte) error { type temp Drive @@ -364,13 +372,9 @@ func (drive *Drive) UnmarshalJSON(b []byte) error { Volumes common.Links VolumeCount int `json:"Volumes@odata.count"` } - type Actions struct { - SecureErase common.ActionTarget `json:"#Drive.SecureErase"` - } var t struct { temp Links links - Actions Actions Assembly common.Link EnvironmentMetrics common.Link } @@ -391,6 +395,7 @@ func (drive *Drive) UnmarshalJSON(b []byte) error { drive.EndpointsCount = t.Links.EndpointCount drive.networkDeviceFunctions = t.Links.NetworkDeviceFunctions.ToStrings() drive.NetworkDeviceFunctionsCount = t.Links.NetworkDeviceFunctionsCount + drive.Oem = t.Oem drive.pcieFunctions = t.Links.PCIeFunctions.ToStrings() drive.PCIeFunctionCount = t.Links.PCIeFunctionsCount drive.softwareImages = t.Links.SoftwareImages.ToStrings() @@ -401,8 +406,6 @@ func (drive *Drive) UnmarshalJSON(b []byte) error { drive.volumes = t.Links.Volumes.ToStrings() drive.VolumesCount = t.Links.VolumeCount - drive.secureEraseTarget = t.Actions.SecureErase.Target - // This is a read/write object, so we need to save the raw object data for later drive.rawData = b @@ -486,5 +489,5 @@ func (drive *Drive) PCIeFunctions() ([]*PCIeFunction, error) { // SecureErase shall perform a secure erase of the drive. func (drive *Drive) SecureErase() error { - return drive.Post(drive.secureEraseTarget, nil) + return drive.Post(drive.Actions.SecureErase.Target, nil) } diff --git a/redfish/drive_test.go b/redfish/drive_test.go index ed3d6f68..2532751a 100644 --- a/redfish/drive_test.go +++ b/redfish/drive_test.go @@ -138,8 +138,8 @@ func TestDrive(t *testing.T) { t.Errorf("Invalid chassis link: %s", result.chassis) } - if result.secureEraseTarget != "/redfish/v1/Chassis/NVMeChassis/Disk.Bay.0/Actions/Drive.SecureErase" { - t.Errorf("Invalid SecureErase target: %s", result.secureEraseTarget) + if result.Actions.SecureErase.Target != "/redfish/v1/Chassis/NVMeChassis/Disk.Bay.0/Actions/Drive.SecureErase" { + t.Errorf("Invalid SecureErase target: %s", result.Actions.SecureErase.Target) } } From 39cf8348805db82e6129348cf0f787d9a6749585 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 10:33:20 -0300 Subject: [PATCH 2/7] oem/smc: introduce drive and drive action oem fields Signed-off-by: Matt Vandermeulen --- oem/smc/drive.go | 62 +++++++++++++++++++++++++++++++++ oem/smc/drive_test.go | 80 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 oem/smc/drive.go create mode 100644 oem/smc/drive_test.go diff --git a/oem/smc/drive.go b/oem/smc/drive.go new file mode 100644 index 00000000..a69d51fd --- /dev/null +++ b/oem/smc/drive.go @@ -0,0 +1,62 @@ +package smc + +import ( + "encoding/json" + + "github.com/stmcginnis/gofish/redfish" +) + +type Drive struct { + redfish.Drive + Oem DriveOem `json:"Oem"` +} + +type DriveOem struct { + Supermicro struct { + Temperature int + PercentageDriveLifeUsed int + DriveFunctional bool + } `json:"Supermicro"` +} + +type DriveTarget struct { + Target string `json:"target"` + ActionInfo string `json:"@Redfish.ActionInfo"` +} + +type DriveActions struct { + redfish.DriveActions + Oem struct { + DriveIndicate DriveTarget `json:"#Drive.Indicate"` + SmcDriveIndicate DriveTarget `json:"#SmcDrive.Indicate"` + } `json:"Oem"` +} + +func FromDrive(drive *redfish.Drive) (Drive, error) { + var oem DriveOem + err := json.Unmarshal(drive.Oem, &oem) + + return Drive{ + Drive: *drive, + Oem: oem, + }, err +} + +func FromDriveActions(da *redfish.DriveActions) (DriveActions, error) { + oemActions := DriveActions{ + DriveActions: *da, + } + + err := json.Unmarshal(da.Oem, &oemActions.Oem) + return oemActions, err +} + +// DriveIndicateTarget checks both the SmcDriveIndicate and DriveIndicateTarget +// Oem entries and returns the first populated target, due to key inconsistencies +func (da DriveActions) DriveIndicateTarget() string { + if len(da.Oem.SmcDriveIndicate.Target) > 0 { + return da.Oem.SmcDriveIndicate.Target + } + + return da.Oem.DriveIndicate.Target +} diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go new file mode 100644 index 00000000..d8045935 --- /dev/null +++ b/oem/smc/drive_test.go @@ -0,0 +1,80 @@ +package smc + +import ( + "encoding/json" + "testing" + + "github.com/stmcginnis/gofish/redfish" +) + +var smcDriveBody = `{ + "@odata.type": "#Drive.v1_6_2.Drive", + "@odata.id": "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22", + "Name": "Disk.Bay.22", + "Id": "22", + "Manufacturer": "INTEL", + "SerialNumber": "PHLWOOFMEOWIAMCATDOG", + "Model": "INTEL SSDPE2KX080T8O", + "StatusIndicator": "OK", + "FailurePredicted": false, + "CapacityBytes": 8001563222016, + "CapableSpeedGbs": 31.5, + "Oem": { + "Supermicro": { + "@odata.type": "#SmcDriveExtensions.v1_0_0.Drive", + "Temperature": 33, + "PercentageDriveLifeUsed": 3, + "DriveFunctional": true + } + }, + "IndicatorLED": "Off", + "Status": { + "State": "Enabled", + "Health": "OK" + }, + "Links": { + "Volumes": [] + }, + "Actions": { + "Oem": { + "#Drive.Indicate": { + "target": "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate", + "@Redfish.ActionInfo": "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/IndicateActionInfo" + } + } + } +}` + +// TestSmcDriveOem tests the parsing of the Drive oem field +func TestSmcDriveOem(t *testing.T) { + drive := &redfish.Drive{} + if err := json.Unmarshal([]byte(smcDriveBody), drive); err != nil { + t.Fatalf("error decoding json: %v", err) + } + + smcDrive, err := FromDrive(drive) + if err != nil { + t.Fatalf("error getting oem info from drive: %v", err) + } + + if smcDrive.Oem.Supermicro.Temperature != 33 { + t.Errorf("unexpected oem drive temerature: %d", smcDrive.Oem.Supermicro.Temperature) + } +} + +// TestSmcDriveActionOem tests the parsing of the Drive Actions field +func TestSmcDriveActionOem(t *testing.T) { + drive := &redfish.Drive{} + if err := json.Unmarshal([]byte(smcDriveBody), drive); err != nil { + t.Fatalf("error decoding json: %v", err) + } + + smcDriveActions, err := FromDriveActions(&drive.Actions) + if err != nil { + t.Fatalf("error getting oem info from drive: %v", err) + } + + if smcDriveActions.DriveIndicateTarget() != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { + t.Errorf("unexpected oem drive indicator target: %s", smcDriveActions.DriveIndicateTarget()) + } +} From f8feb2d5c0dfd5640717e6ce55f4cccf1b04f911 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 12:55:56 -0300 Subject: [PATCH 3/7] oem/smc: address some feedback Signed-off-by: Matt Vandermeulen --- oem/smc/drive.go | 74 +++++++++++++++++++++++++++++-------------- oem/smc/drive_test.go | 21 +++--------- 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/oem/smc/drive.go b/oem/smc/drive.go index a69d51fd..28314a57 100644 --- a/oem/smc/drive.go +++ b/oem/smc/drive.go @@ -6,12 +6,16 @@ import ( "github.com/stmcginnis/gofish/redfish" ) +// Drive extends a redfish.Drive for additional OEM fields type Drive struct { redfish.Drive - Oem DriveOem `json:"Oem"` + smc struct { + Oem driveOem `json:"Oem"` + Actions driveActions `json:"Actions"` + } } -type DriveOem struct { +type driveOem struct { Supermicro struct { Temperature int PercentageDriveLifeUsed int @@ -19,44 +23,66 @@ type DriveOem struct { } `json:"Supermicro"` } -type DriveTarget struct { +type driveTarget struct { Target string `json:"target"` ActionInfo string `json:"@Redfish.ActionInfo"` } -type DriveActions struct { +type driveActions struct { redfish.DriveActions Oem struct { - DriveIndicate DriveTarget `json:"#Drive.Indicate"` - SmcDriveIndicate DriveTarget `json:"#SmcDrive.Indicate"` + DriveIndicate driveTarget `json:"#Drive.Indicate"` + SmcDriveIndicate driveTarget `json:"#SmcDrive.Indicate"` } `json:"Oem"` } +// FromDrive returns an OEM-extended redfish drive func FromDrive(drive *redfish.Drive) (Drive, error) { - var oem DriveOem - err := json.Unmarshal(drive.Oem, &oem) - - return Drive{ + smcDrive := Drive{ Drive: *drive, - Oem: oem, - }, err -} + } + smcDrive.smc.Actions.DriveActions = drive.Actions -func FromDriveActions(da *redfish.DriveActions) (DriveActions, error) { - oemActions := DriveActions{ - DriveActions: *da, + if err := json.Unmarshal(drive.Oem, &smcDrive.smc.Oem); err != nil { + return smcDrive, err } - err := json.Unmarshal(da.Oem, &oemActions.Oem) - return oemActions, err + if err := json.Unmarshal(drive.Actions.Oem, &smcDrive.smc.Actions.Oem); err != nil { + return smcDrive, err + } + + return smcDrive, nil +} + +// Temperature returns the OEM provided temperature for the drive +func (d Drive) Temperature() int { + return d.smc.Oem.Supermicro.Temperature +} + +// PercentageDriveLifeUsed returns the OEM provided drive life estimate as a percentage used +func (d Drive) PercentageDriveLifeUsed() int { + return d.smc.Oem.Supermicro.PercentageDriveLifeUsed } -// DriveIndicateTarget checks both the SmcDriveIndicate and DriveIndicateTarget -// Oem entries and returns the first populated target, due to key inconsistencies -func (da DriveActions) DriveIndicateTarget() string { - if len(da.Oem.SmcDriveIndicate.Target) > 0 { - return da.Oem.SmcDriveIndicate.Target +// Functional returns the OEM provided flag that suggests whether a drive is functional or not +func (d Drive) Functional() bool { + return d.smc.Oem.Supermicro.DriveFunctional +} + +// indicateTarget figures out what uri to follow for indicator light actions. +// This is a separate function for testing. +func (d Drive) indicateTarget() string { + // We check both the SmcDriveIndicate and the DriveIndicate targets + // in the Oem sections - certain models and bmc firmwares will mix + // these up, so we check both + if len(d.smc.Actions.Oem.SmcDriveIndicate.Target) > 0 { + return d.smc.Actions.Oem.SmcDriveIndicate.Target } - return da.Oem.DriveIndicate.Target + return d.smc.Actions.Oem.DriveIndicate.Target +} + +// Indicate will set the indicator light activity, true for on, false for off +func (d Drive) Indicate(active bool) error { + return d.Post(d.indicateTarget(), map[string]interface{}{"Active": active}) } diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go index d8045935..4635d793 100644 --- a/oem/smc/drive_test.go +++ b/oem/smc/drive_test.go @@ -57,24 +57,11 @@ func TestSmcDriveOem(t *testing.T) { t.Fatalf("error getting oem info from drive: %v", err) } - if smcDrive.Oem.Supermicro.Temperature != 33 { - t.Errorf("unexpected oem drive temerature: %d", smcDrive.Oem.Supermicro.Temperature) - } -} - -// TestSmcDriveActionOem tests the parsing of the Drive Actions field -func TestSmcDriveActionOem(t *testing.T) { - drive := &redfish.Drive{} - if err := json.Unmarshal([]byte(smcDriveBody), drive); err != nil { - t.Fatalf("error decoding json: %v", err) - } - - smcDriveActions, err := FromDriveActions(&drive.Actions) - if err != nil { - t.Fatalf("error getting oem info from drive: %v", err) + if smcDrive.Temperature() != 33 { + t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature()) } - if smcDriveActions.DriveIndicateTarget() != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { - t.Errorf("unexpected oem drive indicator target: %s", smcDriveActions.DriveIndicateTarget()) + if smcDrive.indicateTarget() != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { + t.Errorf("unexpected oem drive indicator target: %s", smcDrive.indicateTarget()) } } From 5db2bd6291acc5031fc21d286de4de8791f5b9ab Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 13:19:52 -0300 Subject: [PATCH 4/7] redfish/drive: expose raw data for oem support Signed-off-by: Matt Vandermeulen --- redfish/drive.go | 27 ++++++++++++++------------- redfish/drive_test.go | 4 ++-- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/redfish/drive.go b/redfish/drive.go index c6e248bb..a90b92a0 100644 --- a/redfish/drive.go +++ b/redfish/drive.go @@ -299,8 +299,6 @@ type Drive struct { // WriteCacheEnabled shall indicate whether the drive // write cache is enabled. WriteCacheEnabled bool - // Actions are all the listed actions under the drive - Actions DriveActions // Oem is all the available OEM information for the drive Oem json.RawMessage @@ -333,14 +331,11 @@ type Drive struct { storagePools []string // storagePools []string StoragePoolsCount int - // rawData holds the original serialized JSON so we can compare updates. - rawData []byte -} - -// DriveActions are a set of actions available under a drive -type DriveActions struct { - SecureErase common.ActionTarget `json:"#Drive.SecureErase"` - Oem json.RawMessage + // secureEraseTarget is the URL for SecureErase actions. + secureEraseTarget string + // RawData holds the original serialized JSON so we can compare updates + // as well as access Oem values in the oem package. + RawData []byte } // UnmarshalJSON unmarshals a Drive object from the raw JSON. @@ -372,9 +367,13 @@ func (drive *Drive) UnmarshalJSON(b []byte) error { Volumes common.Links VolumeCount int `json:"Volumes@odata.count"` } + type Actions struct { + SecureErase common.ActionTarget `json:"#Drive.SecureErase"` + } var t struct { temp Links links + Actions Actions Assembly common.Link EnvironmentMetrics common.Link } @@ -406,8 +405,10 @@ func (drive *Drive) UnmarshalJSON(b []byte) error { drive.volumes = t.Links.Volumes.ToStrings() drive.VolumesCount = t.Links.VolumeCount + drive.secureEraseTarget = t.Actions.SecureErase.Target + // This is a read/write object, so we need to save the raw object data for later - drive.rawData = b + drive.RawData = b return nil } @@ -417,7 +418,7 @@ func (drive *Drive) Update() error { // Get a representation of the object's original state so we can find what // to update. original := new(Drive) - err := original.UnmarshalJSON(drive.rawData) + err := original.UnmarshalJSON(drive.RawData) if err != nil { return err } @@ -489,5 +490,5 @@ func (drive *Drive) PCIeFunctions() ([]*PCIeFunction, error) { // SecureErase shall perform a secure erase of the drive. func (drive *Drive) SecureErase() error { - return drive.Post(drive.Actions.SecureErase.Target, nil) + return drive.Post(drive.secureEraseTarget, nil) } diff --git a/redfish/drive_test.go b/redfish/drive_test.go index 2532751a..ed3d6f68 100644 --- a/redfish/drive_test.go +++ b/redfish/drive_test.go @@ -138,8 +138,8 @@ func TestDrive(t *testing.T) { t.Errorf("Invalid chassis link: %s", result.chassis) } - if result.Actions.SecureErase.Target != "/redfish/v1/Chassis/NVMeChassis/Disk.Bay.0/Actions/Drive.SecureErase" { - t.Errorf("Invalid SecureErase target: %s", result.Actions.SecureErase.Target) + if result.secureEraseTarget != "/redfish/v1/Chassis/NVMeChassis/Disk.Bay.0/Actions/Drive.SecureErase" { + t.Errorf("Invalid SecureErase target: %s", result.secureEraseTarget) } } From 2b2fce49628a4a6abf1ba015b82d241d23cc1ba6 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 13:30:48 -0300 Subject: [PATCH 5/7] oem/smc: simplify the drive data Signed-off-by: Matt Vandermeulen --- oem/smc/drive.go | 94 +++++++++++++++++++++++-------------------- oem/smc/drive_test.go | 4 +- 2 files changed, 52 insertions(+), 46 deletions(-) diff --git a/oem/smc/drive.go b/oem/smc/drive.go index 28314a57..c143e93e 100644 --- a/oem/smc/drive.go +++ b/oem/smc/drive.go @@ -2,38 +2,28 @@ package smc import ( "encoding/json" + "errors" + "github.com/stmcginnis/gofish/common" "github.com/stmcginnis/gofish/redfish" ) +// ErrActionNotSupported is returned when the requested OEM-specific action +// does not appear to be supported. This might happen when a device is new +// or upgraded to a new firmware that follows the DMTF standards. +var ErrActionNotSupported = errors.New("oem-specific action unsupported") + // Drive extends a redfish.Drive for additional OEM fields type Drive struct { redfish.Drive - smc struct { - Oem driveOem `json:"Oem"` - Actions driveActions `json:"Actions"` - } -} -type driveOem struct { - Supermicro struct { - Temperature int - PercentageDriveLifeUsed int - DriveFunctional bool - } `json:"Supermicro"` -} - -type driveTarget struct { - Target string `json:"target"` - ActionInfo string `json:"@Redfish.ActionInfo"` -} + // Fields from the SMC OEM section + temperature int + percentageDriveLifeUsed int + driveFunctional bool -type driveActions struct { - redfish.DriveActions - Oem struct { - DriveIndicate driveTarget `json:"#Drive.Indicate"` - SmcDriveIndicate driveTarget `json:"#SmcDrive.Indicate"` - } `json:"Oem"` + // indicateTarget is the uri to hit to change the light state + indicateTarget string } // FromDrive returns an OEM-extended redfish drive @@ -41,48 +31,64 @@ func FromDrive(drive *redfish.Drive) (Drive, error) { smcDrive := Drive{ Drive: *drive, } - smcDrive.smc.Actions.DriveActions = drive.Actions - if err := json.Unmarshal(drive.Oem, &smcDrive.smc.Oem); err != nil { - return smcDrive, err + var t struct { + Oem struct { + Supermicro struct { + Temperature int + PercentageDriveLifeUsed int + DriveFunctional bool + } `json:"Supermicro"` + } `json:"Oem"` + Actions struct { + Oem struct { + DriveIndicate common.ActionTarget `json:"#Drive.Indicate"` + SmcDriveIndicate common.ActionTarget `json:"#SmcDrive.Indicate"` + } `json:"Oem"` + } `json:"Actions"` } - if err := json.Unmarshal(drive.Actions.Oem, &smcDrive.smc.Actions.Oem); err != nil { + // Populate the Oem data + if err := json.Unmarshal(drive.RawData, &t); err != nil { return smcDrive, err } + smcDrive.temperature = t.Oem.Supermicro.Temperature + smcDrive.percentageDriveLifeUsed = t.Oem.Supermicro.PercentageDriveLifeUsed + smcDrive.driveFunctional = t.Oem.Supermicro.DriveFunctional + + // We check both the SmcDriveIndicate and the DriveIndicate targets + // in the Oem sections - certain models and bmc firmwares will mix + // these up, so we check both + smcDrive.indicateTarget = t.Actions.Oem.DriveIndicate.Target + if len(t.Actions.Oem.SmcDriveIndicate.Target) > 0 { + smcDrive.indicateTarget = t.Actions.Oem.SmcDriveIndicate.Target + } + return smcDrive, nil } // Temperature returns the OEM provided temperature for the drive func (d Drive) Temperature() int { - return d.smc.Oem.Supermicro.Temperature + return d.temperature } // PercentageDriveLifeUsed returns the OEM provided drive life estimate as a percentage used func (d Drive) PercentageDriveLifeUsed() int { - return d.smc.Oem.Supermicro.PercentageDriveLifeUsed + return d.percentageDriveLifeUsed } // Functional returns the OEM provided flag that suggests whether a drive is functional or not func (d Drive) Functional() bool { - return d.smc.Oem.Supermicro.DriveFunctional -} - -// indicateTarget figures out what uri to follow for indicator light actions. -// This is a separate function for testing. -func (d Drive) indicateTarget() string { - // We check both the SmcDriveIndicate and the DriveIndicate targets - // in the Oem sections - certain models and bmc firmwares will mix - // these up, so we check both - if len(d.smc.Actions.Oem.SmcDriveIndicate.Target) > 0 { - return d.smc.Actions.Oem.SmcDriveIndicate.Target - } - - return d.smc.Actions.Oem.DriveIndicate.Target + return d.driveFunctional } // Indicate will set the indicator light activity, true for on, false for off func (d Drive) Indicate(active bool) error { - return d.Post(d.indicateTarget(), map[string]interface{}{"Active": active}) + // Return a common error to let the user try falling back on the normal gofish path + if d.indicateTarget == "" { + return ErrActionNotSupported + } + + return d.Post(d.indicateTarget, map[string]interface{}{"Active": active}) } diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go index 4635d793..e2d2f349 100644 --- a/oem/smc/drive_test.go +++ b/oem/smc/drive_test.go @@ -61,7 +61,7 @@ func TestSmcDriveOem(t *testing.T) { t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature()) } - if smcDrive.indicateTarget() != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { - t.Errorf("unexpected oem drive indicator target: %s", smcDrive.indicateTarget()) + if smcDrive.indicateTarget != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { + t.Errorf("unexpected oem drive indicator target: %s", smcDrive.indicateTarget) } } From 0e46b2151831512a2b7d0119e81e954556ed8ac7 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 13:53:18 -0300 Subject: [PATCH 6/7] oem/smc: replace getters with direct members for top level oem data --- oem/smc/drive.go | 27 ++++++--------------------- oem/smc/drive_test.go | 4 ++-- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/oem/smc/drive.go b/oem/smc/drive.go index c143e93e..9013910b 100644 --- a/oem/smc/drive.go +++ b/oem/smc/drive.go @@ -18,9 +18,9 @@ type Drive struct { redfish.Drive // Fields from the SMC OEM section - temperature int - percentageDriveLifeUsed int - driveFunctional bool + Temperature int + PercentageDriveLifeUsed int + DriveFunctional bool // indicateTarget is the uri to hit to change the light state indicateTarget string @@ -53,9 +53,9 @@ func FromDrive(drive *redfish.Drive) (Drive, error) { return smcDrive, err } - smcDrive.temperature = t.Oem.Supermicro.Temperature - smcDrive.percentageDriveLifeUsed = t.Oem.Supermicro.PercentageDriveLifeUsed - smcDrive.driveFunctional = t.Oem.Supermicro.DriveFunctional + smcDrive.Temperature = t.Oem.Supermicro.Temperature + smcDrive.PercentageDriveLifeUsed = t.Oem.Supermicro.PercentageDriveLifeUsed + smcDrive.DriveFunctional = t.Oem.Supermicro.DriveFunctional // We check both the SmcDriveIndicate and the DriveIndicate targets // in the Oem sections - certain models and bmc firmwares will mix @@ -68,21 +68,6 @@ func FromDrive(drive *redfish.Drive) (Drive, error) { return smcDrive, nil } -// Temperature returns the OEM provided temperature for the drive -func (d Drive) Temperature() int { - return d.temperature -} - -// PercentageDriveLifeUsed returns the OEM provided drive life estimate as a percentage used -func (d Drive) PercentageDriveLifeUsed() int { - return d.percentageDriveLifeUsed -} - -// Functional returns the OEM provided flag that suggests whether a drive is functional or not -func (d Drive) Functional() bool { - return d.driveFunctional -} - // Indicate will set the indicator light activity, true for on, false for off func (d Drive) Indicate(active bool) error { // Return a common error to let the user try falling back on the normal gofish path diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go index e2d2f349..0a6fb010 100644 --- a/oem/smc/drive_test.go +++ b/oem/smc/drive_test.go @@ -57,8 +57,8 @@ func TestSmcDriveOem(t *testing.T) { t.Fatalf("error getting oem info from drive: %v", err) } - if smcDrive.Temperature() != 33 { - t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature()) + if smcDrive.Temperature != 33 { + t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature) } if smcDrive.indicateTarget != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" { From 1387dcb35cf1dcf81d900190a46b05bc7e579ab8 Mon Sep 17 00:00:00 2001 From: Matt Vandermeulen Date: Tue, 15 Oct 2024 14:55:08 -0300 Subject: [PATCH 7/7] oem/smc: address linter Signed-off-by: Matt Vandermeulen --- oem/smc/drive.go | 8 ++++++-- oem/smc/drive_test.go | 6 +++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/oem/smc/drive.go b/oem/smc/drive.go index 9013910b..dd2f20fc 100644 --- a/oem/smc/drive.go +++ b/oem/smc/drive.go @@ -1,3 +1,7 @@ +// +// SPDX-License-Identifier: BSD-3-Clause +// + package smc import ( @@ -61,7 +65,7 @@ func FromDrive(drive *redfish.Drive) (Drive, error) { // in the Oem sections - certain models and bmc firmwares will mix // these up, so we check both smcDrive.indicateTarget = t.Actions.Oem.DriveIndicate.Target - if len(t.Actions.Oem.SmcDriveIndicate.Target) > 0 { + if t.Actions.Oem.SmcDriveIndicate.Target != "" { smcDrive.indicateTarget = t.Actions.Oem.SmcDriveIndicate.Target } @@ -69,7 +73,7 @@ func FromDrive(drive *redfish.Drive) (Drive, error) { } // Indicate will set the indicator light activity, true for on, false for off -func (d Drive) Indicate(active bool) error { +func (d *Drive) Indicate(active bool) error { // Return a common error to let the user try falling back on the normal gofish path if d.indicateTarget == "" { return ErrActionNotSupported diff --git a/oem/smc/drive_test.go b/oem/smc/drive_test.go index 0a6fb010..f61a4793 100644 --- a/oem/smc/drive_test.go +++ b/oem/smc/drive_test.go @@ -1,3 +1,7 @@ +// +// SPDX-License-Identifier: BSD-3-Clause +// + package smc import ( @@ -58,7 +62,7 @@ func TestSmcDriveOem(t *testing.T) { } if smcDrive.Temperature != 33 { - t.Errorf("unexpected oem drive temerature: %d", smcDrive.Temperature) + t.Errorf("unexpected oem drive temperature: %d", smcDrive.Temperature) } if smcDrive.indicateTarget != "/redfish/v1/Chassis/NVMeSSD.0.Group.0.StorageBackplane/Drives/Disk.Bay.22/Actions/Oem/Drive.Indicate" {