Support unstructured HTTP update #342
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #342 +/- ##
==========================================
+ Coverage 42.67% 43.11% +0.44%
==========================================
Files 44 44
Lines 3691 3746 +55
==========================================
+ Hits 1575 1615 +40
- Misses 1937 1946 +9
- Partials 179 185 +6
☔ View full report in Codecov by Sentry. |
| @@ -106,7 +106,23 @@ func (c *Conn) FirmwareInstall(ctx context.Context, component, applyAt string, f | |||
| "UpdateFile": reader, | |||
| } | |||
|
|
|||
| resp, err := c.runRequestWithMultipartPayload(http.MethodPost, "/redfish/v1/UpdateService/MultipartUpload", payload) | |||
| updateService, err := c.redfishwrapper.UpdateService() | |||
There was a problem hiding this comment.
I suggest we update the firmwareUpdateCompatible() with the logic below, to return the update URI and it returns an error if the URI is not supported.
There was a problem hiding this comment.
The URI is not enough to know the kind of update. Once the URI will be returned, we won't be able to know if we need to perform a multipart update or an unstructured one.
providers/redfish/firmware.go
Outdated
| @@ -138,6 +161,47 @@ 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): | |||
| resp, _ := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks/" + taskID) | |||
There was a problem hiding this comment.
You might be able to use the TaskService method from Gofish - https://github.com/stmcginnis/gofish/blob/6e8a2909577b8424e751890a5c432352f227097b/serviceroot.go#L257
There was a problem hiding this comment.
I tried but it seems all Clients are not created equal:
../../providers/redfish/firmware.go:185:33: cannot use c.redfishwrapper (variable of type *redfishwrapper.Client) as type "github.com/stmcginnis/gofish/common".Client in argument to gofishrf.GetTask:
*redfishwrapper.Client does not implement "github.com/stmcginnis/gofish/common".Client (missing DeleteWithHeaders method)
| return nil, fmt.Errorf("unable to execute request, no target provided") | ||
| } | ||
|
|
||
| b, _ := io.ReadAll(payload) |
There was a problem hiding this comment.
I'd like to see if we have other options than reading the whole file into memory
There was a problem hiding this comment.
The payloadReadSeeker needs to be of type io.ReadSeeker meaning we need to be able to read and seek anywhere in the data, so we're forced to have it all.
On the multipart update, I see bytes.NewReader(payloadBuffer.Bytes()).
I'm not sure if the whole update is not in memory at that time.
There was a problem hiding this comment.
Thanks for pointing this out, I realised that this was a regression and its supposed to be a stream upload #345
ofaurax
force-pushed
the
unstructured-update-support
branch
from
a3f230e
to
c57b61d
Compare
ofaurax
force-pushed
the
unstructured-update-support
branch
from
c57b61d
to
9365e6c
Compare
| Intel = "Intel" | ||
| // Equinix, formerly Packet |
There was a problem hiding this comment.
what are these identifying? does Packet and Equinix have hardware like Intel and or Quanta does? or do they have a custom redfish implementation?
There was a problem hiding this comment.
maybe this is related to a custom openBMC build Equinix/Packet is running with redfish in it? If this is the case we should make that clear here. Without it I'm left to assume this is hardware that is built by Equinix/Packet like Supermico or Dell hardware.
There was a problem hiding this comment.
In this case the hardware was manufactured with the vendor attribute set to Packet.
|
The work on this PR was moved into #346 since we reworked the implementation. |
What does this PR implement/change/remove?
Even if deprecated, this PR adds support for the "unstructured HTTP update" from Redfish specification.
The current implementation only support multipart update.
This PR adds a fallback to "unstructured HTTP update" as a fallback of HTTP multipart is not available.
Checklist
The BMC firmware and/or BIOS versions that this change applies to (if applicable)
OpenBMC redfish implementation
Description for changelog/release notes