Skip to content

Commit

Permalink
Implement changes based on feedback from GH PR#261
Browse files Browse the repository at this point in the history
  • Loading branch information
@joelrebel
joelrebel committed Apr 22, 2022
1 parent ed5dfbb commit aac60b0
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 110 deletions.
51 changes: 26 additions & 25 deletions bmc/firmware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,23 @@ import (

bmclibErrs "github.com/bmc-toolbox/bmclib/errors"

"github.com/pkg/errors"

"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
)

// FirmwareInstaller defines an interface to install firmware updates
type FirmwareInstaller interface {
// FirmwareInstall uploads firmware update payload to the BMC returning the task ID
FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader) (jobID string, err error)
//
// parameters:
// component - the component slug for the component update being installed.
// applyAt - one of "Immediate", "OnReset".
// forceInstall - purge the install task queued/scheduled firmware install BMC task (if any).
// reader - the io.reader to the firmware update file.
//
// return values:
// taskID - A taskID is returned if the update process on the BMC returns an identifier for the update process.
FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader) (taskID string, err error)
}

// firmwareInstallerProvider is an internal struct to correlate an implementation/provider and its name
Expand All @@ -24,17 +32,8 @@ type firmwareInstallerProvider struct {
FirmwareInstaller
}

// FirmwareInstall uploads and initiates firmware update for the component
//
// parameters:
// component - the component slug for the component update being installed
// applyAt - one of "Immediate", "OnReset"
// forceInstall - purge the install task queued/scheduled firmware install BMC task (if any)
// reader - the io.reader to the firmware update file
//
// return values:
// taskID - A taskID is returned if the update process on the BMC returns an identifier for the update process
func FirmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader, generic []firmwareInstallerProvider) (taskID string, metadata Metadata, err error) {
// firmwareInstall uploads and initiates firmware update for the component
func firmwareInstall(ctx context.Context, component, applyAt string, forceInstall bool, reader io.Reader, generic []firmwareInstallerProvider) (taskID string, metadata Metadata, err error) {
var metadataLocal Metadata
Loop:
for _, elem := range generic {
Expand Down Expand Up @@ -86,11 +85,20 @@ func FirmwareInstallFromInterfaces(ctx context.Context, component, applyAt strin
)
}

return FirmwareInstall(ctx, component, applyAt, forceInstall, reader, implementations)
return firmwareInstall(ctx, component, applyAt, forceInstall, reader, implementations)
}

// FirmwareInstallVerifier defines an interface to check firmware install status
type FirmwareInstallVerifier interface {
// FirmwareInstallStatus returns the status of the firmware install process.
//
// parameters:
// component (optional) - the component slug for the component update being installed.
// installVersion (required) - the version this method should check is installed.
// taskID (optional) - the task identifier.
//
// return values:
// status - returns one of the FirmwareInstall statuses (see devices/constants.go).
FirmwareInstallStatus(ctx context.Context, component, installVersion, taskID string) (status string, err error)
}

Expand All @@ -100,15 +108,8 @@ type firmwareInstallVerifierProvider struct {
FirmwareInstallVerifier
}

// FirmwareInstallStatus returns the status of the firmware install process
//
// parameters:
// component (optional) - the component slug for the component update being installed
// taskID (optional) - the task identifier
//
// return values:
// status - returns one of the FirmwareInstall statuses (see devices/constants.go)
func FirmwareInstallStatus(ctx context.Context, component, installVersion, taskID string, generic []firmwareInstallVerifierProvider) (status string, metadata Metadata, err error) {
// firmwareInstallStatus returns the status of the firmware install process
func firmwareInstallStatus(ctx context.Context, component, installVersion, taskID string, generic []firmwareInstallVerifierProvider) (status string, metadata Metadata, err error) {
var metadataLocal Metadata
Loop:
for _, elem := range generic {
Expand Down Expand Up @@ -160,5 +161,5 @@ func FirmwareInstallStatusFromInterfaces(ctx context.Context, component, install
)
}

return FirmwareInstallStatus(ctx, component, installVersion, taskID, implementations)
return firmwareInstallStatus(ctx, component, installVersion, taskID, implementations)
}
4 changes: 2 additions & 2 deletions bmc/firmware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestFirmwareInstall(t *testing.T) {
}
ctx, cancel := context.WithTimeout(context.Background(), tc.ctxTimeout)
defer cancel()
taskID, metadata, err := FirmwareInstall(ctx, tc.component, tc.applyAt, tc.forceInstall, tc.reader, []firmwareInstallerProvider{{tc.providerName, &testImplementation}})
taskID, metadata, err := firmwareInstall(ctx, tc.component, tc.applyAt, tc.forceInstall, tc.reader, []firmwareInstallerProvider{{tc.providerName, &testImplementation}})
if tc.returnError != nil {
assert.ErrorIs(t, err, tc.returnError)
return
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestFirmwareInstallStatus(t *testing.T) {
}
ctx, cancel := context.WithTimeout(context.Background(), tc.ctxTimeout)
defer cancel()
taskID, metadata, err := FirmwareInstallStatus(ctx, tc.component, tc.installVersion, tc.taskID, []firmwareInstallVerifierProvider{{tc.providerName, &testImplementation}})
taskID, metadata, err := firmwareInstallStatus(ctx, tc.component, tc.installVersion, tc.taskID, []firmwareInstallVerifierProvider{{tc.providerName, &testImplementation}})
if tc.returnError != nil {
assert.ErrorIs(t, err, tc.returnError)
return
Expand Down
6 changes: 3 additions & 3 deletions bmc/inventory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ type inventoryGetterProvider struct {
InventoryGetter
}

// Inventory returns hardware and firmware inventory
func Inventory(ctx context.Context, generic []inventoryGetterProvider) (device *devices.Device, metadata Metadata, err error) {
// inventory returns hardware and firmware inventory
func inventory(ctx context.Context, generic []inventoryGetterProvider) (device *devices.Device, metadata Metadata, err error) {
var metadataLocal Metadata
Loop:
for _, elem := range generic {
Expand Down Expand Up @@ -74,5 +74,5 @@ func GetInventoryFromInterfaces(ctx context.Context, generic []interface{}) (dev
)
}

return Inventory(ctx, implementations)
return inventory(ctx, implementations)
}
2 changes: 1 addition & 1 deletion bmc/inventory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestInventory(t *testing.T) {
}
ctx, cancel := context.WithTimeout(context.Background(), tc.ctxTimeout)
defer cancel()
device, metadata, err := Inventory(ctx, []inventoryGetterProvider{{tc.providerName, &testImplementation}})
device, metadata, err := inventory(ctx, []inventoryGetterProvider{{tc.providerName, &testImplementation}})
if tc.returnError != nil {
assert.ErrorIs(t, err, tc.returnError)
return
Expand Down
6 changes: 3 additions & 3 deletions bmc/postcode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ type postCodeGetterProvider struct {
PostCodeGetter
}

// PostCode returns the device BIOS/UEFI POST code
func PostCode(ctx context.Context, generic []postCodeGetterProvider) (status string, code int, metadata Metadata, err error) {
// postCode returns the device BIOS/UEFI POST code
func postCode(ctx context.Context, generic []postCodeGetterProvider) (status string, code int, metadata Metadata, err error) {
var metadataLocal Metadata
Loop:
for _, elem := range generic {
Expand Down Expand Up @@ -76,5 +76,5 @@ func GetPostCodeInterfaces(ctx context.Context, generic []interface{}) (status s
)
}

return PostCode(ctx, implementations)
return postCode(ctx, implementations)
}
2 changes: 1 addition & 1 deletion bmc/postcode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestPostCode(t *testing.T) {
}
ctx, cancel := context.WithTimeout(context.Background(), tc.ctxTimeout)
defer cancel()
status, code, metadata, err := PostCode(ctx, []postCodeGetterProvider{{tc.providerName, &testImplementation}})
status, code, metadata, err := postCode(ctx, []postCodeGetterProvider{{tc.providerName, &testImplementation}})
if tc.returnError != nil {
assert.ErrorIs(t, err, tc.returnError)
return
Expand Down
14 changes: 2 additions & 12 deletions providers/asrockrack/firmware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
bmclibErrs "github.com/bmc-toolbox/bmclib/errors"
)



// 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
Expand Down Expand Up @@ -118,10 +116,7 @@ func (a *ASRockRack) firmwareInstallBIOS(ctx context.Context, reader io.Reader,

// firmwareUpdateBMCStatus returns the BMC firmware install status
func (a *ASRockRack) firmwareUpdateBMCStatus(ctx context.Context, installVersion string) (status string, err error) {
os.Setenv("BMCLIB_LOG_LEVEL", "trace")
defer os.Unsetenv("BMCLIB_LOG_LEVEL")
endpoint := "api/maintenance/firmware/flash-progress"
p, progressErr := a.flashProgress(ctx, endpoint)
p, progressErr := a.flashProgress(ctx, "api/maintenance/firmware/flash-progress")
if progressErr != nil {
installed, versionErr := a.versionInstalled(ctx, devices.SlugBMC, installVersion)
if err != nil {
Expand Down Expand Up @@ -160,11 +155,7 @@ func (a *ASRockRack) firmwareUpdateBMCStatus(ctx context.Context, installVersion

// firmwareUpdateBIOSStatus returns the BIOS firmware install status
func (a *ASRockRack) firmwareUpdateBIOSStatus(ctx context.Context, installVersion string) (status string, err error) {
os.Setenv("BMCLIB_LOG_LEVEL", "trace")
defer os.Unsetenv("BMCLIB_LOG_LEVEL")

endpoint := "api/asrr/maintenance/BIOS/flash-progress"
p, progressErr := a.flashProgress(ctx, endpoint)
p, progressErr := a.flashProgress(ctx, "api/asrr/maintenance/BIOS/flash-progress")
if progressErr != nil {
installed, versionErr := a.versionInstalled(ctx, devices.SlugBIOS, installVersion)
if versionErr != nil {
Expand Down Expand Up @@ -223,7 +214,6 @@ func (a *ASRockRack) versionInstalled(ctx context.Context, component, version st
}

if strings.EqualFold(installed, version) {
fmt.Println("YEP!")
return true, nil
}

Expand Down
40 changes: 10 additions & 30 deletions providers/asrockrack/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,7 @@ var (
)

func (a *ASRockRack) listUsers(ctx context.Context) ([]*UserAccount, error) {
endpoint := "api/settings/users"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "api/settings/users", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -184,9 +182,7 @@ func (a *ASRockRack) createUpdateUser(ctx context.Context, account *UserAccount)
// at this point all logged in sessions are terminated
// and no logins are permitted
func (a *ASRockRack) setFlashMode(ctx context.Context) error {
endpoint := "api/maintenance/flash"

_, statusCode, err := a.queryHTTPS(ctx, endpoint, "PUT", nil, nil, 0)
_, statusCode, err := a.queryHTTPS(ctx, "api/maintenance/flash", "PUT", nil, nil, 0)
if err != nil {
return err
}
Expand Down Expand Up @@ -262,9 +258,7 @@ func (a *ASRockRack) uploadFirmware(ctx context.Context, endpoint string, fwRead

// 3. Verify uploaded firmware file - to be invoked after uploadFirmware()
func (a *ASRockRack) verifyUploadedFirmware(ctx context.Context) error {
endpoint := "api/maintenance/firmware/verification"

_, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
_, statusCode, err := a.queryHTTPS(ctx, "api/maintenance/firmware/verification", "GET", nil, nil, 0)
if err != nil {
return err
}
Expand Down Expand Up @@ -322,9 +316,7 @@ func (a *ASRockRack) flashProgress(ctx context.Context, endpoint string) (*upgra

// Query firmware information from the BMC
func (a *ASRockRack) firmwareInfo(ctx context.Context) (*firmwareInfo, error) {
endpoint := "api/asrr/fw-info"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "api/asrr/fw-info", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand All @@ -344,9 +336,7 @@ func (a *ASRockRack) firmwareInfo(ctx context.Context) (*firmwareInfo, error) {

// Query BIOS/UEFI POST code information from the BMC
func (a *ASRockRack) postCodeInfo(ctx context.Context) (*biosPOSTCode, error) {
endpoint := "/api/asrr/getbioscode"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "/api/asrr/getbioscode", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand All @@ -366,9 +356,7 @@ func (a *ASRockRack) postCodeInfo(ctx context.Context) (*biosPOSTCode, error) {

// Query the inventory info endpoint
func (a *ASRockRack) inventoryInfo(ctx context.Context) ([]*component, error) {
endpoint := "api/asrr/inventory_info"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "api/asrr/inventory_info", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand All @@ -388,9 +376,7 @@ func (a *ASRockRack) inventoryInfo(ctx context.Context) ([]*component, error) {

// Query the fru info endpoint
func (a *ASRockRack) fruInfo(ctx context.Context) ([]*fru, error) {
endpoint := "api/fru"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "api/fru", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -436,9 +422,7 @@ func (a *ASRockRack) fruInfo(ctx context.Context) ([]*fru, error) {

// Query the sensors endpoint
func (a *ASRockRack) sensors(ctx context.Context) ([]*sensor, error) {
endpoint := "api/sensors"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "api/sensors", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -519,9 +503,7 @@ func (a *ASRockRack) upgradeBIOS(ctx context.Context) error {

// Returns the chassis status object which includes the power state
func (a *ASRockRack) chassisStatusInfo(ctx context.Context) (*chassisStatus, error) {
endpoint := "/api/chassis-status"

resp, statusCode, err := a.queryHTTPS(ctx, endpoint, "GET", nil, nil, 0)
resp, statusCode, err := a.queryHTTPS(ctx, "/api/chassis-status", "GET", nil, nil, 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -573,9 +555,7 @@ func (a *ASRockRack) httpsLogin(ctx context.Context) error {

// Close ends the BMC session
func (a *ASRockRack) httpsLogout(ctx context.Context) error {
urlEndpoint := "api/session"

_, statusCode, err := a.queryHTTPS(ctx, urlEndpoint, "DELETE", nil, nil, 0)
_, statusCode, err := a.queryHTTPS(ctx, "api/session", "DELETE", nil, nil, 0)
if err != nil {
return fmt.Errorf("Error logging out: " + err.Error())
}
Expand Down
Loading

0 comments on commit aac60b0

Please sign in to comment.