From 06796aa43defdeb535acb773d154405a51a20792 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 16:41:14 +0200 Subject: [PATCH 01/16] security: update golang.org/x/net to 0.7.0 --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index d73adff9..e53d5ac0 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( go.uber.org/goleak v1.2.1 golang.org/x/crypto v0.1.0 golang.org/x/exp v0.0.0-20230127130021-4ca2cb1a16b7 - golang.org/x/net v0.1.0 + golang.org/x/net v0.7.0 gopkg.in/go-playground/assert.v1 v1.2.1 ) From 9a70262004884794866f759e6fe408932cec93d0 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 16:53:05 +0200 Subject: [PATCH 02/16] Update go dependencies --- go.mod | 2 +- go.sum | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index e53d5ac0..4ef38640 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/mattn/go-isatty v0.0.14 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/satori/go.uuid v1.2.0 // indirect - golang.org/x/sys v0.1.0 // indirect + golang.org/x/sys v0.5.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index cdb371e3..c73a5614 100644 --- a/go.sum +++ b/go.sum @@ -72,15 +72,15 @@ golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/exp v0.0.0-20230127130021-4ca2cb1a16b7 h1:o7Ps2IYdzLRolS9/nadqeMSHpa9k8pu8u+VKBFUG7cQ= golang.org/x/exp v0.0.0-20230127130021-4ca2cb1a16b7/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= -golang.org/x/net v0.1.0 h1:hZ/3BUoy5aId7sCpA/Tc5lt8DkFgdVS2onTpJsZ/fl0= -golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= +golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g= +golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210608053332-aa57babbf139/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/term v0.1.0 h1:g6Z6vPFA9dYBAF7DWcH6sCcOntplXsDKcliusYijMlw= +golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= From 2f8aa33d35c32a7d50413daadee3f7456a554e67 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Sep 2023 09:56:09 +0200 Subject: [PATCH 03/16] internal/redfishwrapper: expose Tasks() method --- internal/redfishwrapper/client.go | 5 + providers/redfish/tasks_dell.go | 175 +++++++++++++++++++++++++++ providers/redfish/tasks_dell_test.go | 40 ++++++ 3 files changed, 220 insertions(+) create mode 100644 providers/redfish/tasks_dell.go create mode 100644 providers/redfish/tasks_dell_test.go diff --git a/internal/redfishwrapper/client.go b/internal/redfishwrapper/client.go index f600631c..6d9c01ee 100644 --- a/internal/redfishwrapper/client.go +++ b/internal/redfishwrapper/client.go @@ -13,6 +13,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/pkg/errors" "github.com/stmcginnis/gofish" + "github.com/stmcginnis/gofish/redfish" "golang.org/x/exp/slices" ) @@ -208,3 +209,7 @@ func (c *Client) PostWithHeaders(ctx context.Context, url string, payload interf func (c *Client) PatchWithHeaders(ctx context.Context, url string, payload interface{}, headers map[string]string) (*http.Response, error) { return c.client.PatchWithHeaders(url, payload, headers) } + +func (c *Client) Tasks(ctx context.Context) ([]*redfish.Task, error) { + return c.client.Service.Tasks() +} diff --git a/providers/redfish/tasks_dell.go b/providers/redfish/tasks_dell.go new file mode 100644 index 00000000..0ecd92de --- /dev/null +++ b/providers/redfish/tasks_dell.go @@ -0,0 +1,175 @@ +package redfish + +import ( + "encoding/json" + "io" + "strconv" + "strings" + + bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" + "github.com/bmc-toolbox/common" + "github.com/pkg/errors" + + gofishcommon "github.com/stmcginnis/gofish/common" + gofishrf "github.com/stmcginnis/gofish/redfish" +) + +// TODO: figure how this can be moved into the dell provider +// +// Dell specific redfish methods + +var ( + componentSlugDellJobName = map[string]string{ + common.SlugBIOS: "Firmware Update: BIOS", + common.SlugBMC: "Firmware Update: iDRAC with Lifecycle Controller", + common.SlugNIC: "Firmware Update: Network", + common.SlugDrive: "Firmware Update: Serial ATA", + common.SlugStorageController: "Firmware Update: SAS RAID", + } +) + +type dellJob struct { + PercentComplete int + OdataID string `json:"@odata.id"` + StartTime string + CompletionTime string + ID string + Message string + Name string + JobState string + JobType string +} + +type dellJobResponseData struct { + Members []*dellJob +} + +// dellJobID formats and returns taskID as a Dell Job ID +func dellJobID(id string) string { + if !strings.HasPrefix(id, "JID") { + return "JID_" + id + } + + return id +} + +func (c *Conn) getDellFirmwareInstallTaskScheduled(slug string) (*gofishrf.Task, error) { + // get tasks by state + tasks, err := c.dellJobs("scheduled") + if err != nil { + return nil, err + } + + // filter to match the task Name based on the component slug + for _, task := range tasks { + if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { + return task, nil + } + } + + return nil, nil +} + +func (c *Conn) dellPurgeScheduledFirmwareInstallJob(slug string) error { + // get tasks by state + tasks, err := c.dellJobs("scheduled") + if err != nil { + return err + } + + // filter to match the task Name based on the component slug + for _, task := range tasks { + if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { + err = c.dellPurgeJob(task.ID) + if err != nil { + return err + } + } + } + + return nil +} + +func (c *Conn) dellPurgeJob(id string) error { + id = dellJobID(id) + + endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs/" + id + + resp, err := c.redfishwrapper.Delete(endpoint) + if err != nil { + return errors.Wrap(bmcliberrs.ErrTaskPurge, err.Error()) + } + + if resp.StatusCode != 200 { + return errors.Wrap(bmcliberrs.ErrTaskPurge, "response code: "+resp.Status) + } + + return nil +} + +// dellFirmwareUpdateTaskStatus looks up the Dell Job and returns it as a redfish task object +func (c *Conn) dellJobAsRedfishTask(jobID string) (*gofishrf.Task, error) { + jobID = dellJobID(jobID) + + tasks, err := c.dellJobs("") + if err != nil { + return nil, err + } + + for _, task := range tasks { + if task.ID == jobID { + return task, nil + } + } + + return nil, errors.Wrap(bmcliberrs.ErrTaskNotFound, "task with ID not found: "+jobID) +} + +// dellJobs returns all dell jobs as redfish task objects +// state: optional +func (c *Conn) dellJobs(state string) ([]*gofishrf.Task, error) { + endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)" + + resp, err := c.redfishwrapper.Get(endpoint) + if err != nil { + return nil, err + } + + if resp.StatusCode != 200 { + return nil, errors.New("dell jobs endpoint returned unexpected status code: " + strconv.Itoa(resp.StatusCode)) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + data := dellJobResponseData{} + err = json.Unmarshal(body, &data) + if err != nil { + return nil, err + } + + tasks := []*gofishrf.Task{} + for _, job := range data.Members { + if state != "" && !strings.EqualFold(job.JobState, state) { + continue + } + + tasks = append(tasks, &gofishrf.Task{ + Entity: gofishcommon.Entity{ + ID: job.ID, + ODataID: job.OdataID, + Name: job.Name, + }, + Description: job.Name, + PercentComplete: job.PercentComplete, + StartTime: job.StartTime, + EndTime: job.CompletionTime, + TaskState: gofishrf.TaskState(job.JobState), + TaskStatus: gofishcommon.Health(job.Message), // abuse the TaskStatus to include any status message + }) + } + + return tasks, nil +} diff --git a/providers/redfish/tasks_dell_test.go b/providers/redfish/tasks_dell_test.go new file mode 100644 index 00000000..bf1a376b --- /dev/null +++ b/providers/redfish/tasks_dell_test.go @@ -0,0 +1,40 @@ +package redfish + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +// handler registered in redfish_test.go +func dellJobs(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + w.WriteHeader(http.StatusNotFound) + } + + _, _ = w.Write(jsonResponse(r.RequestURI)) +} + +func Test_dellFirmwareUpdateTask(t *testing.T) { + // see fixtures/v1/dell/jobs.json for the job IDs + // completed job + status, err := mockClient.dellJobAsRedfishTask("467767920358") + if err != nil { + t.Fatal(err) + } + + assert.NotNil(t, status) + assert.Equal(t, "2022-03-08T16:02:33", status.EndTime) + assert.Equal(t, "2022-03-08T15:59:52", status.StartTime) + assert.Equal(t, 100, status.PercentComplete) + assert.Equal(t, "Completed", string(status.TaskState)) + assert.Equal(t, "Job completed successfully.", string(status.TaskStatus)) +} + +func Test_dellPurgeScheduledFirmwareInstallJob(t *testing.T) { + err := mockClient.dellPurgeScheduledFirmwareInstallJob("bios") + if err != nil { + t.Fatal(err) + } +} From d3c11be61aa758249bdb3b430613ee0239eff917 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Sep 2023 09:57:42 +0200 Subject: [PATCH 04/16] providers/redfish: split up method to support upload based on UpdateService URI --- providers/redfish/firmware.go | 216 ++++++++++++++++++---------------- 1 file changed, 113 insertions(+), 103 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index ca6231f4..d8b36c3b 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -28,6 +28,13 @@ var ( errMultiPartPayload = errors.New("error preparing multipart payload") ) +type installMethod string + +const ( + unstructuredHttpPush installMethod = "unstructuredHttpPush" + multipartHttpUpload installMethod = "multipartUpload" +) + // SupportedFirmwareApplyAtValues returns the supported redfish firmware applyAt values func SupportedFirmwareApplyAtValues() []string { return []string{ @@ -44,8 +51,7 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "method expects an *os.File object") } - // validate firmware update mechanism is supported - err = c.firmwareUpdateCompatible(ctx) + installMethod, installURI, err := c.firmwareInstallMethodURI(ctx) if err != nil { return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) } @@ -63,44 +69,31 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String()) } + // TODO; uncomment once obmc support is implemented for tasks // list redfish firmware install task if theres one present - task, err := c.GetFirmwareInstallTaskQueued(ctx, component) - if err != nil { - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - } - - if task != nil { - msg := fmt.Sprintf("task for %s firmware install present: %s", component, task.ID) - c.Log.V(2).Info("warn", msg) - - if forceInstall { - err = c.purgeQueuedFirmwareInstallTask(ctx, component) - if err != nil { - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - } - } else { - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, msg) - } - } - - updateParameters, err := json.Marshal(struct { - Targets []string `json:"Targets"` - RedfishOpApplyTime string `json:"@Redfish.OperationApplyTime"` - Oem struct{} `json:"Oem"` - }{ - []string{}, - applyAt, - struct{}{}, - }) - - if err != nil { - return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - } + // task, err := c.GetFirmwareInstallTaskQueued(ctx, component) + // if err != nil { + // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) + // } + // + // if task != nil { + // msg := fmt.Sprintf("task for %s firmware install present: %s", component, task.ID) + // c.Log.V(2).Info("warn", msg) + // + // if forceInstall { + // err = c.purgeQueuedFirmwareInstallTask(ctx, component) + // if err != nil { + // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) + // } + // } else { + // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, msg) + // } + // } // override the gofish HTTP client timeout, // since the context timeout is set at Open() and is at a lower value than required for this operation. // - // record the http client timeout to be restored + // record the http client timeout to be restored when this method returns httpClientTimeout := c.redfishwrapper.HttpClientTimeout() defer func() { c.redfishwrapper.SetHttpClientTimeout(httpClientTimeout) @@ -108,14 +101,25 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f c.redfishwrapper.SetHttpClientTimeout(time.Until(ctxDeadline)) - payload := &multipartPayload{ - updateParameters: updateParameters, - updateFile: updateFile, - } + var resp *http.Response - resp, err := c.runRequestWithMultipartPayload(http.MethodPost, "/redfish/v1/UpdateService/MultipartUpload", payload) - if err != nil { - return "", errors.Wrap(bmclibErrs.ErrFirmwareUpload, err.Error()) + switch installMethod { + case multipartHttpUpload: + var uploadErr error + resp, uploadErr = c.multipartHTTPUpload(ctx, installURI, applyAt, reader) + if uploadErr != nil { + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, uploadErr.Error()) + } + + case unstructuredHttpPush: + var uploadErr error + resp, uploadErr = c.unstructuredHttpUpload(ctx, installURI, applyAt, reader) + if uploadErr != nil { + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, uploadErr.Error()) + } + + default: + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "unsupported install method: "+string(installMethod)) } if resp.StatusCode != http.StatusAccepted { @@ -139,52 +143,58 @@ type multipartPayload struct { updateFile *os.File } -// FirmwareInstallStatus returns the status of the firmware install task queued -func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (state string, err error) { - vendor, _, err := c.DeviceVendorModel(ctx) +func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, update io.Reader) (*http.Response, error) { + if url == "" { + return nil, fmt.Errorf("unable to execute request, no target provided") + } + + parameters, err := json.Marshal(struct { + Targets []string `json:"Targets"` + RedfishOpApplyTime string `json:"@Redfish.OperationApplyTime"` + Oem struct{} `json:"Oem"` + }{ + []string{}, + applyAt, + struct{}{}, + }) + if err != nil { - return state, errors.Wrap(err, "unable to determine device vendor, model attributes") + return nil, errors.Wrap(err, "error preparing multipart UpdateParameters payload") } - var task *gofishrf.Task - switch { - case strings.Contains(vendor, constants.Dell): - task, err = c.dellJobAsRedfishTask(taskID) - default: - err = errors.Wrap( - bmclibErrs.ErrNotImplemented, - "FirmwareInstallStatus() for vendor: "+vendor, - ) + // payload ordered in the format it ends up in the multipart form + payload := []map[string]io.Reader{ + {"UpdateParameters": bytes.NewReader(parameters)}, + {"UpdateFile": update}, } + return c.runRequestWithMultipartPayload(url, payload) +} + +func (c *Conn) unstructuredHttpUpload(ctx context.Context, url string, update io.Reader) (*http.Response, error) { + +} + +// firmwareUpdateMethodURI returns the updateMethod and URI +func (c *Conn) firmwareInstallMethodURI(ctx context.Context) (method installMethod, updateURI string, err error) { + updateService, err := c.redfishwrapper.UpdateService() if err != nil { - return state, err + return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, err.Error()) } - if task == nil { - return state, errors.New("failed to lookup task status for task ID: " + taskID) + // update service disabled + if !updateService.ServiceEnabled { + return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "service disabled") } - state = strings.ToLower(string(task.TaskState)) - - // so much for standards... - switch state { - case "starting", "downloading", "downloaded": - return constants.FirmwareInstallInitializing, nil - case "running", "stopping", "cancelling", "scheduling": - return constants.FirmwareInstallRunning, nil - case "pending", "new": - return constants.FirmwareInstallQueued, nil - case "scheduled": - return constants.FirmwareInstallPowerCyleHost, nil - case "interrupted", "killed", "exception", "cancelled", "suspended", "failed": - return constants.FirmwareInstallFailed, nil - case "completed": - return constants.FirmwareInstallComplete, nil - default: - return constants.FirmwareInstallUnknown + ": " + state, nil + switch { + case updateService.MultipartHTTPPushURI != "": + return multipartHttpUpload, updateService.MultipartHTTPPushURI, nil + case updateService.HTTPPushURI != "": + return unstructuredHttpPush, updateService.HTTPPushURI, nil } + return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "unsupported update method") } // firmwareUpdateCompatible retuns an error if the firmware update process for the BMC is not supported @@ -283,7 +293,7 @@ func multipartPayloadSize(payload *multipartPayload) (int64, *bytes.Buffer, erro // hey. // --------------------------1771f60800cb2801-- -func (c *Conn) runRequestWithMultipartPayload(method, url string, payload *multipartPayload) (*http.Response, error) { +func (c *Conn) runRequestWithMultipartPayload(url string, payload *multipartPayload) (*http.Response, error) { if url == "" { return nil, fmt.Errorf("unable to execute request, no target provided") } @@ -357,7 +367,7 @@ func (c *Conn) runRequestWithMultipartPayload(method, url string, payload *multi // pipeReader wrapped as a io.ReadSeeker to satisfy the gofish method signature reader := pipeReaderFakeSeeker{pipeReader} - return c.redfishwrapper.RunRawRequestWithHeaders(method, url, reader, form.FormDataContentType(), headers) + return c.redfishwrapper.RunRawRequestWithHeaders(http.MethodPost, url, reader, form.FormDataContentType(), headers) } // sets up the UpdateParameters MIMEHeader for the multipart form @@ -375,50 +385,50 @@ func updateParametersFormField(fieldName string, writer *multipart.Writer) (io.W return writer.CreatePart(h) } -// GetFirmwareInstallTaskQueued returns the redfish task object for a queued update task -func (c *Conn) GetFirmwareInstallTaskQueued(ctx context.Context, component string) (*gofishrf.Task, error) { +// FirmwareInstallStatus returns the status of the firmware install task queued +func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (state string, err error) { vendor, _, err := c.DeviceVendorModel(ctx) if err != nil { - return nil, errors.Wrap(err, "unable to determine device vendor, model attributes") + return state, errors.Wrap(err, "unable to determine device vendor, model attributes") } var task *gofishrf.Task - - // check an update task for the component is currently scheduled switch { case strings.Contains(vendor, constants.Dell): - task, err = c.getDellFirmwareInstallTaskScheduled(component) + task, err = c.dellJobAsRedfishTask(taskID) default: err = errors.Wrap( bmclibErrs.ErrNotImplemented, - "GetFirmwareInstallTask() for vendor: "+vendor, + "FirmwareInstallStatus() for vendor: "+vendor, ) } if err != nil { - return nil, err + return state, err } - return task, nil -} - -// purgeQueuedFirmwareInstallTask removes any existing queued firmware install task for the given component slug -func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component string) error { - vendor, _, err := c.DeviceVendorModel(ctx) - if err != nil { - return errors.Wrap(err, "unable to determine device vendor, model attributes") + if task == nil { + return state, errors.New("failed to lookup task status for task ID: " + taskID) } - // check an update task for the component is currently scheduled - switch { - case strings.Contains(vendor, constants.Dell): - err = c.dellPurgeScheduledFirmwareInstallJob(component) + state = strings.ToLower(string(task.TaskState)) + + // so much for standards... + switch state { + case "starting", "downloading", "downloaded": + return constants.FirmwareInstallInitializing, nil + case "running", "stopping", "cancelling", "scheduling": + return constants.FirmwareInstallRunning, nil + case "pending", "new": + return constants.FirmwareInstallQueued, nil + case "scheduled": + return constants.FirmwareInstallPowerCyleHost, nil + case "interrupted", "killed", "exception", "cancelled", "suspended", "failed": + return constants.FirmwareInstallFailed, nil + case "completed": + return constants.FirmwareInstallComplete, nil default: - err = errors.Wrap( - bmclibErrs.ErrNotImplemented, - "purgeFirmwareInstallTask() for vendor: "+vendor, - ) + return constants.FirmwareInstallUnknown + ": " + state, nil } - return err } From f20553716bc1fb65bd643a3b7018b9d2905f052a Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 4 Sep 2023 09:58:56 +0200 Subject: [PATCH 05/16] provides/redfish/tasks: WIP: add a generic Task listing method moved dell specific tasks handling into its own file --- providers/redfish/tasks.go | 173 +++++++------------------------- providers/redfish/tasks_test.go | 40 -------- 2 files changed, 36 insertions(+), 177 deletions(-) delete mode 100644 providers/redfish/tasks_test.go diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go index bc6aaeb1..5be3f2e7 100644 --- a/providers/redfish/tasks.go +++ b/providers/redfish/tasks.go @@ -1,173 +1,72 @@ package redfish import ( - "encoding/json" - "io" - "strconv" + "context" + "fmt" "strings" - bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" - "github.com/bmc-toolbox/common" + "github.com/bmc-toolbox/bmclib/v2/constants" + bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/pkg/errors" - - gofishcommon "github.com/stmcginnis/gofish/common" gofishrf "github.com/stmcginnis/gofish/redfish" ) -// Dell specific redfish methods - -var ( - componentSlugDellJobName = map[string]string{ - common.SlugBIOS: "Firmware Update: BIOS", - common.SlugBMC: "Firmware Update: iDRAC with Lifecycle Controller", - common.SlugNIC: "Firmware Update: Network", - common.SlugDrive: "Firmware Update: Serial ATA", - common.SlugStorageController: "Firmware Update: SAS RAID", - } -) - -type dellJob struct { - PercentComplete int - OdataID string `json:"@odata.id"` - StartTime string - CompletionTime string - ID string - Message string - Name string - JobState string - JobType string -} - -type dellJobResponseData struct { - Members []*dellJob -} - -// dellJobID formats and returns taskID as a Dell Job ID -func dellJobID(id string) string { - if !strings.HasPrefix(id, "JID") { - return "JID_" + id - } - - return id -} - -func (c *Conn) getDellFirmwareInstallTaskScheduled(slug string) (*gofishrf.Task, error) { - // get tasks by state - tasks, err := c.dellJobs("scheduled") +func (c *Conn) activeTask(ctx context.Context) (*gofishrf.Task, error) { + tasks, err := c.redfishwrapper.Tasks(ctx) if err != nil { return nil, err } - // filter to match the task Name based on the component slug - for _, task := range tasks { - if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { - return task, nil - } + for _, t := range tasks { + fmt.Println(t.TaskState) + fmt.Println(t.TaskStatus) + fmt.Println("xxx") } return nil, nil } -func (c *Conn) dellPurgeScheduledFirmwareInstallJob(slug string) error { - // get tasks by state - tasks, err := c.dellJobs("scheduled") +// GetFirmwareInstallTaskQueued returns the redfish task object for a queued update task +func (c *Conn) GetFirmwareInstallTaskQueued(ctx context.Context, component string) (*gofishrf.Task, error) { + vendor, _, err := c.DeviceVendorModel(ctx) if err != nil { - return err - } - - // filter to match the task Name based on the component slug - for _, task := range tasks { - if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { - err = c.dellPurgeJob(task.ID) - if err != nil { - return err - } - } + return nil, errors.Wrap(err, "unable to determine device vendor, model attributes") } - return nil -} - -func (c *Conn) dellPurgeJob(id string) error { - id = dellJobID(id) - - endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs/" + id + var task *gofishrf.Task - resp, err := c.redfishwrapper.Delete(endpoint) - if err != nil { - return errors.Wrap(bmcliberrs.ErrTaskPurge, err.Error()) + // check an update task for the component is currently scheduled + switch { + case strings.Contains(vendor, constants.Dell): + task, err = c.getDellFirmwareInstallTaskScheduled(component) + default: + //task, err = c.redfishwrapper.Tasks(ctx) } - if resp.StatusCode != 200 { - return errors.Wrap(bmcliberrs.ErrTaskPurge, "response code: "+resp.Status) - } - - return nil -} - -// dellFirmwareUpdateTaskStatus looks up the Dell Job and returns it as a redfish task object -func (c *Conn) dellJobAsRedfishTask(jobID string) (*gofishrf.Task, error) { - jobID = dellJobID(jobID) - - tasks, err := c.dellJobs("") if err != nil { return nil, err } - for _, task := range tasks { - if task.ID == jobID { - return task, nil - } - } - - return nil, errors.Wrap(bmcliberrs.ErrTaskNotFound, "task with ID not found: "+jobID) + return task, nil } -// dellJobs returns all dell jobs as redfish task objects -// state: optional -func (c *Conn) dellJobs(state string) ([]*gofishrf.Task, error) { - endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)" - - resp, err := c.redfishwrapper.Get(endpoint) +// purgeQueuedFirmwareInstallTask removes any existing queued firmware install task for the given component slug +func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component string) error { + vendor, _, err := c.DeviceVendorModel(ctx) if err != nil { - return nil, err - } - - if resp.StatusCode != 200 { - return nil, errors.New("dell jobs endpoint returned unexpected status code: " + strconv.Itoa(resp.StatusCode)) + return errors.Wrap(err, "unable to determine device vendor, model attributes") } - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - - data := dellJobResponseData{} - err = json.Unmarshal(body, &data) - if err != nil { - return nil, err - } - - tasks := []*gofishrf.Task{} - for _, job := range data.Members { - if state != "" && !strings.EqualFold(job.JobState, state) { - continue - } - - tasks = append(tasks, &gofishrf.Task{ - Entity: gofishcommon.Entity{ - ID: job.ID, - ODataID: job.OdataID, - Name: job.Name, - }, - Description: job.Name, - PercentComplete: job.PercentComplete, - StartTime: job.StartTime, - EndTime: job.CompletionTime, - TaskState: gofishrf.TaskState(job.JobState), - TaskStatus: gofishcommon.Health(job.Message), // abuse the TaskStatus to include any status message - }) + // check an update task for the component is currently scheduled + switch { + case strings.Contains(vendor, constants.Dell): + err = c.dellPurgeScheduledFirmwareInstallJob(component) + default: + err = errors.Wrap( + bmclibErrs.ErrNotImplemented, + "purgeFirmwareInstallTask() for vendor: "+vendor, + ) } - return tasks, nil + return err } diff --git a/providers/redfish/tasks_test.go b/providers/redfish/tasks_test.go deleted file mode 100644 index bf1a376b..00000000 --- a/providers/redfish/tasks_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package redfish - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/assert" -) - -// handler registered in redfish_test.go -func dellJobs(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" { - w.WriteHeader(http.StatusNotFound) - } - - _, _ = w.Write(jsonResponse(r.RequestURI)) -} - -func Test_dellFirmwareUpdateTask(t *testing.T) { - // see fixtures/v1/dell/jobs.json for the job IDs - // completed job - status, err := mockClient.dellJobAsRedfishTask("467767920358") - if err != nil { - t.Fatal(err) - } - - assert.NotNil(t, status) - assert.Equal(t, "2022-03-08T16:02:33", status.EndTime) - assert.Equal(t, "2022-03-08T15:59:52", status.StartTime) - assert.Equal(t, 100, status.PercentComplete) - assert.Equal(t, "Completed", string(status.TaskState)) - assert.Equal(t, "Job completed successfully.", string(status.TaskStatus)) -} - -func Test_dellPurgeScheduledFirmwareInstallJob(t *testing.T) { - err := mockClient.dellPurgeScheduledFirmwareInstallJob("bios") - if err != nil { - t.Fatal(err) - } -} From f1fd7c61e7551eec7389b6044165f71472cd8f60 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 11:35:58 +0200 Subject: [PATCH 06/16] providers/redfish/firmware: Fix parameters mismatch for unstructuredHttpUpload --- providers/redfish/firmware.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index d8b36c3b..a182b36a 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -171,7 +171,7 @@ func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, upd return c.runRequestWithMultipartPayload(url, payload) } -func (c *Conn) unstructuredHttpUpload(ctx context.Context, url string, update io.Reader) (*http.Response, error) { +func (c *Conn) unstructuredHttpUpload(ctx context.Context, url, applyAt string, update io.Reader) (*http.Response, error) { } From b5eda2a6fc531d6457812e864886a710661d84e7 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Wed, 13 Sep 2023 12:59:57 +0200 Subject: [PATCH 07/16] providers/redfish/firmware: Use correct type for multipartHTTP update --- providers/redfish/firmware.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index a182b36a..a7758469 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -106,7 +106,7 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f switch installMethod { case multipartHttpUpload: var uploadErr error - resp, uploadErr = c.multipartHTTPUpload(ctx, installURI, applyAt, reader) + resp, uploadErr = c.multipartHTTPUpload(ctx, installURI, applyAt, updateFile) if uploadErr != nil { return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, uploadErr.Error()) } @@ -143,7 +143,7 @@ type multipartPayload struct { updateFile *os.File } -func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, update io.Reader) (*http.Response, error) { +func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, update *os.File) (*http.Response, error) { if url == "" { return nil, fmt.Errorf("unable to execute request, no target provided") } @@ -163,9 +163,9 @@ func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, upd } // payload ordered in the format it ends up in the multipart form - payload := []map[string]io.Reader{ - {"UpdateParameters": bytes.NewReader(parameters)}, - {"UpdateFile": update}, + payload := &multipartPayload{ + updateParameters: []byte(parameters), + updateFile: update, } return c.runRequestWithMultipartPayload(url, payload) From c38881ef5fb60e7162940f69aa9c1c9855443cc9 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 11:27:53 +0200 Subject: [PATCH 08/16] providers/redfish/firmware: implement unstructured HTTP updates --- providers/redfish/firmware.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index a7758469..2e18870e 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -173,6 +173,15 @@ func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, upd func (c *Conn) unstructuredHttpUpload(ctx context.Context, url, applyAt string, update io.Reader) (*http.Response, error) { + if url == "" { + return nil, fmt.Errorf("unable to execute request, no target provided") + } + + b, _ := io.ReadAll(update) + + payloadReadSeeker := bytes.NewReader(b) + return c.redfishwrapper.RunRawRequestWithHeaders(http.MethodPost, url, payloadReadSeeker, "application/octet-stream", nil) + } // firmwareUpdateMethodURI returns the updateMethod and URI From 1cb7fb9fe85a5d2bb9bfe9be5a8f3c434cba2834 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 15:06:53 +0200 Subject: [PATCH 09/16] providers/redfish/tasks: add a function to get current task status for OpenBMC --- providers/redfish/tasks.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go index 5be3f2e7..04eb255b 100644 --- a/providers/redfish/tasks.go +++ b/providers/redfish/tasks.go @@ -2,12 +2,14 @@ package redfish import ( "context" + "encoding/json" "fmt" "strings" "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/pkg/errors" + gofishcommon "github.com/stmcginnis/gofish/common" gofishrf "github.com/stmcginnis/gofish/redfish" ) @@ -70,3 +72,25 @@ func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component str return err } + +func (c *Conn) openbmcGetTask(jsonstr []byte) (task *gofishrf.Task, err error) { + + type TaskStatus struct { + TaskState string + TaskStatus string + } + + var status TaskStatus + + err = json.Unmarshal(jsonstr, &status) + if err != nil { + fmt.Println(err) + } else { + task = &gofishrf.Task{ + TaskState: gofishrf.TaskState(status.TaskState), + TaskStatus: gofishcommon.Health(status.TaskStatus), + } + } + + return task, err +} From 1018c580f855334532cc6a97347fd040299a65c5 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 12:58:36 +0200 Subject: [PATCH 10/16] providers/redfish/firmware: get task ID for OpenBMC, return Task object for OpenBMC status --- providers/redfish/firmware.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index 2e18870e..99475c81 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -131,8 +131,16 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f // The response contains a location header pointing to the task URI // Location: /redfish/v1/TaskService/Tasks/JID_467696020275 - if strings.Contains(resp.Header.Get("Location"), "JID_") { + var location = resp.Header.Get("Location") + if strings.Contains(location, "JID_") { taskID = strings.Split(resp.Header.Get("Location"), "JID_")[1] + } else if strings.Contains(location, "/Monitor") { + // OpenBMC returns a monitor URL in Location + // Location: /redfish/v1/TaskService/Tasks/12/Monitor + splits := strings.Split(location, "/") + taskID = splits[5] + } else { + return "", bmclibErrs.ErrTaskNotFound } return taskID, nil @@ -405,6 +413,24 @@ func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, compon switch { case strings.Contains(vendor, constants.Dell): task, err = c.dellJobAsRedfishTask(taskID) + case strings.Contains(vendor, constants.Packet) || strings.Contains(vendor, constants.Equinix): + + resp, _ := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks/" + taskID) + if resp.StatusCode != 200 { + err = errors.Wrap( + bmclibErrs.ErrFirmwareInstall, + "HTTP Error: "+fmt.Sprint(resp.StatusCode), + ) + + state = "failed" + break + } + + data, _ := io.ReadAll(resp.Body) + resp.Body.Close() + + task, err = c.openbmcGetTask(data) + default: err = errors.Wrap( bmclibErrs.ErrNotImplemented, From 56a836a6aa67327c29ee6e49b51ae1a195331955 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 17:21:37 +0200 Subject: [PATCH 11/16] providers/redfish: Add tests for openBMC status --- providers/redfish/firmware.go | 6 +++- providers/redfish/firmware_test.go | 44 ++++++++++++++++++++++++++++++ providers/redfish/main_test.go | 1 + providers/redfish/tasks_test.go | 27 ++++++++++++++++++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 providers/redfish/tasks_test.go diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index 99475c81..b6ddf3ab 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -180,7 +180,6 @@ func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, upd } func (c *Conn) unstructuredHttpUpload(ctx context.Context, url, applyAt string, update io.Reader) (*http.Response, error) { - if url == "" { return nil, fmt.Errorf("unable to execute request, no target provided") } @@ -409,6 +408,11 @@ func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, compon return state, errors.Wrap(err, "unable to determine device vendor, model attributes") } + // component is not used, we hack it for tests, easier than mocking + if component == "testOpenbmc" { + vendor = "defaultVendor" + } + var task *gofishrf.Task switch { case strings.Contains(vendor, constants.Dell): diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go index b0fbedec..187244ab 100644 --- a/providers/redfish/firmware_test.go +++ b/providers/redfish/firmware_test.go @@ -233,3 +233,47 @@ func TestFirmwareUpdateCompatible(t *testing.T) { t.Fatal(err) } } + + +// referenced in main_test.go +func openbmcStatus(w http.ResponseWriter, r *http.Request) { + + if r.URL.Path != "/redfish/v1/TaskService/Tasks/15" { + // return an HTTP error, don't care to return correct data after + http.Error(w, "404 page not found:"+r.URL.Path, http.StatusNotFound) + } + + mytask := `{ + "@odata.id": "/redfish/v1/TaskService/Tasks/15", + "@odata.type": "#Task.v1_4_3.Task", + "Id": "15", + "Messages": [ + { + "@odata.type": "#Message.v1_1_1.Message", + "Message": "The task with Id '15' has started.", + "MessageArgs": [ + "15" + ], + "MessageId": "TaskEvent.1.0.3.TaskStarted", + "MessageSeverity": "OK", + "Resolution": "None." + } + ], + "Name": "Task 15", + "TaskState": "TestState", + "TaskStatus": "TestStatus" +} +` + _, _ = w.Write([]byte(mytask)) + +} + +func Test_FirmwareInstall2(t *testing.T) { + state, err := mockClient.FirmwareInstallStatus(context.TODO(), "", "testOpenbmc", "15") + if err != nil { + t.Fatal(err) + } + if state != "unknown: teststate" { + t.Fatal("Wrong test state:", state) + } +} diff --git a/providers/redfish/main_test.go b/providers/redfish/main_test.go index 924dfe8a..379292ba 100644 --- a/providers/redfish/main_test.go +++ b/providers/redfish/main_test.go @@ -59,6 +59,7 @@ func TestMain(m *testing.M) { handler.HandleFunc("/redfish/v1/SessionService/Sessions", sessionService) handler.HandleFunc("/redfish/v1/UpdateService/MultipartUpload", multipartUpload) handler.HandleFunc("/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)", dellJobs) + handler.HandleFunc("/redfish/v1/TaskService/Tasks/", openbmcStatus) return httptest.NewTLSServer(handler) }() diff --git a/providers/redfish/tasks_test.go b/providers/redfish/tasks_test.go new file mode 100644 index 00000000..2f3f91ae --- /dev/null +++ b/providers/redfish/tasks_test.go @@ -0,0 +1,27 @@ +package redfish + +import ( + "testing" +) + +func Test_openbmcGetTask(t *testing.T) { + var err error + + task, err := mockClient.openbmcGetTask("15") + if err != nil { + t.Fatal(err) + } + if task.TaskState != "TestState" { + t.Fatal("Wrong test state:", task.TaskState) + } + + // inexistent + task, err = mockClient.openbmcGetTask("151515") + if task != nil { + t.Fatal("Task should be nil, but got:", task) + } + if err == nil { + t.Fatal("err shouldn't be nil:", err) + } + +} From c49aa4949fb9e9d8fd468aca466c057805dab75e Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Tue, 5 Sep 2023 11:42:40 +0200 Subject: [PATCH 12/16] providers/redfish: Move default task logic into GetTask --- providers/redfish/firmware.go | 26 +++----------------------- providers/redfish/firmware_test.go | 1 - providers/redfish/tasks.go | 22 ++++++++++++++++++++-- providers/redfish/tasks_test.go | 6 +++--- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index b6ddf3ab..c8d753c1 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -184,9 +184,10 @@ func (c *Conn) unstructuredHttpUpload(ctx context.Context, url, applyAt string, return nil, fmt.Errorf("unable to execute request, no target provided") } + // TODO: transform this to read the update so that we don't hold the data in memory b, _ := io.ReadAll(update) - payloadReadSeeker := bytes.NewReader(b) + return c.redfishwrapper.RunRawRequestWithHeaders(http.MethodPost, url, payloadReadSeeker, "application/octet-stream", nil) } @@ -417,29 +418,8 @@ func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, compon switch { case strings.Contains(vendor, constants.Dell): task, err = c.dellJobAsRedfishTask(taskID) - case strings.Contains(vendor, constants.Packet) || strings.Contains(vendor, constants.Equinix): - - resp, _ := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks/" + taskID) - if resp.StatusCode != 200 { - err = errors.Wrap( - bmclibErrs.ErrFirmwareInstall, - "HTTP Error: "+fmt.Sprint(resp.StatusCode), - ) - - state = "failed" - break - } - - data, _ := io.ReadAll(resp.Body) - resp.Body.Close() - - task, err = c.openbmcGetTask(data) - default: - err = errors.Wrap( - bmclibErrs.ErrNotImplemented, - "FirmwareInstallStatus() for vendor: "+vendor, - ) + task, err = c.GetTask(taskID) } if err != nil { diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go index 187244ab..5731d16d 100644 --- a/providers/redfish/firmware_test.go +++ b/providers/redfish/firmware_test.go @@ -234,7 +234,6 @@ func TestFirmwareUpdateCompatible(t *testing.T) { } } - // referenced in main_test.go func openbmcStatus(w http.ResponseWriter, r *http.Request) { diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go index 04eb255b..4a5390bb 100644 --- a/providers/redfish/tasks.go +++ b/providers/redfish/tasks.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "strings" "github.com/bmc-toolbox/bmclib/v2/constants" @@ -73,7 +74,24 @@ func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component str return err } -func (c *Conn) openbmcGetTask(jsonstr []byte) (task *gofishrf.Task, err error) { +// GetTask returns the current Task fir the given TaskID +func (c *Conn) GetTask(taskID string) (task *gofishrf.Task, err error) { + + resp, err := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks/" + taskID) + if err != nil { + return nil, err + } + if resp.StatusCode != 200 { + err = errors.Wrap( + bmclibErrs.ErrFirmwareInstallStatus, + "HTTP Error: "+fmt.Sprint(resp.StatusCode), + ) + + return nil, err + } + + data, _ := io.ReadAll(resp.Body) + resp.Body.Close() type TaskStatus struct { TaskState string @@ -82,7 +100,7 @@ func (c *Conn) openbmcGetTask(jsonstr []byte) (task *gofishrf.Task, err error) { var status TaskStatus - err = json.Unmarshal(jsonstr, &status) + err = json.Unmarshal(data, &status) if err != nil { fmt.Println(err) } else { diff --git a/providers/redfish/tasks_test.go b/providers/redfish/tasks_test.go index 2f3f91ae..f763b1bd 100644 --- a/providers/redfish/tasks_test.go +++ b/providers/redfish/tasks_test.go @@ -4,10 +4,10 @@ import ( "testing" ) -func Test_openbmcGetTask(t *testing.T) { +func Test_GetTask(t *testing.T) { var err error - task, err := mockClient.openbmcGetTask("15") + task, err := mockClient.GetTask("15") if err != nil { t.Fatal(err) } @@ -16,7 +16,7 @@ func Test_openbmcGetTask(t *testing.T) { } // inexistent - task, err = mockClient.openbmcGetTask("151515") + task, err = mockClient.GetTask("151515") if task != nil { t.Fatal("Task should be nil, but got:", task) } From 64802530b9b65a4dea530c1cb9106c828662d1d2 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Mon, 4 Sep 2023 17:11:57 +0200 Subject: [PATCH 13/16] providers/redfish: Detect duplicate update requests --- providers/redfish/firmware.go | 37 +++++++++++++++++------------------ providers/redfish/tasks.go | 3 ++- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index c8d753c1..b6d68a81 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -69,26 +69,25 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String()) } - // TODO; uncomment once obmc support is implemented for tasks // list redfish firmware install task if theres one present - // task, err := c.GetFirmwareInstallTaskQueued(ctx, component) - // if err != nil { - // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - // } - // - // if task != nil { - // msg := fmt.Sprintf("task for %s firmware install present: %s", component, task.ID) - // c.Log.V(2).Info("warn", msg) - // - // if forceInstall { - // err = c.purgeQueuedFirmwareInstallTask(ctx, component) - // if err != nil { - // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) - // } - // } else { - // return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, msg) - // } - // } + task, err := c.GetFirmwareInstallTaskQueued(ctx, component) + if err != nil { + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) + } + + if task != nil { + msg := fmt.Sprintf("task for %s firmware install present: %s", component, task.ID) + c.Log.V(2).Info("warn", msg) + + if forceInstall { + err = c.purgeQueuedFirmwareInstallTask(ctx, component) + if err != nil { + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error()) + } + } else { + return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, msg) + } + } // override the gofish HTTP client timeout, // since the context timeout is set at Open() and is at a lower value than required for this operation. diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go index 4a5390bb..4ebf2055 100644 --- a/providers/redfish/tasks.go +++ b/providers/redfish/tasks.go @@ -20,6 +20,7 @@ func (c *Conn) activeTask(ctx context.Context) (*gofishrf.Task, error) { return nil, err } + // TODO: correctly get an update task if there is one, using redfish API for _, t := range tasks { fmt.Println(t.TaskState) fmt.Println(t.TaskStatus) @@ -43,7 +44,7 @@ func (c *Conn) GetFirmwareInstallTaskQueued(ctx context.Context, component strin case strings.Contains(vendor, constants.Dell): task, err = c.getDellFirmwareInstallTaskScheduled(component) default: - //task, err = c.redfishwrapper.Tasks(ctx) + task, err = c.activeTask(ctx) } if err != nil { From b85640ef6f69875245c2c7086b4e649b0758637c Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Wed, 6 Sep 2023 09:40:54 +0200 Subject: [PATCH 14/16] providers/redfish/firmware: Add TaskIDFromLocationURI and tests --- providers/redfish/firmware.go | 26 ++++++++++++++++++++------ providers/redfish/firmware_test.go | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index b6d68a81..48f95287 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -131,14 +131,28 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f // The response contains a location header pointing to the task URI // Location: /redfish/v1/TaskService/Tasks/JID_467696020275 var location = resp.Header.Get("Location") - if strings.Contains(location, "JID_") { - taskID = strings.Split(resp.Header.Get("Location"), "JID_")[1] - } else if strings.Contains(location, "/Monitor") { + + taskID, err = TaskIDFromLocationURI(location) + + return taskID, err +} + +func TaskIDFromLocationURI(uri string) (taskID string, err error) { + + if strings.Contains(uri, "JID_") { + taskID = strings.Split(uri, "JID_")[1] + } else if strings.Contains(uri, "/Monitor") { // OpenBMC returns a monitor URL in Location // Location: /redfish/v1/TaskService/Tasks/12/Monitor - splits := strings.Split(location, "/") - taskID = splits[5] - } else { + splits := strings.Split(uri, "/") + if len(splits) >= 6 { + taskID = splits[5] + } else { + taskID = "" + } + } + + if taskID == "" { return "", bmclibErrs.ErrTaskNotFound } diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go index 5731d16d..c42c2bfa 100644 --- a/providers/redfish/firmware_test.go +++ b/providers/redfish/firmware_test.go @@ -276,3 +276,23 @@ func Test_FirmwareInstall2(t *testing.T) { t.Fatal("Wrong test state:", state) } } + +func Test_TaskIDFromLocationURI(t *testing.T) { + var task string + var err error + + task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/JID_467696020275") + if err != nil || task != "467696020275" { + t.Fatal("Wrong task ID 467696020275. task,err=", task, err) + } + + task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/12/Monitor") + if err != nil || task != "12" { + t.Fatal("Wrong task ID 12. task,err=", task, err) + } + + task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/NO-TASK-ID") + if err == nil { + t.Fatal("Should return an error. task,err=", task, err) + } +} From 4dec4c689145282b566e44355d5c21709c123408 Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Thu, 14 Sep 2023 10:07:19 +0200 Subject: [PATCH 15/16] providers/redfish/firmware: removed firmwareUpdateCompatible --- providers/redfish/firmware.go | 24 +----------------------- providers/redfish/firmware_test.go | 7 ------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go index 48f95287..9db2f436 100644 --- a/providers/redfish/firmware.go +++ b/providers/redfish/firmware.go @@ -186,7 +186,7 @@ func (c *Conn) multipartHTTPUpload(ctx context.Context, url, applyAt string, upd // payload ordered in the format it ends up in the multipart form payload := &multipartPayload{ updateParameters: []byte(parameters), - updateFile: update, + updateFile: update, } return c.runRequestWithMultipartPayload(url, payload) @@ -227,28 +227,6 @@ func (c *Conn) firmwareInstallMethodURI(ctx context.Context) (method installMeth return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "unsupported update method") } -// firmwareUpdateCompatible retuns an error if the firmware update process for the BMC is not supported -func (c *Conn) firmwareUpdateCompatible(ctx context.Context) (err error) { - updateService, err := c.redfishwrapper.UpdateService() - if err != nil { - return err - } - - // TODO: check for redfish version - - // update service disabled - if !updateService.ServiceEnabled { - return errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "service disabled") - } - - // for now we expect multipart HTTP push update support - if updateService.MultipartHTTPPushURI == "" { - return errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "Multipart HTTP push updates not supported") - } - - return nil -} - // pipeReaderFakeSeeker wraps the io.PipeReader and implements the io.Seeker interface // to meet the API requirements for the Gofish client https://github.com/stmcginnis/gofish/blob/46b1b33645ed1802727dc4df28f5d3c3da722b15/client.go#L434 // diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go index c42c2bfa..16b224cc 100644 --- a/providers/redfish/firmware_test.go +++ b/providers/redfish/firmware_test.go @@ -227,13 +227,6 @@ func TestMultipartPayloadSize(t *testing.T) { } } -func TestFirmwareUpdateCompatible(t *testing.T) { - err := mockClient.firmwareUpdateCompatible(context.TODO()) - if err != nil { - t.Fatal(err) - } -} - // referenced in main_test.go func openbmcStatus(w http.ResponseWriter, r *http.Request) { From 0a859e1b900ce24cf8115f5ea98974d6436c022e Mon Sep 17 00:00:00 2001 From: Olivier FAURAX Date: Thu, 14 Sep 2023 18:41:46 +0200 Subject: [PATCH 16/16] providers/redfish/tasks: check if a task is running before update --- providers/redfish/tasks.go | 62 +++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go index 4ebf2055..49bd1713 100644 --- a/providers/redfish/tasks.go +++ b/providers/redfish/tasks.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "regexp" "strings" "github.com/bmc-toolbox/bmclib/v2/constants" @@ -15,16 +16,61 @@ import ( ) func (c *Conn) activeTask(ctx context.Context) (*gofishrf.Task, error) { - tasks, err := c.redfishwrapper.Tasks(ctx) + resp, err := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks") if err != nil { + fmt.Println("err1", err) return nil, err } + if resp.StatusCode != 200 { + err = errors.Wrap( + bmclibErrs.ErrFirmwareInstallStatus, + "HTTP Error: "+fmt.Sprint(resp.StatusCode), + ) + + return nil, err + } + + data, _ := io.ReadAll(resp.Body) + resp.Body.Close() + + type TaskId struct { + OdataID string `json:"@odata.id"` + TaskState string + TaskStatus string + } - // TODO: correctly get an update task if there is one, using redfish API - for _, t := range tasks { - fmt.Println(t.TaskState) - fmt.Println(t.TaskStatus) - fmt.Println("xxx") + type Tasks struct { + Members []TaskId + } + + var status Tasks + + err = json.Unmarshal(data, &status) + if err != nil { + fmt.Println(err) + } + + // For each task, check if it's running + // It's usually the latest that is running, so it would be faster to + // start by the end, but an easy way to do this is only available in go 1.21 + // for _, t := range slices.Reverse(status.Members) { // when go 1.21 + for _, t := range status.Members { + re := regexp.MustCompile("/redfish/v1/TaskService/Tasks/([0-9]+)") + taskmatch := re.FindSubmatch([]byte(t.OdataID)) + if len(taskmatch) < 1 { + continue + } + + tasknum := string(taskmatch[1]) + + task, err := c.GetTask(tasknum) + if err != nil { + continue + } + + if task.TaskState == "Running" { + return task, nil + } } return nil, nil @@ -67,8 +113,8 @@ func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component str err = c.dellPurgeScheduledFirmwareInstallJob(component) default: err = errors.Wrap( - bmclibErrs.ErrNotImplemented, - "purgeFirmwareInstallTask() for vendor: "+vendor, + bmclibErrs.ErrFirmwareInstall, + "Update is already running", ) }