From d624d59a66ed2e0d549c5712d2eff11c0f0a063c Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Dec 2023 15:44:51 +0100 Subject: [PATCH 1/6] constants: remove state which indicates the BMC requires a power cycle This is replaced by the firmware install step, and subsequently will be moved into a type FirmwareInstallProperties{} which will replace FirmwareInstallSteps --- constants/constants.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/constants/constants.go b/constants/constants.go index d2a6bd10..5b3f44fa 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -77,10 +77,6 @@ const ( FirmwareInstallPowerCycleHost = "powercycle-host" PowerCycleHost TaskState = "powercycle-host" - // FirmwareInstallPowerCycleBMC indicates the firmware install requires a BMC power cycle - FirmwareInstallPowerCycleBMC = "powercycle-bmc" - PowerCycleBMC TaskState = "powercycle-bmc" - FirmwareInstallUnknown = "unknown" Unknown TaskState = "unknown" @@ -103,6 +99,12 @@ const ( // FirmwareInstallStepPowerOffHost indicates the host requires to be powered off. FirmwareInstallStepPowerOffHost FirmwareInstallStep = "power-off-host" + // FirmwareInstallStepResetBMCPostInstall indicates the BMC requires a reset after the install. + FirmwareInstallStepResetBMCPostInstall FirmwareInstallStep = "reset-bmc-post-install" + + // FirmwareInstallStepResetBMCOnInstallFailure indicates the BMC requires a reset if an install fails. + FirmwareInstallStepResetBMCOnInstallFailure FirmwareInstallStep = "reset-bmc-on-install-failure" + // device BIOS/UEFI POST code bmclib identifiers POSTStateBootINIT = "boot-init/pxe" POSTStateUEFI = "uefi" From 34128bb1010466564fdca05c3cdacca100bf213d Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Dec 2023 15:46:10 +0100 Subject: [PATCH 2/6] errors: ErrBMCUpdating is returned if the BMC is known to be going through an update --- errors/errors.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/errors/errors.go b/errors/errors.go index c1fe94b4..19c8c133 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -120,6 +120,9 @@ var ( // ErrRedfishNoSystems is returned when the API of the device provides and empty array of systems. ErrRedfishNoSystems = errors.New("redfish: no Systems were found on the device") + + // ErrBMCUpdating is returned when the BMC is going through an update and will not serve other queries. + ErrBMCUpdating = errors.New("a BMC firmware update is in progress") ) type ErrUnsupportedHardware struct { From 038acdd613d478ffa1a4f5507311507bfbd1b672 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Dec 2023 15:48:34 +0100 Subject: [PATCH 3/6] providers/asrockrack: update for newer Firmware interface methods - Fix up inventory collection on E3C256D4ID-NL --- providers/asrockrack/asrockrack.go | 63 ++++++++++++++++++------- providers/asrockrack/asrockrack_test.go | 12 ++--- providers/asrockrack/helpers.go | 8 ++-- providers/asrockrack/helpers_test.go | 4 +- providers/asrockrack/inventory.go | 17 ++++++- providers/asrockrack/inventory_test.go | 3 +- providers/asrockrack/mock_test.go | 5 -- 7 files changed, 72 insertions(+), 40 deletions(-) diff --git a/providers/asrockrack/asrockrack.go b/providers/asrockrack/asrockrack.go index a3d7fa95..c8f2238b 100644 --- a/providers/asrockrack/asrockrack.go +++ b/providers/asrockrack/asrockrack.go @@ -1,16 +1,19 @@ package asrockrack import ( - "bytes" "context" "crypto/x509" + "fmt" "net/http" + "strings" "github.com/bmc-toolbox/bmclib/v2/constants" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/providers" + "github.com/bmc-toolbox/common" "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" + "github.com/pkg/errors" ) const ( @@ -23,13 +26,17 @@ const ( var ( // Features implemented by asrockrack https Features = registrar.Features{ - providers.FeatureInventoryRead, - providers.FeatureFirmwareInstall, - providers.FeatureFirmwareInstallStatus, providers.FeaturePostCodeRead, providers.FeatureBmcReset, providers.FeatureUserCreate, providers.FeatureUserUpdate, + providers.FeatureFirmwareUpload, + providers.FeatureFirmwareInstallUploaded, + providers.FeatureFirmwareTaskStatus, + providers.FeatureFirmwareInstallSteps, + providers.FeatureInventoryRead, + providers.FeaturePowerSet, + providers.FeaturePowerState, } ) @@ -38,6 +45,7 @@ type ASRockRack struct { ip string username string password string + deviceModel string loginSession *loginSession httpClient *http.Client resetRequired bool // Indicates if the BMC requires a reset @@ -100,24 +108,45 @@ func (a *ASRockRack) Name() string { return ProviderName } -// Compatible implements the registrar.Verifier interface -// returns true if the BMC is identified to be an asrockrack -func (a *ASRockRack) Compatible(ctx context.Context) bool { - resp, statusCode, err := a.queryHTTPS(ctx, "/", "GET", nil, nil, 0) - if err != nil { - return false +// Open a connection to a BMC, implements the Opener interface +func (a *ASRockRack) Open(ctx context.Context) (err error) { + if err := a.httpsLogin(ctx); err != nil { + return err } - if statusCode != 200 { - return false + return a.supported(ctx) +} + +func (a *ASRockRack) supported(ctx context.Context) error { + supported := []string{ + "E3C256D4ID-NL", + "E3C246D4ID-NL", + "E3C246D4I-NL", } - return bytes.Contains(resp, []byte(`ASRockRack`)) -} + if a.deviceModel == "" { + device := common.NewDevice() + device.Metadata = map[string]string{} -// Open a connection to a BMC, implements the Opener interface -func (a *ASRockRack) Open(ctx context.Context) (err error) { - return a.httpsLogin(ctx) + err := a.fruAttributes(ctx, &device) + if err != nil { + return errors.Wrap(err, "failed to identify device model") + } + + if device.Model == "" { + return errors.Wrap(err, "failed to identify device model - empty model attribute") + } + + a.deviceModel = device.Model + } + + for _, s := range supported { + if strings.EqualFold(a.deviceModel, s) { + return nil + } + } + + return fmt.Errorf("device model not supported: %s", a.deviceModel) } // Close a connection to a BMC, implements the Closer interface diff --git a/providers/asrockrack/asrockrack_test.go b/providers/asrockrack/asrockrack_test.go index cc59d32e..9b450ed4 100644 --- a/providers/asrockrack/asrockrack_test.go +++ b/providers/asrockrack/asrockrack_test.go @@ -4,18 +4,12 @@ import ( "context" "os" "testing" + "time" "gopkg.in/go-playground/assert.v1" ) -func Test_Compatible(t *testing.T) { - b := aClient.Compatible(context.TODO()) - if !b { - t.Errorf("expected true, got false") - } -} - -func Test_httpLogin(t *testing.T) { +func TestHttpLogin(t *testing.T) { err := aClient.httpsLogin(context.TODO()) if err != nil { t.Errorf(err.Error()) @@ -24,7 +18,7 @@ func Test_httpLogin(t *testing.T) { assert.Equal(t, "l5L29IP7", aClient.loginSession.CSRFToken) } -func Test_Close(t *testing.T) { +func TestClose(t *testing.T) { err := aClient.httpsLogin(context.TODO()) if err != nil { t.Errorf(err.Error()) diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index c7f06532..37722f44 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -10,11 +10,11 @@ import ( "net/http" "net/http/httputil" "os" - "strings" "github.com/bmc-toolbox/bmclib/v2/constants" - "github.com/bmc-toolbox/bmclib/v2/errors" + brrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/common" + "github.com/pkg/errors" ) // API session setup response payload @@ -377,7 +377,7 @@ func (a *ASRockRack) postCodeInfo(ctx context.Context) (*biosPOSTCode, error) { } // Query the inventory info endpoint -func (a *ASRockRack) inventoryInfo(ctx context.Context) ([]*component, error) { +func (a *ASRockRack) inventoryInfoE3C246D41D(ctx context.Context) ([]*component, error) { resp, statusCode, err := a.queryHTTPS(ctx, "api/asrr/inventory_info", "GET", nil, nil, 0) if err != nil { return nil, err @@ -553,7 +553,7 @@ func (a *ASRockRack) httpsLogin(ctx context.Context) error { } if statusCode == 401 { - return errors.ErrLoginFailed + return brrs.ErrLoginFailed } // Unmarshal login session diff --git a/providers/asrockrack/helpers_test.go b/providers/asrockrack/helpers_test.go index a1b18fc7..83752b75 100644 --- a/providers/asrockrack/helpers_test.go +++ b/providers/asrockrack/helpers_test.go @@ -32,13 +32,13 @@ func Test_FirmwareInfo(t *testing.T) { assert.Equal(t, expected, fwInfo) } -func Test_inventoryInfo(t *testing.T) { +func TestInventoryInfo(t *testing.T) { err := aClient.httpsLogin(context.TODO()) if err != nil { t.Errorf(err.Error()) } - inventory, err := aClient.inventoryInfo(context.TODO()) + inventory, err := aClient.inventoryInfoE3C246D41D(context.TODO()) if err != nil { t.Fatal(err.Error()) } diff --git a/providers/asrockrack/inventory.go b/providers/asrockrack/inventory.go index 5b0f2b3c..2f2044e7 100644 --- a/providers/asrockrack/inventory.go +++ b/providers/asrockrack/inventory.go @@ -28,9 +28,12 @@ func (a *ASRockRack) Inventory(ctx context.Context) (device *common.Device, err } // populate device health based on sensor readings + // + // sensor data collection can fail for a myriad of reasons + // we log the error and keep going err = a.systemHealth(ctx, device) if err != nil { - return nil, err + a.log.V(2).Error(err, "sensor data collection error", "deviceModel", a.deviceModel) } return device, nil @@ -139,7 +142,17 @@ func (a *ASRockRack) systemAttributes(ctx context.Context, device *common.Device device.Metadata["node_id"] = fwInfo.NodeID - components, err := a.inventoryInfo(ctx) + switch device.Model { + case "E3C246D4ID-NL", "E3C246D4I-NL": + return a.componentAttributesE3C246(ctx, fwInfo, device) + default: + return nil + } +} + +func (a *ASRockRack) componentAttributesE3C246(ctx context.Context, fwInfo *firmwareInfo, device *common.Device) error { + // TODO: implement newer device inventory + components, err := a.inventoryInfoE3C246D41D(ctx) if err != nil { return err } diff --git a/providers/asrockrack/inventory_test.go b/providers/asrockrack/inventory_test.go index 5e7302cb..56e01895 100644 --- a/providers/asrockrack/inventory_test.go +++ b/providers/asrockrack/inventory_test.go @@ -7,12 +7,13 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_GetInventory(t *testing.T) { +func TestGetInventory(t *testing.T) { device, err := aClient.Inventory(context.TODO()) if err != nil { t.Fatal(err) } + aClient.deviceModel = "E3C246D4I-NL" assert.NotNil(t, device) assert.Equal(t, "ASRockRack", device.Vendor) assert.Equal(t, "E3C246D4I-NL", device.Model) diff --git a/providers/asrockrack/mock_test.go b/providers/asrockrack/mock_test.go index 091697ed..b471f853 100644 --- a/providers/asrockrack/mock_test.go +++ b/providers/asrockrack/mock_test.go @@ -307,11 +307,6 @@ func session(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusBadRequest) } case "DELETE": - //1for h, values := range r.Header { - //1 for _, v := range values { - //1 fmt.Println(h, v) - //1 } - //1} if r.Header.Get("X-Csrftoken") != "l5L29IP7" { w.WriteHeader(http.StatusBadRequest) } From 7884aedaf263c154c38dca46a9557c7983b1af86 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Dec 2023 15:50:16 +0100 Subject: [PATCH 4/6] providers/asrockrack: rework firmware install methods for newer interface --- providers/asrockrack/asrockrack_test.go | 7 +- providers/asrockrack/firmware.go | 207 ++++++++++++++---------- providers/asrockrack/helpers.go | 21 ++- 3 files changed, 145 insertions(+), 90 deletions(-) diff --git a/providers/asrockrack/asrockrack_test.go b/providers/asrockrack/asrockrack_test.go index 9b450ed4..755207b7 100644 --- a/providers/asrockrack/asrockrack_test.go +++ b/providers/asrockrack/asrockrack_test.go @@ -30,7 +30,7 @@ func TestClose(t *testing.T) { } } -func Test_FirwmwareUpdateBMC(t *testing.T) { +func TestFirwmwareUpdateBMC(t *testing.T) { err := aClient.httpsLogin(context.TODO()) if err != nil { t.Errorf(err.Error()) @@ -48,7 +48,10 @@ func Test_FirwmwareUpdateBMC(t *testing.T) { } defer fh.Close() - err = aClient.firmwareInstallBMC(context.TODO(), fh, 0) + ctx, cancel := context.WithTimeout(context.TODO(), time.Minute*15) + defer cancel() + + err = aClient.firmwareUploadBMC(ctx, fh) if err != nil { t.Errorf(err.Error()) } diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index f5d80ccf..817f6273 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -2,9 +2,10 @@ package asrockrack import ( "context" - "io" + "fmt" "os" "strings" + "time" "github.com/pkg/errors" @@ -21,125 +22,167 @@ const ( versionStrEmpty = 2 ) -// FirmwareInstall uploads and initiates firmware update for the component -func (a *ASRockRack) FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader) (jobID string, err error) { - var size int64 - if file, ok := reader.(*os.File); ok { - finfo, err := file.Stat() - if err != nil { - a.log.V(2).Error(err, "unable to determine file size") - } - - size = finfo.Size() +// bmc client interface implementations methods +func (a *ASRockRack) FirmwareInstallSteps(ctx context.Context, component string) ([]constants.FirmwareInstallStep, error) { + switch strings.ToUpper(component) { + case common.SlugBMC: + return []constants.FirmwareInstallStep{ + constants.FirmwareInstallStepUpload, + constants.FirmwareInstallStepInstallUploaded, + constants.FirmwareInstallStepInstallStatus, + constants.FirmwareInstallStepResetBMCPostInstall, + constants.FirmwareInstallStepResetBMCOnInstallFailure, + }, nil } - component = strings.ToUpper(component) - switch component { + return nil, errors.Wrap(bmclibErrs.ErrFirmwareUpload, "component unsupported: "+component) +} + +func (a *ASRockRack) FirmwareUpload(ctx context.Context, component string, file *os.File) (taskID string, err error) { + switch strings.ToUpper(component) { case common.SlugBIOS: - err = a.firmwareInstallBIOS(ctx, reader, size) + return "", a.firmwareUploadBIOS(ctx, file) case common.SlugBMC: - err = a.firmwareInstallBMC(ctx, reader, size) - default: - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "component unsupported: "+component) + return "", a.firmwareUploadBMC(ctx, file) } - if err != nil { - err = errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - } + return "", errors.Wrap(bmclibErrs.ErrFirmwareUpload, "component unsupported: "+component) - return jobID, err } -// FirmwareInstallStatus returns the status of the firmware install process, a bool value indicating if the component requires a reset -func (a *ASRockRack) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (status string, err error) { - component = strings.ToUpper(component) - switch component { - case common.SlugBIOS, common.SlugBMC: - return a.firmwareUpdateStatus(ctx, component, installVersion) - default: - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "component unsupported: "+component) +func (a *ASRockRack) firmwareUploadBMC(ctx context.Context, file *os.File) error { + // // expect atleast 5 minutes left in the deadline to proceed with the upload + d, _ := ctx.Deadline() + if time.Until(d) < 5*time.Minute { + return errors.New("remaining context deadline insufficient to perform update: " + time.Until(d).String()) } -} - -// firmwareInstallBMC uploads and installs firmware for the BMC component -func (a *ASRockRack) firmwareInstallBMC(ctx context.Context, reader io.Reader, fileSize int64) error { - var err error - // 0. take the model so that we use a different endpoint on E3C256D4ID-NL - device := common.NewDevice() - device.Metadata = map[string]string{} - err = a.fruAttributes(ctx, &device) - if err != nil { - return errors.Wrap(err, "failed to get model in step 0/4") - } - - // 1. set the device to flash mode - prepares the flash // Beware: this locks some capabilities, e.g. the access to fruAttributes a.log.V(2).WithValues("step", "1/4").Info("set device to flash mode, takes a minute...") - err = a.setFlashMode(ctx) + err := a.setFlashMode(ctx) if err != nil { - return errors.Wrap(err, "failed in step 1/4 - set device to flash mode") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 1/3 - set device to flash mode: "+err.Error(), + ) } - // 2. upload firmware image file - fwEndpoint := "api/maintenance/firmware" + var fwEndpoint string + switch a.deviceModel { // E3C256D4ID-NL calls a different endpoint for firmware upload - if strings.EqualFold(device.Model, "E3C256D4ID-NL") { + case "E3C256D4ID-NL": fwEndpoint = "api/maintenance/firmware/firmware" + default: + fwEndpoint = "api/maintenance/firmware" } + a.log.V(2).WithValues("step", "2/4").Info("upload BMC firmware image to " + fwEndpoint) - err = a.uploadFirmware(ctx, fwEndpoint, reader, fileSize) + err = a.uploadFirmware(ctx, fwEndpoint, file) if err != nil { - return errors.Wrap(err, "failed in step 2/4 - upload BMC firmware image") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 2/3 - upload BMC firmware image: "+err.Error(), + ) } - // 3. BMC to verify the uploaded file - err = a.verifyUploadedFirmware(ctx) a.log.V(2).WithValues("step", "3/4").Info("verify uploaded BMC firmware") + err = a.verifyUploadedFirmware(ctx) if err != nil { - return errors.Wrap(err, "failed in step 3/4 - verify uploaded BMC firmware") - } - - // 4. Run the upgrade - preserving current config - a.log.V(2).WithValues("step", "4/4").Info("proceed with BMC firmware install, preserve current configuration") - err = a.upgradeBMC(ctx) - if err != nil { - return errors.Wrap(err, "failed in step 4/4 - proceed with BMC firmware install") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 3/3 - verify uploaded BMC firmware: "+err.Error(), + ) } return nil } -// firmwareInstallBIOS uploads and installs firmware for the BIOS component -func (a *ASRockRack) firmwareInstallBIOS(ctx context.Context, reader io.Reader, fileSize int64) error { - var err error - - // 1. upload firmware image file +func (a *ASRockRack) firmwareUploadBIOS(ctx context.Context, file *os.File) error { a.log.V(2).WithValues("step", "1/3").Info("upload BIOS firmware image") - err = a.uploadFirmware(ctx, "api/asrr/maintenance/BIOS/firmware", reader, fileSize) + err := a.uploadFirmware(ctx, "api/asrr/maintenance/BIOS/firmware", file) if err != nil { - return errors.Wrap(err, "failed in step 1/3 - upload BIOS firmware image") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 1/3 - upload BIOS firmware image: "+err.Error(), + ) } - // 2. set update parameters to preserve configurations a.log.V(2).WithValues("step", "2/3").Info("set BIOS preserve flash configuration") err = a.biosUpgradeConfiguration(ctx) if err != nil { - return errors.Wrap(err, "failed in step 2/3 - set flash configuration") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 2/3 - set flash configuration: "+err.Error(), + ) } // 3. run upgrade a.log.V(2).WithValues("step", "3/3").Info("proceed with BIOS firmware install") err = a.upgradeBIOS(ctx) if err != nil { - return errors.Wrap(err, "failed in step 3/3 - proceed with BIOS firmware install") + return errors.Wrap( + bmclibErrs.ErrFirmwareUpload, + "failed in step 3/3 - proceed with BIOS firmware install: "+err.Error(), + ) } return nil } +func (a *ASRockRack) FirmwareInstallUploaded(ctx context.Context, component, uploadTaskID string) (installTaskID string, err error) { + switch strings.ToUpper(component) { + case common.SlugBIOS: + return "", a.firmwareInstallUploadedBIOS(ctx) + case common.SlugBMC: + return "", a.firmwareInstallUploadedBMC(ctx) + } + + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "component unsupported: "+component) +} + +// firmwareInstallUploadedBIOS uploads and installs firmware for the BMC component +func (a *ASRockRack) firmwareInstallUploadedBIOS(ctx context.Context) error { + // 4. Run the upgrade - preserving current config + a.log.V(2).WithValues("step", "install").Info("proceed with BIOS firmware install, preserve current configuration") + err := a.upgradeBIOS(ctx) + if err != nil { + return errors.Wrap( + bmclibErrs.ErrFirmwareInstallUploaded, + "failed in step 4/4 - proceed with BMC firmware install: "+err.Error(), + ) + } + + return nil +} + +// firmwareInstallUploadedBMC uploads and installs firmware for the BMC component +func (a *ASRockRack) firmwareInstallUploadedBMC(ctx context.Context) error { + // 4. Run the upgrade - preserving current config + a.log.V(2).WithValues("step", "install").Info("proceed with BMC firmware install, preserve current configuration") + err := a.upgradeBMC(ctx) + if err != nil { + return errors.Wrap( + bmclibErrs.ErrFirmwareInstallUploaded, + "failed in step 4/4 - proceed with BMC firmware install"+err.Error(), + ) + } + + return nil +} + +// FirmwareTaskStatus returns the status of a firmware related task queued on the BMC. +func (a *ASRockRack) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) { + component = strings.ToUpper(component) + switch component { + case common.SlugBIOS, common.SlugBMC: + return a.firmwareUpdateStatus(ctx, component, installVersion) + default: + return "", "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "component unsupported: "+component) + } +} + // firmwareUpdateBIOSStatus returns the BIOS firmware install status -func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, installVersion string) (status string, err error) { +func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, installVersion string) (state constants.TaskState, status string, err error) { var endpoint string component = strings.ToUpper(component) switch component { @@ -148,7 +191,7 @@ func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, case common.SlugBMC: endpoint = "api/maintenance/firmware/flash-progress" default: - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "component unsupported: "+component) + return "", "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "component unsupported: "+component) } // 1. query the flash progress endpoint @@ -160,13 +203,15 @@ func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, } if progress != nil { + status = fmt.Sprintf("action: %s, progress: %s", progress.Action, progress.Progress) + switch progress.State { case 0: - return constants.FirmwareInstallRunning, nil + return constants.Running, status, nil case 1: // "Flashing To be done" - return constants.FirmwareInstallQueued, nil + return constants.Queued, status, nil case 2: - return constants.FirmwareInstallComplete, nil + return constants.Complete, status, nil default: a.log.V(3).WithValues("state", progress.State).Info("warn", "bmc returned unknown flash progress state") } @@ -179,7 +224,7 @@ func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, installStatus, err = a.versionInstalled(ctx, component, installVersion) if err != nil { - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, err.Error()) + return "", "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, err.Error()) } switch installStatus { @@ -188,17 +233,17 @@ func (a *ASRockRack) firmwareUpdateStatus(ctx context.Context, component string, // TODO: we should pass the force parameter to firmwareUpdateStatus, // so that we can know if we expect a version change or not a.log.V(3).Info("Nil progress + no version change -> unknown") - return constants.FirmwareInstallUnknown, nil + return constants.Unknown, status, nil } - return constants.FirmwareInstallComplete, nil + return constants.Complete, status, nil case versionStrEmpty: - return constants.FirmwareInstallUnknown, nil + return constants.Unknown, status, nil case versionStrMismatch: - return constants.FirmwareInstallRunning, nil + return constants.Running, status, nil } - return constants.FirmwareInstallUnknown, nil + return constants.Unknown, status, nil } // versionInstalled returns int values on the status of the firmware version install diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index 37722f44..a550f253 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -189,7 +189,8 @@ func (a *ASRockRack) setFlashMode(ctx context.Context) error { pConfig := &preserveConfig{} // preserve config is needed by e3c256d4i - if strings.EqualFold(device.Model, "E3C256D4ID-NL") { + switch device.Model { + case "E3C256D4ID-NL": pConfig = &preserveConfig{PreserveConfig: 1} } @@ -222,14 +223,20 @@ func multipartSize(fieldname, filename string) int64 { } // 2 Upload the firmware file -func (a *ASRockRack) uploadFirmware(ctx context.Context, endpoint string, fwReader io.Reader, fileSize int64) error { +func (a *ASRockRack) uploadFirmware(ctx context.Context, endpoint string, file *os.File) error { + var size int64 + finfo, err := file.Stat() + if err != nil { + return errors.Wrap(err, "unable to determine file size") + } + + size = finfo.Size() + fieldName, fileName := "fwimage", "image" - contentLength := multipartSize(fieldName, fileName) + fileSize + contentLength := multipartSize(fieldName, fileName) + size // Before reading the file, rewind to the beginning - if file, ok := fwReader.(*os.File); ok { - _, _ = file.Seek(0, 0) - } + _, _ = file.Seek(0, 0) // setup pipe pipeReader, pipeWriter := io.Pipe() @@ -250,7 +257,7 @@ func (a *ASRockRack) uploadFirmware(ctx context.Context, endpoint string, fwRead } // copy from source into form part writer - _, err = io.Copy(part, fwReader) + _, err = io.Copy(part, file) if err != nil { errCh <- err return From 905a95485261e7e07fe0ff85324028ee0caf5f5e Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Dec 2023 15:52:10 +0100 Subject: [PATCH 5/6] providers/asrockrack: GetPowerState() return ErrBMCUpdating when the BMC is under and update --- providers/asrockrack/helpers.go | 6 ++++-- providers/asrockrack/power.go | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index a550f253..3166614b 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -180,8 +180,10 @@ func (a *ASRockRack) createUpdateUser(ctx context.Context, account *UserAccount) } // 1 Set BMC to flash mode and prepare flash area -// at this point all logged in sessions are terminated -// and no logins are permitted +// +// with the BMC set in flash mode, no new logins are accepted +// and only a few endpoints can be queried with the existing session +// one of the few being the install progress/flash status endpoint. func (a *ASRockRack) setFlashMode(ctx context.Context) error { device := common.NewDevice() device.Metadata = map[string]string{} diff --git a/providers/asrockrack/power.go b/providers/asrockrack/power.go index d8a8efa1..b94f5a19 100644 --- a/providers/asrockrack/power.go +++ b/providers/asrockrack/power.go @@ -20,6 +20,30 @@ type power struct { func (a *ASRockRack) PowerStateGet(ctx context.Context) (state string, err error) { info, err := a.chassisStatusInfo(ctx) if err != nil { + if strings.Contains(err.Error(), "401") { + // during a BMC update, only the flash-progress endpoint can be queried + // and so we cannot determine server power status + // we don't return an error here because we don't want the bmclib client to retry another provider. + progress, err := a.flashProgress(ctx, "/api/maintenance/firmware/flash-progress") + if err == nil && progress.Action != "" { + a.log.V(2).WithValues( + "action", progress.Action, + "progress", progress.Progress, + "state", progress.State, + ).Info("bmc in flash mode, power status cannot be determined") + + return "", errors.Wrap( + bmclibErrs.ErrBMCUpdating, + fmt.Sprintf( + "action: %s, progress: %s, state: %d", + progress.Action, + progress.Progress, + progress.State, + ), + ) + } + } + return "", errors.Wrap(bmclibErrs.ErrPowerStatusRead, err.Error()) } @@ -105,7 +129,8 @@ func (a *ASRockRack) resetBMC(ctx context.Context) error { return err } - if statusCode != http.StatusOK { + // The E3C256D4ID BMC returns a 500 status error on the BMC reset request + if statusCode != http.StatusOK && statusCode != http.StatusInternalServerError { return fmt.Errorf("non 200 response: %d", statusCode) } From 83402890df4a46b7f272ae4bcf5069b850bababe Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Tue, 5 Dec 2023 09:29:30 +0100 Subject: [PATCH 6/6] providers/asrr: use constants for the model names --- providers/asrockrack/asrockrack.go | 10 +++++++--- providers/asrockrack/helpers.go | 2 +- providers/asrockrack/inventory.go | 2 +- providers/asrockrack/inventory_test.go | 4 ++-- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/providers/asrockrack/asrockrack.go b/providers/asrockrack/asrockrack.go index c8f2238b..721c10dc 100644 --- a/providers/asrockrack/asrockrack.go +++ b/providers/asrockrack/asrockrack.go @@ -21,6 +21,10 @@ const ( ProviderName = "asrockrack" // ProviderProtocol for the provider implementation ProviderProtocol = "vendorapi" + + E3C256D4ID_NL = "E3C256D4ID-NL" + E3C246D4ID_NL = "E3C246D4ID-NL" + E3C246D4I_NL = "E3C246D4I-NL" ) var ( @@ -119,9 +123,9 @@ func (a *ASRockRack) Open(ctx context.Context) (err error) { func (a *ASRockRack) supported(ctx context.Context) error { supported := []string{ - "E3C256D4ID-NL", - "E3C246D4ID-NL", - "E3C246D4I-NL", + E3C256D4ID_NL, + E3C246D4ID_NL, + E3C246D4I_NL, } if a.deviceModel == "" { diff --git a/providers/asrockrack/helpers.go b/providers/asrockrack/helpers.go index 3166614b..503bda88 100644 --- a/providers/asrockrack/helpers.go +++ b/providers/asrockrack/helpers.go @@ -192,7 +192,7 @@ func (a *ASRockRack) setFlashMode(ctx context.Context) error { pConfig := &preserveConfig{} // preserve config is needed by e3c256d4i switch device.Model { - case "E3C256D4ID-NL": + case E3C256D4ID_NL: pConfig = &preserveConfig{PreserveConfig: 1} } diff --git a/providers/asrockrack/inventory.go b/providers/asrockrack/inventory.go index 2f2044e7..d571a241 100644 --- a/providers/asrockrack/inventory.go +++ b/providers/asrockrack/inventory.go @@ -143,7 +143,7 @@ func (a *ASRockRack) systemAttributes(ctx context.Context, device *common.Device device.Metadata["node_id"] = fwInfo.NodeID switch device.Model { - case "E3C246D4ID-NL", "E3C246D4I-NL": + case E3C246D4ID_NL, E3C246D4I_NL: return a.componentAttributesE3C246(ctx, fwInfo, device) default: return nil diff --git a/providers/asrockrack/inventory_test.go b/providers/asrockrack/inventory_test.go index 56e01895..8d7e1c3a 100644 --- a/providers/asrockrack/inventory_test.go +++ b/providers/asrockrack/inventory_test.go @@ -13,10 +13,10 @@ func TestGetInventory(t *testing.T) { t.Fatal(err) } - aClient.deviceModel = "E3C246D4I-NL" + aClient.deviceModel = E3C246D4I_NL assert.NotNil(t, device) assert.Equal(t, "ASRockRack", device.Vendor) - assert.Equal(t, "E3C246D4I-NL", device.Model) + assert.Equal(t, E3C246D4I_NL, device.Model) assert.Equal(t, "L2.07B", device.BIOS.Firmware.Installed) assert.Equal(t, "0.01.00", device.BMC.Firmware.Installed)