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 SetOneTimeBoot to use standard ComputerSystem 'Boot' property #54201

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@billdodd
Copy link
Contributor

billdodd commented Mar 21, 2019

SUMMARY

Updated the logic for the SetOneTimeBoot command to use the standard 'Boot' property in the ComputerSystem resource. The original code was using Bios attributes which is not conformant with the Redfish spec.

I also updated the logic to not PATCH the system if the Boot properties are already set to the needed values. In this case, the Ansible 'changed' property is returned as False.

If the PATCH is applied, the 'changed' property is returned as True.

Fixes #51814

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

redfish_utils.py
redfish_command.py

ADDITIONAL INFORMATION

Playbooks from the original problem description in issue #51814:

---
- hosts: localhost
  gather_facts: false
  vars:
    baseuri: 10.240.50.78
    user: USERID
    password: PASSW0RD
  tasks:
    - name: Set one time boot option
      redfish_command:
        category: Systems
        command: SetOneTimeBoot
        bootdevice: "NIC 1 PXE Boot - IPv4"
        baseuri: "{{ baseuri }}"
        username: "{{ username }}"
        password: "{{ password }}"

Error result from the original problem description in issue #51814:

$ bin/ansible-playbook ../test_redfish_command.yml  -vvvv
ansible-playbook 2.8.0.dev0
  config file = None
  configured module search path = [u'/home/xander/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/ansible/lib/ansible
  executable location = bin/ansible-playbook
  python version = 2.7.12 (default, Nov 12 2018, 14:36:49) [GCC 5.4.0 20160609]
No config file found; using defaults
setting up inventory plugins
host_list declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
script declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
auto declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
yaml declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
ini declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
toml declined parsing /etc/ansible/hosts as it did not pass it's verify_file() method
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does
not match 'all'

Loading callback plugin default of type stdout, v2.0 from /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/ansible/lib/ansible/plugins/callback/default.pyc

PLAYBOOK: test_redfish_command.yml ***************************************************************************Positional arguments: ../test_redfish_command.yml
become_user: root
become_method: sudo
inventory: (u'/etc/ansible/hosts',)
tags: (u'all',)
forks: 5
verbosity: 4
connection: smart
timeout: 10
1 plays in ../test_redfish_command.yml

PLAY [localhost] *********************************************************************************************META: ran handlers

TASK [Set one time boot option] ******************************************************************************task path: /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/test_redfish_command.yml:9
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: xander
<127.0.0.1> EXEC /bin/sh -c 'echo ~xander && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588 `" && echo ansible-tmp-1549463086.08-258158793933588="` echo /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588 `" ) && sleep 0'
Using module file /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/ansible/lib/ansible/modules/remote_management/redfish/redfish_command.py
<127.0.0.1> PUT /home/xander/.ansible/tmp/ansible-local-1602jVNR9M/tmpEuc2cS TO /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/ /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py", line 113, in <module>
    _ansiballz_main()
  File "/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py", line 105, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py", line 48, in invoke_module
    imp.load_module('__main__', mod, module, MOD_DESC)
  File "/tmp/ansible_redfish_command_payload_yaYKzO/__main__.py", line 256, in <module>
  File "/tmp/ansible_redfish_command_payload_yaYKzO/__main__.py", line 231, in main
  File "/tmp/ansible_redfish_command_payload_yaYKzO/ansible_redfish_command_payload.zip/ansible/module_utils/redfish_utils.py", line 679, in set_one_time_boot_device
KeyError: 'BootMode'

fatal: [localhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py\", line 113, in <module>\n    _ansiballz_main()\n  File
\"/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py\", line 105, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/xander/.ansible/tmp/ansible-tmp-1549463086.08-258158793933588/AnsiballZ_redfish_command.py\", line 48, in invoke_module\n
  imp.load_module('__main__', mod, module, MOD_DESC)\n  File \"/tmp/ansible_redfish_command_payload_yaYKzO/__main__.py\", line 256, in <module>\n  File \"/tmp/ansible_redfish_command_payload_yaYKzO/__main__.py\", line 231, in main\n  File \"/tmp/ansible_redfish_command_payload_yaYKzO/ansible_redfish_command_payload.zip/ansible/module_utils/redfish_utils.py\", line 679, in set_one_time_boot_device\nKeyError: 'BootMode'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}
        to retry, use: --limit @/mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/test_redfish_command.retry

PLAY RECAP ***************************************************************************************************localhost                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0

After the fix, the KeyError: 'BootMode' error is no longer seen.

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 21, 2019

@xmadsen

This comment has been minimized.

Copy link
Contributor

xmadsen commented Mar 22, 2019

@billdodd this looks good to me! Works on the Lenovo systems I've tested against.

@billdodd

This comment has been minimized.

Copy link
Contributor Author

billdodd commented Mar 22, 2019

@xmadsen Great! Thanks for checking it out.

@samerhaj

This comment has been minimized.

Copy link

samerhaj commented Mar 27, 2019

@billdodd This feedback could be an enhancement (not necessarily needed for this particular change):

Per schema definition, if BootSourceOverrideTarget is set to "UefiTarget", then you must also set UefiTargetBootSourceOverride to the UEFI device path of that target.

There are also additional extensions added recently to the schema version 1.5.0 (2017.3):

BootNext is a new property. If BootSourceOverrideTarget is set to UefiBootNext, then BootNext needs to be set to the proper Boot source such as "Boot0001" or "Boot0002".

@billdodd

This comment has been minimized.

Copy link
Contributor Author

billdodd commented Mar 27, 2019

@samerhaj - Thanks for the feedback.

For these two cases, it seems like we will need to add a new input parameter (or possibly two parameters) for the user to specify the UEFI device path or the Boot source as appropriate.

Question: Is it expected (or likely) that implementations will specify allowable values for these properties (via UefiTargetBootSourceOverride@Redfish.AllowableValues and BootNext@Redfish.AllowableValues)?

I think I'd prefer to go ahead and get this PR merged as-is and then add support for these 2 additional scenarios in a follow-up PR.

@mraineri - Do you have any comments/inputs about these UefiTarget and UefiBootNext cases? (general comments or preference regarding adding then in this PR or a follow-up PR)

@ansibot ansibot added the stale_ci label Apr 4, 2019

@mraineri

This comment has been minimized.

Copy link

mraineri commented Apr 11, 2019

  • If BootSource is "UefiTarget", user also needs to provide the UEFI target for the "UefiTargetBootSourceOverride" property
  • If BootSource is "UefiBootNext", user also needs to provide the "BootNext" property

@billdodd billdodd force-pushed the billdodd:fix/51814 branch from ad6299b to c6331e1 Apr 16, 2019

@billdodd

This comment has been minimized.

Copy link
Contributor Author

billdodd commented Apr 17, 2019

ready_for_review

@billdodd

This comment has been minimized.

Copy link
Contributor Author

billdodd commented Apr 17, 2019

@mraineri and @jose-delarosa I added the "UefiTarget" and "UefiBootNext" support discussed on the call last week. Ready for review now.

@jose-delarosa

This comment has been minimized.

Copy link
Contributor

jose-delarosa commented Apr 17, 2019

Looks good.

shipit

@ansibot ansibot added shipit and removed community_review labels Apr 17, 2019

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.