-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Supported check #388
Supported check #388
Conversation
…rrors instead of defining another one
This is to ensure the code within this provider only executes on dells
This is to ensure the Openbmc provider executes code only on hardware identified to be Openbmc
providers/dell/idrac.go
Outdated
@@ -204,21 +205,37 @@ func (c *Conn) Compatible(ctx context.Context) bool { | |||
|
|||
// PowerStateGet gets the power state of a BMC machine | |||
func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { | |||
if err := c.deviceSupported(ctx); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provider already has this check in Open
. I'm not following the value add for putting this in all these individual methods. As far as I can tell deviceSupported
doesnt do any method specific checks either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, you're right the dell, openbmc checks were redundant.
I noticed dell showing up as a successfulProvider
on another device - which was odd, but I haven't been able to reproduce the case, and theres no way it should end up in there. Changes reverted.
What does this PR implement/change/remove?
Verify hardware support in each Redfish based provider.
This is to ensure each of these hardware vendor specific providers will not proceed to execute methods unless the hardware is identified as supported.
Checklist
The HW vendor this change applies to (if applicable)