ibm-openbmc / dev

Product Development Project Mgmt and Tracking
16 stars 2 forks source link

Redfish: Code Update - Support for MultipartHttpPushUri #1372

Closed geissonator closed 1 year ago

geissonator commented 4 years ago

The latest UpdateService schema (https://redfish.dmtf.org/schemas/v1/UpdateService.v1_6_0.json) described a new MultipartHttpPushUri.

This will require getting multi part push capability into bmcweb and then supporting this new property in the redfish update code within bmcweb.

I see some work already with https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/27113 to get this into bmcweb.

I don't think there's a whole lot we need extra in the MultipartHttpPushUri vs. the HttpPushUri. Maybe the ApplyTime. But getting support in for this will have us on an official update interface.

geissonator commented 4 years ago

This function wont be able to be fully merged until https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/27113 is available. Some work could be done now though with understanding what the Redfish API will look like and what changes in https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/update_service.hpp will be required to support it.

mine260309 commented 4 years ago

@geissonator Could you help to clarify the scope of the tasks related to MultipartHttpPushUri? There is some work already done by @asmithakarun (I do not know if there is a related task or not).

Preferably, we could have two (or more) tasks to track the separated efforts.

mine260309 commented 4 years ago

Assign to me temporarily to get this task's scope.

geissonator commented 4 years ago

Sounds like the needed multi-part library has been upstreamed to yocto but we are stuck on our python3 migration with bringing down that update.

mine260309 commented 4 years ago

From @tomjoseph83, the multi-part library is introduced not for this task. So except for the library, all other tasks related to MultipartHttpPushUri in Redfish Code Update will be tracked in this task.

lxwinspur commented 4 years ago

Hi, @geissonator . I did not make any changes to the bmcweb code and I used the following command and the test UpdateService is well.

curl -k -H "X-Auth-Token: $token" -H "Content-Type: multipart/form-data" -X POST -T <image file path> https://${bmc}/redfish/v1/UpdateService

curl -k -H "X-Auth-Token: $token" -H "Content-Type: multipart/form-data" -X POST -T obmc-phosphor-image-fp5280g2-20200511062717.static.mtd.tar https://${bmc}/redfish/v1/UpdateService
{
  "@Message.ExtendedInfo": [
    {
      "@odata.type": "/redfish/v1/$metadata#Message.v1_0_0.Message",
      "Message": "Successfully Completed Request",
      "MessageArgs": [],
      "MessageId": "Base.1.4.0.Success",
      "Resolution": "None",
      "Severity": "OK"
    }
  ]
}

so I want to know what other changes I need to make in this task. thanks :)

geissonator commented 4 years ago

Hey @lxwinspur , thanks for taking a look at this. A pretty good overview/example can be found here:

http://redfish.dmtf.org/schemas/DSP0266_1.8.0.html#multipart-http-push-updates-a-id-http-push-update-a-

Basically, we need to define a URI and fill it into the new MultipartHttpPushUri property. The example seems to use /redfish/v1/UpdateService/upload

We would then need to write the code within bmcweb to handle a POST to this new URI and to process the potential UpdateParameters within that POST operation. I think we can keep it to properties we currently support which I think is really just ApplyTime. Support for other properties can be added as needed in the future.

https://redfish.dmtf.org/schemas/v1/UpdateService.v1_8_0.json is the latest schema for all this.

lxwinspur commented 4 years ago

review: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/32174

gtmills commented 4 years ago

@lxwinspur I see the commit as Work In Progress in Gerrit. What is the outlook here?

lxwinspur commented 4 years ago

@gtmills @Ed Tanous suggests using the following way to parse multipart-parser https://github.com/FooBarWidget/multipart-parser. Therefore, I removed the mimetic library and changing the related code.

gtmills commented 4 years ago

@lxwinspur Any progress here?

lxwinspur commented 4 years ago

@gtmills I need to rely on Ed's commit as follows: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974

but I tested it and found a minor problem and I need to discuss with Ed

Also, after the above commit merged, I need to update my code. Thanks:)

gtmills commented 3 years ago

If we want to get https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974 in, we will have to do the work.

mzipse commented 3 years ago

@lxwinspur , are you or your team willing to go work on finishing https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974 ? This isn't at the top of the priority list but it's highly desirable.

lxwinspur commented 3 years ago

@mzipse Yes, I have fixed this commit, and wait for Ed to review it, But it looks like he did not do anything.

So, should we downstream this patch to ibm-openbmc?

gtmills commented 3 years ago

Could someone from IPS review this?

So, should we downstream this patch to ibm-openbmc?

Lets keep going upstream on it but yeah by the end of the month we can pull in downstream

gtmills commented 3 years ago

Can we get unit tests added to https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974

lxwinspur commented 3 years ago

@gtmills I have added unit tests, please take a look again, thanks

gtmills commented 2 years ago

I see comments on https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974 and marked as WIP, can we keep going on this? :)

gtmills commented 2 years ago

@lxwinspur Can you resolve merge conflicts on https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/32174/ as well

lxwinspur commented 2 years ago

@lxwinspur Can you resolve merge conflicts on https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/32174/ as well

Sure, But please review this patch first. https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/34974

geissonator commented 2 years ago

How we looking here @lxwinspur ?

lxwinspur commented 2 years ago

How we looking here @lxwinspur ?

https://gerrit.openbmc.org/c/openbmc/bmcweb/+/32174 waiting for review

lxwinspur commented 1 year ago

Merged by: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/32174

@geissonator @mzipse Can we close this issue?

geissonator commented 1 year ago

Nice job George!