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

Feat/add GetBootOverride to Systems category of redfish_facts #54273

Open
wants to merge 9 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@xmadsen
Copy link
Contributor

commented Mar 22, 2019

SUMMARY

This PR adds a GetBootOverride command to the Systems category of redfish_facts. It returns the target, boot mode, and type of override (once vs. more frequent) for the system, and returns a message indicating there is no override if the state of the override property is not Enabled.

Fixes #54205

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

redfish_facts

ADDITIONAL INFORMATION

@xmadsen

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@xmadsen xmadsen changed the title Feat/add getbootoverride Feat/add GetBootOverride to Systems category of redfish_facts Mar 22, 2019

@ansibot

This comment has been minimized.


result['boot_override'] = {}
if "BootSourceOverrideEnabled" in boot:
if boot["BootSourceOverrideEnabled"] is not False:

This comment has been minimized.

Copy link
@samerhaj

samerhaj Mar 27, 2019

the value of this property is "Disabled", "Once" or "Continuous".. Maybe no need to check its value for the output

This comment has been minimized.

Copy link
@xmadsen

xmadsen Mar 27, 2019

Author Contributor

I will set it to check for being Disabled and return that there is no override present if so.

@samerhaj

This comment has been minimized.

Copy link

commented Mar 27, 2019

May also need to add BootSourceOverrideTarget@Redfish.AllowableValues if it is available since it gives the user the possible values for the BootsourceOverrideTarget

@billdodd
Copy link
Contributor

left a comment

Could you also a "multi" wrapper for this so that we get the boot override info for all systems in the service?

@ansibot ansibot removed the needs_triage label Mar 28, 2019

@billdodd

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

Code looks good. Just need to fix up whatever the merge conflict is.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

@xmadsen this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot ansibot added the merge_commit label Apr 1, 2019

@xmadsen xmadsen force-pushed the xmadsen:feat/add-getbootoverride branch Apr 1, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

@xmadsen this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot ansibot removed the needs_revision label Apr 1, 2019

@xmadsen xmadsen force-pushed the xmadsen:feat/add-getbootoverride branch to edd21ed Apr 4, 2019

@ansibot ansibot added the stale_ci label Apr 12, 2019

@xmadsen

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

@billdodd @jose-delarosa this should be ready for review

@billdodd

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@billdodd @jose-delarosa this should be ready for review

Thanks. I'll review today.

@billdodd

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@xmadsen

Could you also a "multi" wrapper for this so that we get the boot override info for all systems in the service?

Looks like it still needs a multi wrapper to handle multiple systems.

May also need to add BootSourceOverrideTarget@Redfish.AllowableValues if it is available since it gives the user the possible values for the BootsourceOverrideTarget

And I don't see this in the PR either. Did some changes get lost in a rebase?

@xmadsen

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

Hm, yeah it looks like some code disappeared in some rebasing, I'll work on this today.

@ansibot ansibot removed the stale_ci label Apr 18, 2019

@xmadsen

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

@billdodd @jose-delarosa should be fixed now!

@billdodd
Copy link
Contributor

left a comment

@xmadsen - Looks good. Can you just fix the one small introduced typo. And add an example in the EXAMPLES section.

Show resolved Hide resolved lib/ansible/module_utils/redfish_utils.py Outdated
Update lib/ansible/module_utils/redfish_utils.py
Co-Authored-By: xmadsen <xander.madsen@gmail.com>
@billdodd

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@xmadsen - Right after I made the comment about the typo I also noticed there is no example for the new command. Could you also add that? Sorry I didn't catch that the first time.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@xmadsen this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.