OEM actions implementation example

[mikeletux]

Hello @stmcginnis,

Here it comes an interesting one. (In reference to #142) First of all, I don't expect you to merge this code (who knows though!).

I've came up with this as "plugin" system for Gofish. This has been implemented on top of update service as a PoC. Turns out the main idea is to leave an Oem interface{} in all structs that may contain OEM data/actions so we can store different structs (Dell/HP/Lenovo/...) with information about OEM info/actions from vendors.

Also I created a package called dell for the sake of this PoC that contains OEM structs for update service for Dell redfish implementation. For different vendors we would need to create additional packages.

Here comes the trick. I've added a function to Client interface (as well as for APIClient and TestClient implementations) called GetVendor(). This function becomes handy when it comes to figure out at all UnmarshalJSON methods in redfish package which is the vendor we are dealing with. Known that, we could grab the correct structs and use them to parse OEM data. (Check the switch on redfish/updateservice.go).

Also I've updated the tests to make it work with this PoC design. Turns out example data comes with Dell OEM actions 😄

In case the user don't want to use the plugin system, I've added a new attribute to Config called OemExtensions so in case this is not set or set to false, the function GetVendor() will return an empty string, not including any vendor OEM information.

Let me know! /Miguel

[stmcginnis]

Awesome, thanks @mikeletux. I'll try to take a look soon, but initial thought is I kind of like the idea.

cc: @sosodev

[sosodev]

I like the concept :+1: At first I was concerned that it won't help users handle OEM data on their end but I think this could encourage people to contribute OEM plugins to gofish directly if we document it well. Which is good because gofish will likely reach a point where it just does everything users need out of the box.

[stmcginnis]

I really like the plugin idea. I's like to invert the model though. I'm thinking this would make it easier for vendors to provide OEM library extensions without necessarily needing to get them added to gofish first, especially if they are in the process of developing their implementations. It could also make it a little more scalable by not needing to build in awareness for extensions into the base gofish code.

So what I'm thinking is take these ideas, but just provide wrapper OEM objects around it. We can have the popular ones in tree, but they wouldn't necessarily have to be if the vendor didn't want them to be.

To keep things cleanly separated, and to make it possible for vendors to handle Redfish and Swordfish implementations if they want to, let's create a top level oem folder. So root of the project would be something like:

.
├── common
├── examples
├── oem
├── redfish
├── swordfish
└── tools

Then oem would contain vendor implementations like:

oem
├── dell
├── hpe
├── lenovo
├── openbmc
└── supermicro

From the core gofish code we can just handle things like normal. Then it would be up to the consuming code to handle getting the OEM specific objects. Something like:

import github.com/stmcginnis/gofish/oem/openbmc

chassis, err := service.Chassis()
openbmcChassis := openbmc.Chassis(chassis)
if openbmcChassis.SomeOEMProperty {
    openbmcChassis.SomeOEMAction()
}

And openbmcChassis would be something like:

type OpenBMCChassis struct {
    redfish.Chassis
    // Set of custom OEM properties
}

func (c *OpenBMCChassis) OEMAction() {
    // Custom OEM action handling
}

The plugin code would probably need access to the rawData as suggested earlier so it can perform custom unmarshalling of the JSON. Or to keep it clean, maybe just helper methods to return just the Actions and object.OEM property information. Though I'm starting to think it would be good to provide access to that full raw JSON blob even though I originally wanted to keep that hidden.

So then a nice benefit would be that any vendor could provide their own plugin code for this, something like:

// Totally separate implementation that may or may not eventually be added to gofish itself
import github.com/lenovo/gofish-plugin/lenovo

chassis, err := service.Chassis()
lenovoChassis := lenovo.Chassis(chassis)

So just some thoughts as I start to look into this more. Feel free to poke holes in these ideas. :) Let me know what you think one way or the other.

[mikeletux]

Hi @stmcginnis ,

Really like your idea. Actually, this one leaves the current codebase as it is (well, actually we need to change/add some things like the Oem interface{} by json.RawMessage to allow plugins to parse this data).

It's late my time, but if you don't mind, I can update this PR to follow this approach 😄

Also, I think it makes sense for every vendor to keep its gofish-plugin in a separate repository, since that will give devs agility to implement every OEM part, without bothering you everytime code needs to be pushed.

Let's discuss it further tomorrow! /Miguel

[mikeletux]

@stmcginnis ,

I've updated the updateService accordingly to support the plugin system. I removed everything related to Dell from the previous pull request. Now I've just included a field called OemActions json.RawMessage that will hold vendor specific actions, that can eventually be parsed by the vendor.

Also change the interface{} type from Oem and change it for json.RawMessage, so it can be parsed by the vendor as well.

Down here I leave a code snipped that I've written to parse the custom actions from Dell. The same approach can be followed by all vendors: https://github.com/mikeletux/terraform-provider-redfish/blob/dell-gofish/gofish/dell/updateService.go

The GetUpdateService function gets a redfish.UpdateService pointer (original struct from gofish), and it returns a custom UpdateService with all data plus the Dell actions.

If you like it, we can start doing this from now on. From my point of view looks very convenient, because every vendor is responsible of its part of the API.

Best regards! /Miguel

[stmcginnis]

Looks good @mikeletux - thanks!

[mikeletux]

@stmcginnis do you plan to merge this? I was planning to do something similar to Manager.

Let me know!

[stmcginnis]

Yeah, this looks good @mikeletux. Let's merge this one now that jobs are happy and add support for other services in other PRs. Thanks for your work on this!