stmcginnis / gofish

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

feat(oem/supermicro): Add Supermicro Manager OEM functions #213

Closed feuerrot closed 2 weeks ago

feuerrot commented 2 years ago

This PR includes functions for:

I'd appreciate any hints regarding the supermicro.Manager-structure and how custom manager extensions should be integrated - while it does work accessing the syslog configuration using smm.supermicroManager.Oem.Supermicro.Syslogs() seems to be rather awkward.

stmcginnis commented 2 years ago

Awesome!

while it does work accessing the syslog configuration using smm.supermicroManager.Oem.Supermicro.Syslogs() seems to be rather awkward.

The way it was invisioned for these OEM extensions it would be a matter of creating the OEM type from the standard type. So something roughly along these lines:

manager := c.GetManager()

smManager := supermicro.FromManager(manager)
for _, logs := range smManager.SysLogs() {
    // process
}

So rather than trying to mirror the whole structure, you just hydrate a new OEM object using the existing Redfish object. A decent example is probably here: https://github.com/stmcginnis/gofish/blob/main/oem/dell/eventservice.go

feuerrot commented 1 year ago

@stmcginnis If I understand the Redfish Specification correctly, Supermicros Manager OEM extensions are separate resouces and not part of the manager itself, while Dells Eventservice contains the extensions directly. Does this make a difference in how the Supermicro extensions should be accessed from the manager?

stmcginnis commented 1 year ago

@stmcginnis If I understand the Redfish Specification correctly, Supermicros Manager OEM extensions are separate resouces and not part of the manager itself, while Dells Eventservice contains the extensions directly. Does this make a difference in how the Supermicro extensions should be accessed from the manager?

Thanks for the details. That may change a little of how the resulting structs are defined, but that wouldn't change how they are accessed. All OEM data should be accessed by using the standard Redfish objects, then using an OEM implementation to access the oem parts to handle vendor specific (i.e. non-standard) paths.

feuerrot commented 1 year ago

I modified the manager accordingly.

feuerrot commented 4 months ago

Hi, sorry for my late reply: I don't have access to the Supermicro hardware or the fork anymore, as I switched jobs last year. Maybe @mxey could add the missing changes to this PR?

mxey commented 4 months ago

Maybe @mxey could add the missing changes to this PR?

Yes, I'll look into this.

mxey commented 3 months ago

@stmcginnis I responded to all the open review comments. I decided to add another commit to the pull request, since previous review changes also appeared to be extra commits. I would suggest squashing it all together at the end.

stmcginnis commented 2 weeks ago

Sorry, we've had a push to add more Supermicro support, so unfortunately this is now superceded by other work. There's some great stuff here, so feel free to add to or improve the current code in the repo. Thanks a ton for your effort on this!

mxey commented 2 weeks ago

@stmcginnis No worries, looks like you added everything we needed :)