stmcginnis / gofish

Gofish is a Golang client library for DMTF Redfish and SNIA Swordfish interaction.
BSD 3-Clause "New" or "Revised" License
204 stars 109 forks source link

expose the ActionInfo property under Actions under the Managers colle… #345

Closed iamsli closed 2 months ago

iamsli commented 2 months ago

exposes the ActionInfo property under Actions under the Managers collection

per Redfish docs ActionInfo is a common property across the Actions object. Exposing that property in the Managers collection

stmcginnis commented 2 months ago

Thanks. It's a recommended property, but from what I've seen there are only a small number of vendors that actually implement it so far.

This just captures the string to a private property. Is there more you are looking at adding to do something with this? It may be better to add it to the ActionInfo struct and update this spot to use a struct that extends that to get the AllowedResetTypes.

iamsli commented 2 months ago

ah thank you. i didn't see that struct. i will update this now. for reference this is the output i'm seeing:

  "Actions": {
    "#Manager.Reset": {
      "@Redfish.ActionInfo": "/redfish/v1/Managers/Bluefield_BMC/ResetActionInfo",
      "target": "/redfish/v1/Managers/Bluefield_BMC/Actions/Manager.Reset"
    },

a call to manager.Reset() is failing because nothing is reported under allowableValues even though the vendor has this implemented, but to find this info we have to get it from the ActionInfo endpoint. How do you think this should be handled?

stmcginnis commented 2 months ago

a call to manager.Reset() is failing because nothing is reported under allowableValues

That wouldn't be why the reset call is failing. If there are no reported allowable reset types, it will just skip performing the check up front. So the type of reset you are requesting must not be one of the supported kinds, but that wouldn't change the result you are getting. It's just a little more efficient for Gofish to tell you right away if something isn't supported, versus attempting to perform the action and the system telling you the same thing.

So you most likely just need to change the ResetType that you are passing in. Gofish would just give you a slightly better failure message maybe, but it still wouldn't tell you what the valid values would be.

iamsli commented 2 months ago

correct me if i'm wrong but the gofish Reset() makes a Reset call with an empty payload when AllowableValues is empty. In this case, AllowableValues is empty so i'm are making an empty call and getting an error message. The reset type i'm passing in via gofish and via curl are the same. the curl call is working but not the gofish call

stmcginnis commented 2 months ago

Ah, you're right. Looks like we had to do that to make HPE happy.

So I think this PR is part of the way there. If supported types is empty, then we could check if there is an action info link and make an extra call to query the system for supported types. Or to minimize overhead, I guess the presence of that action info link could be enough to differentiate from the HPE systems that don't report anything at all, so if so just put the ResetType in the payload.

Actually querying the system using the link would be more correct, just thinking about minimizing that round trip time. If you want to try things out and see what works best, I could go either way.

Thanks!

iamsli commented 2 months ago

I agree with you on minimizing roundtrip time and implemented it so that it just checks if actionInfo is there. LMK what you think!