Fix errors in hpe specific get methods #7952

drawks · 2024-02-06T20:38:18Z

SUMMARY

While using the redfish_info module against some HPE hardware I encountered a variety of uncaught exceptions. After tracing it back I found these two methods to be the culprit. As they have a variety of syntax and logical errors.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

redfish

ADDITIONAL INFORMATION

It is worth noting that these methods and their inclusion in the all commands to the Chassis category of the redfish_info module does result in attempting to get information from OEM/Vendor specific extensions in a module which seems to otherwise attempt to only cover the generic redfish interface.

Additionally, due to the way these methods have been implemented, they both return a single value even when multiple chassis uris have a matching api command to return min fan speed and thermal configuration. I've left that logic as is so that consumers of this module do not experience breakage from a change in the "shape" of the returned data.

Ultimately I /think/ that these methods and the codepath that calls them should be removed in a future release and we should consider adding a deprecation warning.

Resolves #7951

before the change

$ ansible -m community.general.redfish_info -a '{"category": ["Chassis"], "command": "all", "baseuri": "<redacted>", "username": "administrator", "password": "<redacted>"}' -i localhost, -c local localhost

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: AttributeError: 'RedfishUtils' object has no attribute 'chassis_uri_list'
localhost | FAILED! => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3"
    },
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/home/drawks/.ansible/tmp/ansible-tmp-1707250729.0465906-2384637-17649427234300/AnsiballZ_redfish_info.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/home/drawks/.ansible/tmp/ansible-tmp-1707250729.0465906-2384637-17649427234300/AnsiballZ_redfish_info.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/drawks/.ansible/tmp/ansible-tmp-1707250729.0465906-2384637-17649427234300/AnsiballZ_redfish_info.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.community.general.plugins.modules.redfish_info', init_globals=dict(_module_fqn='ansible_collections.community.general.plugins.modules.redfish_info', _modlib_path=modlib_path),\n  File \"/usr/lib64/python3.9/runpy.py\", line 225, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib64/python3.9/runpy.py\", line 97, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib64/python3.9/runpy.py\", line 87, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_community.general.redfish_info_payload_paa25mq4/ansible_community.general.redfish_info_payload.zip/ansible_collections/community/general/plugins/modules/redfish_info.py\", line 568, in <module>\n  File \"/tmp/ansible_community.general.redfish_info_payload_paa25mq4/ansible_community.general.redfish_info_payload.zip/ansible_collections/community/general/plugins/modules/redfish_info.py\", line 501, in main\n  File \"/tmp/ansible_community.general.redfish_info_payload_paa25mq4/ansible_community.general.redfish_info_payload.zip/ansible_collections/community/general/plugins/module_utils/redfish_utils.py\", line 3225, in get_hpe_thermal_config\nAttributeError: 'RedfishUtils' object has no attribute 'chassis_uri_list'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}

after:

localhost | SUCCESS => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3"
    },
    "changed": false,
    "redfish_facts": {
        "chassis": {
            "entries": [
< snip>
                    {
                        "Chassis": {
                            "Status": {
                                "Health": "OK"
                            }
                        }
                    }
                ]
            ],
            "ret": true
        },
        "hpe_fan_percent_min": {
            "ret": false
        },
        "hpe_thermal_config": {
            "ret": false
        },
        "psu": {
            "msg": "Key PowerSupplies not found",
            "ret": false
        },
        "thermals": {
            "entries": [
                {
                    "Name": "01-Inlet Ambient",
                    "PhysicalContext": "Intake",
                    "ReadingCelsius": 16,
< snip >
}

ansibullbot · 2024-02-06T20:55:44Z

cc @Bhavya06 @jyundt @mraineri @rajeevkallur @renxulei @tomasg2012 @xmadsen
click here for bot help

mraineri · 2024-02-06T22:02:30Z

Yuck, I didn't realize we had some HPE additions in there... We might need to think about removing those long-term since this really is meant to just cover vanilla Redfish.

At the very least, the changes you're introducing are all good; besides the trailing whitespaces, everything looks good to me. Thanks for finding this!

* corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue ansible-collections#7951

drawks · 2024-02-06T22:15:52Z

Yuck, I didn't realize we had some HPE additions in there... We might need to think about removing those long-term since this really is meant to just cover vanilla Redfish.

At the very least, the changes you're introducing are all good; besides the trailing whitespaces, everything looks good to me. Thanks for finding this!

I've corrected lines flagged by the linter and squashed my commits.

drawks · 2024-02-06T22:20:13Z

Also, I'm not sure what the release pattern is for bug fixes to this repo, but it'd be really swell if this was backported to 7.7.x so that EPEL might pick it up for rhel, alma and rocky 9.3 users

mraineri · 2024-02-07T14:08:12Z

I'm assuming since @felixfontein tagged it with "backport-7" it should make it back into that release. Hopefully they'll confirm that.

changelogs/fragments/7951-fix-redfish_info-exception.yml

felixfontein · 2024-02-11T12:05:13Z

I'm assuming since @felixfontein tagged it with "backport-7" it should make it back into that release. Hopefully they'll confirm that.

Yes, it will, assuming there is no conflict during backporting. If that happens, you'd (you = whoever wants to have it in stable-7 ;) ) have to manually backport it to stable-7 in case you want it to appear in a new 7.x.y release.

patchback · 2024-02-24T19:32:24Z

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/dd7c3ad10d706267ffcd1bea1337a148dfce60af/pr-7952

Backported as #8021

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* Fix errors in hpe specific get methods * corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue #7951 * Update changelog. --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit dd7c3ad)

patchback · 2024-02-24T19:32:32Z

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/dd7c3ad10d706267ffcd1bea1337a148dfce60af/pr-7952

Backported as #8022

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* Fix errors in hpe specific get methods * corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue #7951 * Update changelog. --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit dd7c3ad)

felixfontein · 2024-02-24T19:33:33Z

@drawks thanks for your contribution!
@mraineri thanks for reviewing!

The backport to stable-7 worked, so it should appear in the 7.5.5 release.

… methods (#8021) Fix errors in hpe specific get methods (#7952) * Fix errors in hpe specific get methods * corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue #7951 * Update changelog. --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit dd7c3ad) Co-authored-by: Dave Rawks <[email protected]>

… methods (#8022) Fix errors in hpe specific get methods (#7952) * Fix errors in hpe specific get methods * corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue #7951 * Update changelog. --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit dd7c3ad) Co-authored-by: Dave Rawks <[email protected]>

* Fix errors in hpe specific get methods * corrects reference to non existent `self.chassis_uri_list` to `self.chassis_uris` * corrects syntactically incorrect dereferences * removes an uneccessary variable assignment to `chassis_uri_list` in `get_psu_inventory` method * adds changelog fragment for above indicating fix of issue ansible-collections#7951 * Update changelog. --------- Co-authored-by: Felix Fontein <[email protected]>

This comment was marked as outdated.

Sign in to view

drawks force-pushed the redfish_fix branch from 437f790 to 057ae7e Compare February 6, 2024 22:15

ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 6, 2024

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 backport-8 Automatically create a backport for the stable-8 branch labels Feb 7, 2024

felixfontein reviewed Feb 11, 2024

View reviewed changes

changelogs/fragments/7951-fix-redfish_info-exception.yml Outdated Show resolved Hide resolved

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Feb 19, 2024

Update changelog.

41d59e8

ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label Feb 23, 2024

felixfontein merged commit dd7c3ad into ansible-collections:main Feb 24, 2024
121 checks passed

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Feb 24, 2024

patchback bot mentioned this pull request Feb 24, 2024

[PR #7952/dd7c3ad1 backport][stable-7] Fix errors in hpe specific get methods #8021

Merged

patchback bot mentioned this pull request Feb 24, 2024

[PR #7952/dd7c3ad1 backport][stable-8] Fix errors in hpe specific get methods #8022

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix errors in hpe specific get methods #7952

Fix errors in hpe specific get methods #7952

drawks commented Feb 6, 2024

This comment was marked as outdated.

ansibullbot commented Feb 6, 2024

mraineri commented Feb 6, 2024

drawks commented Feb 6, 2024

drawks commented Feb 6, 2024

mraineri commented Feb 7, 2024

felixfontein commented Feb 11, 2024 •

edited

Loading

patchback bot commented Feb 24, 2024 •

edited

Loading

patchback bot commented Feb 24, 2024 •

edited

Loading

felixfontein commented Feb 24, 2024

Fix errors in hpe specific get methods #7952

Fix errors in hpe specific get methods #7952

Conversation

drawks commented Feb 6, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

This comment was marked as outdated.

ansibullbot commented Feb 6, 2024

mraineri commented Feb 6, 2024

drawks commented Feb 6, 2024

drawks commented Feb 6, 2024

mraineri commented Feb 7, 2024

felixfontein commented Feb 11, 2024 • edited Loading

patchback bot commented Feb 24, 2024 • edited Loading

Backport to stable-7: 💚 backport PR created

patchback bot commented Feb 24, 2024 • edited Loading

Backport to stable-8: 💚 backport PR created

felixfontein commented Feb 24, 2024

felixfontein commented Feb 11, 2024 •

edited

Loading

patchback bot commented Feb 24, 2024 •

edited

Loading

patchback bot commented Feb 24, 2024 •

edited

Loading