Summary
Since 0e66cb8 ("Show template variables and prettify JSON strings in dry-run output"), _prepare_node_attributes() in osism/tasks/conductor/ironic.py returns a 2-tuple (node_attributes, template_vars). Two callers in osism/tasks/conductor/utils.py were not updated and still treat the return value as a dict:
_get_conductor_redfish_credentials (utils.py:223)
_get_conductor_redfish_address (utils.py:255)
Both do:
node_attributes = _prepare_node_attributes(device, get_ironic_parameters)
if (
"driver_info" in node_attributes
and node_attributes.get("driver") == "redfish"
):
Because node_attributes is now a tuple, "driver_info" in node_attributes is a tuple-membership test that is always False. The if short-circuits before .get() is reached, so no exception is raised and nothing is logged — both functions silently return None, None / None regardless of the device's configuration.
Impact
These helpers are the conductor-configuration fallback for Redfish and the only source of credentials for osism redfish list <hostname> <EthernetInterfaces|NetworkAdapters|NetworkDeviceFunctions>:
- All internal call sites use
get_redfish_connection(hostname, ignore_ssl_errors=True) without username/password, so _get_conductor_redfish_credentials is invoked on every call. With the bug, credentials always come back None, None, the BMC session is built with SessionOrBasicAuth(username=None, password=None), and against any BMC requiring authentication the call fails. The exception is swallowed in redfish.py, so the command returns an empty list / "Could not establish Redfish connection" with no hint that credentials were dropped. The conductor-credential fallback is effectively dead.
_get_conductor_redfish_address likewise returns None, so base_url silently stays at the default https://{hostname} instead of the conductor-configured address (degrades rather than hard-fails).
Fix
Unpack the tuple at both call sites:
node_attributes, _ = _prepare_node_attributes(device, get_ironic_parameters)
and add regression tests for _get_conductor_redfish_credentials and _get_conductor_redfish_address (tests/unit/tasks/conductor/test_utils.py currently has no coverage of either function).
References
Found during review of #2342 (#2342 (review) — see the CHANGES_REQUESTED review body by @ideaship).
Summary
Since 0e66cb8 ("Show template variables and prettify JSON strings in dry-run output"),
_prepare_node_attributes()inosism/tasks/conductor/ironic.pyreturns a 2-tuple(node_attributes, template_vars). Two callers inosism/tasks/conductor/utils.pywere not updated and still treat the return value as a dict:_get_conductor_redfish_credentials(utils.py:223)_get_conductor_redfish_address(utils.py:255)Both do:
Because
node_attributesis now a tuple,"driver_info" in node_attributesis a tuple-membership test that is alwaysFalse. Theifshort-circuits before.get()is reached, so no exception is raised and nothing is logged — both functions silently returnNone, None/Noneregardless of the device's configuration.Impact
These helpers are the conductor-configuration fallback for Redfish and the only source of credentials for
osism redfish list <hostname> <EthernetInterfaces|NetworkAdapters|NetworkDeviceFunctions>:get_redfish_connection(hostname, ignore_ssl_errors=True)without username/password, so_get_conductor_redfish_credentialsis invoked on every call. With the bug, credentials always come backNone, None, the BMC session is built withSessionOrBasicAuth(username=None, password=None), and against any BMC requiring authentication the call fails. The exception is swallowed inredfish.py, so the command returns an empty list / "Could not establish Redfish connection" with no hint that credentials were dropped. The conductor-credential fallback is effectively dead._get_conductor_redfish_addresslikewise returnsNone, sobase_urlsilently stays at the defaulthttps://{hostname}instead of the conductor-configured address (degrades rather than hard-fails).Fix
Unpack the tuple at both call sites:
and add regression tests for
_get_conductor_redfish_credentialsand_get_conductor_redfish_address(tests/unit/tasks/conductor/test_utils.pycurrently has no coverage of either function).References
Found during review of #2342 (#2342 (review) — see the CHANGES_REQUESTED review body by @ideaship).