From 901dfdfea8a15cf7b24ed417a3b151aecf3ff496 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 20 Nov 2023 15:30:42 +0100 Subject: [PATCH 1/5] bmc: Adds a TaskState constant type which is returned by FirmwareTaskStatus Update the FirmwareTaskStatus method to return the the TaskState constant. This is to remove the FirmwareInstall prefix from the current FirmwareInstall* constants and to have a generic Task State type. --- bmc/firmware.go | 6 +++--- bmc/firmware_test.go | 20 ++++++++++---------- client.go | 2 +- constants/constants.go | 29 ++++++++++++++++++----------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/bmc/firmware.go b/bmc/firmware.go index b139e488..c6fb44ec 100644 --- a/bmc/firmware.go +++ b/bmc/firmware.go @@ -406,7 +406,7 @@ type FirmwareTaskVerifier interface { // return values: // state - returns one of the FirmwareTask statuses (see devices/constants.go). // status - returns firmware task progress or other arbitrary task information. - FirmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string) (state string, status string, err error) + FirmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) } // firmwareTaskVerifierProvider is an internal struct to correlate an implementation/provider and its name @@ -416,7 +416,7 @@ type firmwareTaskVerifierProvider struct { } // firmwareTaskStatus returns the status of the firmware upload process. -func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []firmwareTaskVerifierProvider) (state, status string, metadata Metadata, err error) { +func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []firmwareTaskVerifierProvider) (state constants.TaskState, status string, metadata Metadata, err error) { var metadataLocal Metadata for _, elem := range generic { @@ -446,7 +446,7 @@ func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, c } // FirmwareTaskStatusFromInterfaces identifies implementations of the FirmwareTaskVerifier interface and passes the found implementations to the firmwareTaskStatus() wrapper. -func FirmwareTaskStatusFromInterfaces(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []interface{}) (state, status string, metadata Metadata, err error) { +func FirmwareTaskStatusFromInterfaces(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []interface{}) (state constants.TaskState, status string, metadata Metadata, err error) { metadata = newMetadata() implementations := make([]firmwareTaskVerifierProvider, 0) diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index 454db942..3f4d300f 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -8,9 +8,9 @@ import ( "time" "github.com/bmc-toolbox/bmclib/v2/constants" - "github.com/bmc-toolbox/bmclib/v2/errors" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/common" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -41,7 +41,7 @@ func TestFirmwareInstall(t *testing.T) { providersAttempted int }{ {"success with metadata", common.SlugBIOS, string(constants.OnReset), false, nil, "1234", nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", common.SlugBIOS, string(constants.OnReset), false, nil, "1234", errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", common.SlugBIOS, string(constants.OnReset), false, nil, "1234", bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", common.SlugBIOS, string(constants.OnReset), false, nil, "1234", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } @@ -136,7 +136,7 @@ func TestFirmwareInstallStatus(t *testing.T) { providersAttempted int }{ {"success with metadata", common.SlugBIOS, "1.1", "1234", constants.FirmwareInstallComplete, nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", common.SlugBIOS, "1.1", "1234", constants.FirmwareInstallFailed, errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", common.SlugBIOS, "1.1", "1234", constants.FirmwareInstallFailed, bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", common.SlugBIOS, "1.1", "1234", "", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } @@ -459,12 +459,12 @@ func TestFirmwareInstallSteps(t *testing.T) { } type firmwareTaskStatusTester struct { - returnState string + returnState constants.TaskState returnStatus string returnError error } -func (f *firmwareTaskStatusTester) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state string, status string, err error) { +func (f *firmwareTaskStatusTester) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) { return f.returnState, f.returnStatus, f.returnError } @@ -479,7 +479,7 @@ func TestFirmwareTaskStatus(t *testing.T) { component string taskID string installVersion string - returnState string + returnState constants.TaskState returnStatus string returnError error ctxTimeout time.Duration @@ -487,7 +487,7 @@ func TestFirmwareTaskStatus(t *testing.T) { providersAttempted int }{ {"success with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.FirmwareInstallComplete, "Upload completed", nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.FirmwareInstallFailed, "Upload failed", errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.FirmwareInstallFailed, "Upload failed", bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", "", "", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } @@ -523,15 +523,15 @@ func TestFirmwareTaskStatusFromInterfaces(t *testing.T) { component string taskID string installVersion string - returnState string + returnState constants.TaskState returnStatus string returnError error ctxTimeout time.Duration providerName string providersAttempted int }{ - {"success with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.FirmwareInstallComplete, "uploading", nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.FirmwareInstallFailed, "failed", errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"success with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.Complete, "uploading", nil, 5 * time.Second, "foo", 1}, + {"failure with metadata", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", constants.Failed, "failed", bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", constants.FirmwareInstallStepUpload, common.SlugBIOS, "1234", "1.0", "", "", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } diff --git a/client.go b/client.go index 0bac7eae..f5a13f91 100644 --- a/client.go +++ b/client.go @@ -623,7 +623,7 @@ func (c *Client) FirmwareUpload(ctx context.Context, component string, file *os. } // FirmwareTaskStatus pass through library function to check firmware task statuses -func (c *Client) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state, status string, err error) { +func (c *Client) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) { ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "FirmwareTaskStatus") defer span.End() diff --git a/constants/constants.go b/constants/constants.go index 2bd1b80a..d2a6bd10 100644 --- a/constants/constants.go +++ b/constants/constants.go @@ -6,12 +6,11 @@ type ( // The FirmwareInstallStep identifies each phase of a firmware install process. FirmwareInstallStep string + + TaskState string ) const ( - // Unknown is the constant that defines unknown things - Unknown = "Unknown" - // EnvEnableDebug is the const for the environment variable to cause bmclib to dump debugging debugging information. // the valid parameter for this environment variable is 'true' EnvEnableDebug = "DEBUG_BMCLIB" @@ -49,33 +48,41 @@ const ( // FirmwareInstallInitializing indicates the device is performing init actions to install the update // this covers the redfish states - 'starting', 'downloading' // no action is required from the callers part in this state - FirmwareInstallInitializing = "initializing" + FirmwareInstallInitializing = "initializing" + Initializing TaskState = "initializing" // FirmwareInstallQueued indicates the device has queued the update, but has not started the update task yet // this covers the redfish states - 'pending', 'new' // no action is required from the callers part in this state - FirmwareInstallQueued = "queued" + FirmwareInstallQueued = "queued" + Queued TaskState = "queued" // FirmwareInstallRunner indicates the device is installing the update // this covers the redfish states - 'running', 'stopping', 'cancelling' // no action is required from the callers part in this state - FirmwareInstallRunning = "running" + FirmwareInstallRunning = "running" + Running TaskState = "running" // FirmwareInstallComplete indicates the device completed the firmware install // this covers the redfish state - 'complete' - FirmwareInstallComplete = "complete" + FirmwareInstallComplete = "complete" + Complete TaskState = "complete" // FirmwareInstallFailed indicates the firmware install failed // this covers the redfish states - 'interrupted', 'killed', 'exception', 'cancelled', 'suspended' - FirmwareInstallFailed = "failed" + FirmwareInstallFailed = "failed" + Failed TaskState = "failed" // FirmwareInstallPowerCycleHost indicates the firmware install requires a host power cycle - FirmwareInstallPowerCycleHost = "powercycle-host" + FirmwareInstallPowerCycleHost = "powercycle-host" + PowerCycleHost TaskState = "powercycle-host" // FirmwareInstallPowerCycleBMC indicates the firmware install requires a BMC power cycle - FirmwareInstallPowerCycleBMC = "powercycle-bmc" + FirmwareInstallPowerCycleBMC = "powercycle-bmc" + PowerCycleBMC TaskState = "powercycle-bmc" - FirmwareInstallUnknown = "unknown" + FirmwareInstallUnknown = "unknown" + Unknown TaskState = "unknown" // FirmwareInstallStepUploadInitiateInstall identifies the step to upload _and_ initialize the firmware install. // as part of the same call. From af663881d78848743d316b5fbcee18a358866487 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 20 Nov 2023 15:32:49 +0100 Subject: [PATCH 2/5] redfishwrapper: to return constants.TaskState --- internal/redfishwrapper/task.go | 36 +++++++++--- internal/redfishwrapper/task_test.go | 86 +++++++++++++++++++++++++--- 2 files changed, 105 insertions(+), 17 deletions(-) diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go index 869da835..592712f2 100644 --- a/internal/redfishwrapper/task.go +++ b/internal/redfishwrapper/task.go @@ -11,6 +11,10 @@ import ( gofishrf "github.com/stmcginnis/gofish/redfish" ) +var ( + errUnexpectedTaskState = errors.New("unexpected task state") +) + func (c *Client) Task(ctx context.Context, taskID string) (*gofishrf.Task, error) { tasks, err := c.Tasks(ctx) if err != nil { @@ -28,29 +32,43 @@ func (c *Client) Task(ctx context.Context, taskID string) (*gofishrf.Task, error return nil, bmclibErrs.ErrTaskNotFound } -func (c *Client) TaskStatus(ctx context.Context, taskID string) (state, status string, err error) { +func (c *Client) TaskStatus(ctx context.Context, taskID string) (constants.TaskState, string, error) { task, err := c.Task(ctx, taskID) if err != nil { return "", "", errors.Wrap(err, "error querying redfish for taskID: "+taskID) } taskInfo := fmt.Sprintf("id: %s, state: %s, status: %s", task.ID, task.TaskState, task.TaskStatus) - state = strings.ToLower(string(task.TaskState)) + state := strings.ToLower(string(task.TaskState)) + return c.ConvertTaskState(state), taskInfo, nil +} +func (c *Client) ConvertTaskState(state string) constants.TaskState { switch state { case "starting", "downloading", "downloaded": - return constants.FirmwareInstallInitializing, taskInfo, nil + return constants.Initializing case "running", "stopping", "cancelling", "scheduling": - return constants.FirmwareInstallRunning, taskInfo, nil + return constants.Running case "pending", "new": - return constants.FirmwareInstallQueued, taskInfo, nil + return constants.Queued case "scheduled": - return constants.FirmwareInstallPowerCycleHost, taskInfo, nil + return constants.PowerCycleHost case "interrupted", "killed", "exception", "cancelled", "suspended", "failed": - return constants.FirmwareInstallFailed, taskInfo, nil + return constants.Failed case "completed": - return constants.FirmwareInstallComplete, taskInfo, nil + return constants.Complete + default: + return constants.Unknown + } +} + +func (c *Client) TaskStateActive(state constants.TaskState) (bool, error) { + switch state { + case constants.Initializing, constants.Running, constants.Queued: + return true, nil + case constants.Complete, constants.Failed: + return false, nil default: - return constants.FirmwareInstallUnknown, taskInfo, nil + return false, errors.Wrap(errUnexpectedTaskState, string(state)) } } diff --git a/internal/redfishwrapper/task_test.go b/internal/redfishwrapper/task_test.go index 607b150b..cd7fbabd 100644 --- a/internal/redfishwrapper/task_test.go +++ b/internal/redfishwrapper/task_test.go @@ -13,6 +13,76 @@ import ( "github.com/stretchr/testify/assert" ) +func TestConvertTaskState(t *testing.T) { + testCases := []struct { + testName string + state string + expected constants.TaskState + }{ + {"starting state", "starting", constants.Initializing}, + {"downloading state", "downloading", constants.Initializing}, + {"downloaded state", "downloaded", constants.Initializing}, + {"running state", "running", constants.Running}, + {"stopping state", "stopping", constants.Running}, + {"cancelling state", "cancelling", constants.Running}, + {"scheduling state", "scheduling", constants.Running}, + {"pending state", "pending", constants.Queued}, + {"new state", "new", constants.Queued}, + {"scheduled state", "scheduled", constants.PowerCycleHost}, + {"interrupted state", "interrupted", constants.Failed}, + {"killed state", "killed", constants.Failed}, + {"exception state", "exception", constants.Failed}, + {"cancelled state", "cancelled", constants.Failed}, + {"suspended state", "suspended", constants.Failed}, + {"failed state", "failed", constants.Failed}, + {"completed state", "completed", constants.Complete}, + {"unknown state", "unknown_state", constants.Unknown}, + } + + client := Client{} + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + result := client.ConvertTaskState(tc.state) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestTaskStateActive(t *testing.T) { + testCases := []struct { + testName string + taskState constants.TaskState + expected bool + err error + }{ + {"active initializing", constants.Initializing, true, nil}, + {"active running", constants.Running, true, nil}, + {"active queued", constants.Queued, true, nil}, + {"inactive complete", constants.Complete, false, nil}, + {"inactive failed", constants.Failed, false, nil}, + {"unknown state", "foobar", false, errUnexpectedTaskState}, + } + + client := &Client{} + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + active, err := client.TaskStateActive(tc.taskState) + + if tc.err != nil { + assert.ErrorIs(t, err, tc.err) + return + } + + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, tc.expected, active) + }) + } +} + func TestTaskStatus(t *testing.T) { type hmap map[string]func(http.ResponseWriter, *http.Request) withHandler := func(s string, f func(http.ResponseWriter, *http.Request)) hmap { @@ -29,7 +99,7 @@ func TestTaskStatus(t *testing.T) { tests := map[string]struct { hmap hmap - expectedState string + expectedState constants.TaskState expectedStatus string expectedErr error }{ @@ -38,7 +108,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_starting.json"), ), - expectedState: constants.FirmwareInstallInitializing, + expectedState: constants.Initializing, expectedStatus: "id: 1, state: Starting, status: OK", expectedErr: nil, }, @@ -47,7 +117,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_running.json"), ), - expectedState: constants.FirmwareInstallRunning, + expectedState: constants.Running, expectedStatus: "id: 1, state: Running, status: OK", expectedErr: nil, }, @@ -56,7 +126,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_pending.json"), ), - expectedState: constants.FirmwareInstallQueued, + expectedState: constants.Queued, expectedStatus: "id: 1, state: Pending, status: OK", expectedErr: nil, }, @@ -65,7 +135,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_scheduled.json"), ), - expectedState: constants.FirmwareInstallPowerCycleHost, + expectedState: constants.PowerCycleHost, expectedStatus: "id: 1, state: Scheduled, status: OK", expectedErr: nil, }, @@ -74,7 +144,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_failed.json"), ), - expectedState: constants.FirmwareInstallFailed, + expectedState: constants.Failed, expectedStatus: "id: 1, state: Failed, status: OK", expectedErr: nil, }, @@ -83,7 +153,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_completed.json"), ), - expectedState: constants.FirmwareInstallComplete, + expectedState: constants.Complete, expectedStatus: "id: 1, state: Completed, status: OK", expectedErr: nil, }, @@ -92,7 +162,7 @@ func TestTaskStatus(t *testing.T) { "/redfish/v1/TaskService/Tasks/1", endpointFunc(t, "tasks/tasks_1_unknown.json"), ), - expectedState: constants.FirmwareInstallUnknown, + expectedState: constants.Unknown, expectedStatus: "id: 1, state: foobared, status: OK", expectedErr: nil, }, From 30e9048670f89505534492796d8b8d1bf82cf133 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 20 Nov 2023 15:33:33 +0100 Subject: [PATCH 3/5] providers/supermicro: return TaskState constant in FirmwareTaskStatus --- providers/supermicro/firmware.go | 2 +- providers/supermicro/supermicro.go | 2 +- providers/supermicro/x11.go | 2 +- providers/supermicro/x11_firmware_bios.go | 22 +++++++++---------- providers/supermicro/x11_firmware_bmc.go | 14 ++++++------ providers/supermicro/x11_firmware_bmc_test.go | 12 +++++----- providers/supermicro/x12.go | 2 +- 7 files changed, 28 insertions(+), 28 deletions(-) diff --git a/providers/supermicro/firmware.go b/providers/supermicro/firmware.go index b0540b04..e4a7c8ca 100644 --- a/providers/supermicro/firmware.go +++ b/providers/supermicro/firmware.go @@ -68,7 +68,7 @@ func (c *Client) FirmwareInstallUploaded(ctx context.Context, component, uploadT } // FirmwareTaskStatus returns the status of a firmware related task queued on the BMC. -func (c *Client) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state, status string, err error) { +func (c *Client) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) { if err := c.serviceClient.supportsFirmwareInstall(ctx, c.bmc.deviceModel()); err != nil { return "", "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, err.Error()) } diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go index bb0e4818..47f167b9 100644 --- a/providers/supermicro/supermicro.go +++ b/providers/supermicro/supermicro.go @@ -104,7 +104,7 @@ type bmcQueryor interface { firmwareInstallSteps(component string) ([]constants.FirmwareInstallStep, error) firmwareUpload(ctx context.Context, component string, file *os.File) (taskID string, err error) firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string) (installTaskID string, err error) - firmwareTaskStatus(ctx context.Context, component, taskID string) (state, status string, err error) + firmwareTaskStatus(ctx context.Context, component, taskID string) (state constants.TaskState, status string, err error) // query device model from the bmc queryDeviceModel(ctx context.Context) (model string, err error) // returns the device model, that was queried previously with queryDeviceModel diff --git a/providers/supermicro/x11.go b/providers/supermicro/x11.go index 705c29f2..08525e7a 100644 --- a/providers/supermicro/x11.go +++ b/providers/supermicro/x11.go @@ -131,7 +131,7 @@ func (c *x11) firmwareInstallUploaded(ctx context.Context, component, _ string) return "", errors.Wrap(bmclibErrs.ErrFirmwareInstallUploaded, "component unsupported: "+component) } -func (c *x11) firmwareTaskStatus(ctx context.Context, component, _ string) (state, status string, err error) { +func (c *x11) firmwareTaskStatus(ctx context.Context, component, _ string) (state constants.TaskState, status string, err error) { component = strings.ToUpper(component) switch component { diff --git a/providers/supermicro/x11_firmware_bios.go b/providers/supermicro/x11_firmware_bios.go index 255d43ee..0b989e2d 100644 --- a/providers/supermicro/x11_firmware_bios.go +++ b/providers/supermicro/x11_firmware_bios.go @@ -376,7 +376,7 @@ func (c *x11) setBIOSUpdateDone(ctx context.Context) error { } // statusBIOSFirmwareInstall returns the status of the firmware install process -func (c *x11) statusBIOSFirmwareInstall(ctx context.Context) (state, status string, err error) { +func (c *x11) statusBIOSFirmwareInstall(ctx context.Context) (state constants.TaskState, status string, err error) { payload := []byte(`fwtype=1&_`) headers := map[string]string{"Content-Type": "application/x-www-form-urlencoded; charset=UTF-8"} @@ -393,7 +393,7 @@ func (c *x11) statusBIOSFirmwareInstall(ctx context.Context) (state, status stri // at the end of the install the BMC resets itself and the response is in HTML. if bytes.Contains(resp, []byte(``)) || !bytes.Contains(resp, []byte(``)) { // reopen session here, check firmware install status - return constants.FirmwareInstallUnknown, "session expired/unexpected response", bmclibErrs.ErrSessionExpired + return constants.Unknown, "session expired/unexpected response", bmclibErrs.ErrSessionExpired } // as long as the response is xml, the firmware install is running @@ -404,17 +404,17 @@ func (c *x11) statusBIOSFirmwareInstall(ctx context.Context) (state, status stri switch { // 1% indicates the file has been uploaded and the firmware install is not yet initiated case bytes.Contains(resp, []byte("0")) && bytes.Contains(resp, []byte("1")): - return constants.FirmwareInstallFailed, percent, bmclibErrs.ErrBMCColdResetRequired + return constants.Failed, percent, bmclibErrs.ErrBMCColdResetRequired // 0% along with the check on the component endpoint indicates theres no update in progress case (bytes.Contains(resp, []byte("0")) && bytes.Contains(resp, []byte("0"))): if err := c.checkComponentUpdateMisc(ctx, "postUpdate"); err != nil { if errors.Is(err, bmclibErrs.ErrHostPowercycleRequired) { - return constants.FirmwareInstallPowerCycleHost, percent, nil + return constants.PowerCycleHost, percent, nil } } - return constants.FirmwareInstallComplete, "all done!", nil + return constants.Complete, "all done!", nil // status 0 and 100% indicates the update is complete and requires a few post update calls case bytes.Contains(resp, []byte("0")) && bytes.Contains(resp, []byte("100")): @@ -427,22 +427,22 @@ func (c *x11) statusBIOSFirmwareInstall(ctx context.Context) (state, status stri // tells the BMC it can get out of the BIOS update mode if err := c.checkComponentUpdateMisc(ctx, "postUpdate"); err != nil { if errors.Is(err, bmclibErrs.ErrHostPowercycleRequired) { - return constants.FirmwareInstallPowerCycleHost, percent, nil + return constants.PowerCycleHost, percent, nil } - return constants.FirmwareInstallPowerCycleHost, percent, err + return constants.PowerCycleHost, percent, err } - return constants.FirmwareInstallPowerCycleHost, percent, nil + return constants.PowerCycleHost, percent, nil // status 8 and percent 0 indicates its initializing the update case bytes.Contains(resp, []byte("8")) && bytes.Contains(resp, []byte("0")): - return constants.FirmwareInstallRunning, percent, nil + return constants.Running, percent, nil // status 8 and any other percent value indicates its running case bytes.Contains(resp, []byte("8")) && bytes.Contains(resp, []byte("")): - return constants.FirmwareInstallRunning, percent, nil + return constants.Running, percent, nil } - return constants.FirmwareInstallUnknown, "", nil + return constants.Unknown, "", nil } diff --git a/providers/supermicro/x11_firmware_bmc.go b/providers/supermicro/x11_firmware_bmc.go index 39ec304a..246aa5e8 100644 --- a/providers/supermicro/x11_firmware_bmc.go +++ b/providers/supermicro/x11_firmware_bmc.go @@ -243,24 +243,24 @@ func (c *x11) initiateBMCFirmwareInstall(ctx context.Context) error { } // statusBMCFirmwareInstall returns the status of the firmware install process -func (c *x11) statusBMCFirmwareInstall(ctx context.Context) (state, status string, err error) { +func (c *x11) statusBMCFirmwareInstall(ctx context.Context) (state constants.TaskState, status string, err error) { payload := []byte(`fwtype=0&_`) headers := map[string]string{"Content-Type": "application/x-www-form-urlencoded; charset=UTF-8"} resp, httpStatus, err := c.query(ctx, "cgi/upgrade_process.cgi", http.MethodPost, bytes.NewReader(payload), headers, 0) if err != nil { - return constants.FirmwareInstallUnknown, "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, err.Error()) + return constants.Unknown, "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, err.Error()) } if httpStatus != http.StatusOK { - return constants.FirmwareInstallUnknown, "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "Unexpected http status code: "+strconv.Itoa(httpStatus)) + return constants.Unknown, "", errors.Wrap(bmclibErrs.ErrFirmwareInstallStatus, "Unexpected http status code: "+strconv.Itoa(httpStatus)) } // if theres html or no xml in the response, the session expired // at the end of the install the BMC resets itself and the response is in HTML. if bytes.Contains(resp, []byte(``)) || !bytes.Contains(resp, []byte(``)) { // reopen session here, check firmware install status - return constants.FirmwareInstallUnknown, "session expired/unexpected response", bmclibErrs.ErrSessionExpired + return constants.Unknown, "session expired/unexpected response", bmclibErrs.ErrSessionExpired } // as long as the response is xml, the firmware install is running @@ -274,12 +274,12 @@ func (c *x11) statusBMCFirmwareInstall(ctx context.Context) (state, status strin // // 0% indicates its either not running or complete case "0%", "100%": - return constants.FirmwareInstallComplete, percent, nil + return constants.Complete, percent, nil // until 2% its initializing case "1%", "2%": - return constants.FirmwareInstallInitializing, percent, nil + return constants.Initializing, percent, nil // any other percent value indicates its active default: - return constants.FirmwareInstallRunning, percent, nil + return constants.Running, percent, nil } } diff --git a/providers/supermicro/x11_firmware_bmc_test.go b/providers/supermicro/x11_firmware_bmc_test.go index b6b0ad13..9c7024f5 100644 --- a/providers/supermicro/x11_firmware_bmc_test.go +++ b/providers/supermicro/x11_firmware_bmc_test.go @@ -366,7 +366,7 @@ func TestX11InitiateBMCFirmwareInstall(t *testing.T) { func TestX11StatusBMCFirmwareInstall(t *testing.T) { testcases := []struct { name string - expectState string + expectState constants.TaskState expectStatus string errorContains string endpoint string @@ -374,7 +374,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { }{ { "state complete 0", - constants.FirmwareInstallComplete, + constants.Complete, "0%", "", "/cgi/upgrade_process.cgi", @@ -399,7 +399,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { }, { "state complete 100", - constants.FirmwareInstallComplete, + constants.Complete, "100%", "", "/cgi/upgrade_process.cgi", @@ -424,7 +424,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { }, { "state initializing", - constants.FirmwareInstallInitializing, + constants.Initializing, "1%", "", "/cgi/upgrade_process.cgi", @@ -449,7 +449,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { }, { "status running", - constants.FirmwareInstallRunning, + constants.Running, "95%", "", "/cgi/upgrade_process.cgi", @@ -474,7 +474,7 @@ func TestX11StatusBMCFirmwareInstall(t *testing.T) { }, { "status unknown", - constants.FirmwareInstallUnknown, + constants.Unknown, "", "session expired", "/cgi/upgrade_process.cgi", diff --git a/providers/supermicro/x12.go b/providers/supermicro/x12.go index e027d858..a48c0006 100644 --- a/providers/supermicro/x12.go +++ b/providers/supermicro/x12.go @@ -294,7 +294,7 @@ func (c *x12) firmwareInstallUploaded(ctx context.Context, component, uploadTask return c.redfish.StartUpdateForUploadedFirmware(ctx) } -func (c *x12) firmwareTaskStatus(ctx context.Context, component, taskID string) (state, status string, err error) { +func (c *x12) firmwareTaskStatus(ctx context.Context, component, taskID string) (state constants.TaskState, status string, err error) { if err = c.supportsInstall(component); err != nil { return "", "", errors.Wrap(brrs.ErrFirmwareTaskStatus, err.Error()) } From 382e12143d9abfab8caac9da2ab9231e82a8f67c Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 20 Nov 2023 15:34:24 +0100 Subject: [PATCH 4/5] bmc/firmware: import bmclib/errors as bmclibErrors --- bmc/firmware_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index 3f4d300f..be19f222 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -230,7 +230,7 @@ func TestFirmwareInstallUploaded(t *testing.T) { providersAttempted int }{ {"success with metadata", common.SlugBIOS, "1234", "5678", nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", common.SlugBIOS, "1234", "", errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", common.SlugBIOS, "1234", "", bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", common.SlugBIOS, "1234", "", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } @@ -325,7 +325,7 @@ func TestFirmwareUpload(t *testing.T) { providersAttempted int }{ {"success with metadata", common.SlugBIOS, nil, "1234", nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", common.SlugBIOS, nil, "1234", errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", common.SlugBIOS, nil, "1234", bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", common.SlugBIOS, nil, "1234", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } @@ -430,7 +430,7 @@ func TestFirmwareInstallSteps(t *testing.T) { providersAttempted int }{ {"success with metadata", common.SlugBIOS, []constants.FirmwareInstallStep{constants.FirmwareInstallStepUpload, constants.FirmwareInstallStepInstallStatus}, nil, 5 * time.Second, "foo", 1}, - {"failure with metadata", common.SlugBIOS, nil, errors.ErrNon200Response, 5 * time.Second, "foo", 1}, + {"failure with metadata", common.SlugBIOS, nil, bmclibErrs.ErrNon200Response, 5 * time.Second, "foo", 1}, {"failure with context timeout", common.SlugBIOS, nil, context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, } From 6c585e65e7337ca73069133c5b696398262635b4 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 20 Nov 2023 15:41:55 +0100 Subject: [PATCH 5/5] bmc/firmware_test: fix unused import --- bmc/firmware_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index be19f222..0756bcd2 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -10,7 +10,6 @@ import ( "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/common" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" )