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

Add GetChassisThermals command to Chassis category of redfish_facts #54399

Open
wants to merge 6 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@xmadsen
Copy link
Contributor

xmadsen commented Mar 26, 2019

SUMMARY

This pull request adds a GetChassisThermals command to the Chassis category of redfish_facts. It retrieves from the Thermal/Temperatures field various temperature-related data for each sensor populated therein.

Fixes #54231

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

redfish_facts

ADDITIONAL INFORMATION
- name: Test Redfish modules
  hosts: localhost
  gather_facts: false
  vars:
    baseuri: 10.243.5.31
    user: USERID
    password: PASSW0RD

  tasks:
    - name: Get Chassis Thermals
      redfish_facts:
        category: Chassis
        command: GetChassisThermals
        baseuri: "{{ baseuri }}"
        username: "{{ user }}"
        password: "{{ password }}"
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 = /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/ansible/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
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'


PLAYBOOK: test_redfish_facts.1.yml **********************************************************************************1 plays in ../test_redfish_facts.1.yml

PLAY [Test Redfish modules] *****************************************************************************************META: ran handlers

TASK [Get Chassis Thermals] *****************************************************************************************task path: /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/test_redfish_facts.1.yml:10
<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-1553609768.02-132527517191880 `" && echo ansible-tmp-1553609768.02-132527517191880="` echo /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880 `" ) && sleep 0'
Using module file /mnt/c/Users/amadsen/Coding/lenovo-redfish-automated-testing/ansible/lib/ansible/modules/remote_management/redfish/redfish_facts.py
<127.0.0.1> PUT /home/xander/.ansible/tmp/ansible-local-8740sZYjwN/tmpESZy_z TO /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880/AnsiballZ_redfish_facts.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880/ /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880/AnsiballZ_redfish_facts.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880/AnsiballZ_redfish_facts.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/xander/.ansible/tmp/ansible-tmp-1553609768.02-132527517191880/ > /dev/null 2>&1 && sleep 0'
ok: [localhost] => {
    "ansible_facts": {
        "redfish_facts": {
            "thermals": {
                "entries": [
                    {
                        "Name": "Ambient Temp", 
                        "PhysicalContext": "Intake", 
                        "ReadingCelsius": 24, 
                        "UpperThresholdCritical": 47, 
                        "UpperThresholdFatal": 50
                    }, 
                    {
                        "Name": "Exhaust Temp", 
                        "PhysicalContext": "Exhaust", 
                        "ReadingCelsius": 27
                    }, 
                    {
                        "Name": "CPU1 Temp", 
                        "PhysicalContext": "CPU", 
                        "ReadingCelsius": 26
                    }, 
                    {
                        "Name": "CPU2 Temp", 
                        "PhysicalContext": "CPU", 
                        "ReadingCelsius": 24
                    }, 
                    {
                        "Name": "CPU1 DTS", 
                        "PhysicalContext": "CPU", 
                        "ReadingCelsius": -510, 
                        "UpperThresholdCritical": -2, 
                        "UpperThresholdFatal": 0
                    }, 
                    {
                        "Name": "CPU2 DTS", 
                        "PhysicalContext": "CPU", 
                        "ReadingCelsius": -510, 
                        "UpperThresholdCritical": -2, 
                        "UpperThresholdFatal": 0
                    }, 
                    {
                        "Name": "DIMM 5 Temp", 
                        "PhysicalContext": "Memory", 
                        "ReadingCelsius": 27
                    }, 
                    {
                        "Name": "DIMM 17 Temp", 
                        "PhysicalContext": "Memory", 
                        "ReadingCelsius": 27
                    }, 
                    {
                        "Name": "PCH Temp", 
                        "PhysicalContext": "ComputeBay", 
                        "ReadingCelsius": 49
                    }
                ], 
                "ret": true
            }
        }
    }, 
    "changed": false, 
    "invocation": {
        "module_args": {
            "baseuri": "10.243.5.31", 
            "category": [
                "Chassis"
            ], 
            "command": [
                "GetChassisThermals"
            ], 
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
            "username": "USERID"
        }
    }
}
META: ran handlers
META: ran handlers

PLAY RECAP **********************************************************************************************************localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
@xmadsen

This comment has been minimized.

Copy link
Contributor Author

xmadsen commented Mar 26, 2019

@ansibot

This comment has been minimized.

@samerhaj

This comment has been minimized.

Copy link

samerhaj commented Mar 27, 2019

Some feedback:

  • The following properties should be included if they are present and not null, since they have useful information:
    LowerThresholdFatal
    LowerThresholdNonCritical
    LowerThresholdCritical
    UpperThresholdFatal
    UpperThresholdNonCritical
    UpperThresholdCritical
    MinReadingRangeTemp
    MaxReadingRangeTemp
    SensorNumber

in addition to the ones you added: Name, PhysicalContext, and ReadingCelsius.

  • Since you are combining Temperatures from multiple chassis, and also since a single Chassis will have Temp sensors spanning multiple components, the RelatedItems[] is a key piece of data that need to be included in the output to map the reading to the correct component.
@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 27, 2019

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/module_utils/redfish_utils.py:837:73: trailing-whitespace Trailing whitespace
lib/ansible/module_utils/redfish_utils.py:838:70: trailing-whitespace Trailing whitespace
lib/ansible/module_utils/redfish_utils.py:839:73: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/module_utils/redfish_utils.py:837:74: W291 trailing whitespace
lib/ansible/module_utils/redfish_utils.py:838:71: W291 trailing whitespace
lib/ansible/module_utils/redfish_utils.py:839:74: W291 trailing whitespace

click here for bot help

@samerhaj

This comment has been minimized.

Copy link

samerhaj commented Mar 27, 2019

Looks good! Just fix the trailing whitespaces, and shipit

@samerhaj

This comment has been minimized.

Copy link

samerhaj commented Mar 27, 2019

shipit

@billdodd
Copy link
Contributor

billdodd left a comment

Reviewed and tested. Looks good to me.

shipit

@ansibot ansibot removed the needs_triage label Mar 28, 2019

@jose-delarosa

This comment has been minimized.

Copy link
Contributor

jose-delarosa commented Apr 1, 2019

Looks good @xmadsen, might need to rebase your branch. Thank you.

shipit

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot 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-getchassisthermals branch Apr 1, 2019

@ansibot ansibot removed the merge_commit label Apr 1, 2019

@xmadsen xmadsen force-pushed the xmadsen:feat/add-getchassisthermals branch to 8ce5e2d Apr 3, 2019

@billdodd
Copy link
Contributor

billdodd left a comment

Approving again after rebase

shipit

@jose-delarosa

This comment has been minimized.

Copy link
Contributor

jose-delarosa commented Apr 3, 2019

Looks good.

shipit

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

@gundalow

This comment has been minimized.

Copy link
Contributor

gundalow commented Apr 5, 2019

Another rebase needed :(

@gundalow gundalow self-assigned this Apr 5, 2019

@gundalow

This comment has been minimized.

Copy link
Contributor

gundalow commented Apr 5, 2019

If someone could poke me on IRC once this has been rebased I'll get it merged. I get too much GitHub email to keep on top of.

@billdodd

This comment has been minimized.

Copy link
Contributor

billdodd commented Apr 5, 2019

@gundalow - Thanks. I'll ping you on IRC once this has been rebased.

@xmadsen - For the 3(?) open chassis related PRs that need rebasing and/or updates, it might be best to do one of them, then wait for it to be merged. Then do the next one, etc. Otherwise, they'll probably keep conflicting with each other in redfish_facts.py.

@ansibot ansibot added the stale_ci label Apr 13, 2019

@xmadsen xmadsen force-pushed the xmadsen:feat/add-getchassisthermals branch from 8ce5e2d to f9982f1 Apr 16, 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.