Skip to content
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

fix manager reset action when no allowed values for reset type are provided #279

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

vsvastey
Copy link
Contributor

@vsvastey vsvastey commented Oct 1, 2023

Sometimes Manager.Reset action doesn't provide AllowableValues for ResetTypes, like that:

  "Actions": {
    "#Manager.Reset": {
      "target": "/redfish/v1/Managers/1/Actions/Manager.Reset/"
    }
  }

In this case current implementation provides a payload {"Action": "Manager.Reset"} which doesn't work at least for HP gen9 (iLO4) that I can test on. I get Base.0.10.ActionNotSupported error.

According to the specification:

    Manager_v1_18_0_ResetRequestBody:
        ....
        ResetType:
          ...
          x-longDescription: This parameter shall contain the type of reset.  The
            service can accept a request without the parameter and perform an implementation
            specific default reset.  Services should include the @Redfish.AllowableValues
            annotation for this parameter to ensure compatibility with clients, even
            when ActionInfo has been implemented.
         ...

it seems like an empty payload should be sent in this case.

I've tested an empty payload on HP gen9 and it works as expected.

Please, consider the fix for this case.

@stmcginnis
Copy link
Owner

Looks reasonable to me. @leslie-qiwa originally added this special handling for HPE awhile back. Let's just wait a bit to see if they want to chime in here or confirm from their environment.

@leslie-qiwa
Copy link
Contributor

I can not find such a server in our system now. The change looks good for me. You can go ahead merging, and I'll let you know if the issue comes up.

@stmcginnis stmcginnis merged commit 5aa985a into stmcginnis:main Oct 3, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants