From 5050580bb83b01a840811e2f7b6a850671ef96ed Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Thu, 24 Sep 2026 18:24:55 +0200 Subject: [PATCH 1/2] Stop redfish sensor health from raising on a missing Health SensorReading computed health and states with .get() and then overwrote both with a strict lookup, so a sensor whose Status lacks Health, or carries a value outside the health map, raised KeyError and aborted the whole sensor listing. Keep the tolerant lookup, and drop the states of a reading that is OK, as the copy in command.py already does. --- confluent_server/aiohmi/redfish/oem/generic.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/confluent_server/aiohmi/redfish/oem/generic.py b/confluent_server/aiohmi/redfish/oem/generic.py index 0aa296b9..17799ca6 100644 --- a/confluent_server/aiohmi/redfish/oem/generic.py +++ b/confluent_server/aiohmi/redfish/oem/generic.py @@ -111,6 +111,7 @@ def natural_sort(iterable): class SensorReading(object): def __init__(self, healthinfo, sensor=None, value=None, units=None, unavailable=False): + self.states = [] if sensor: self.name = sensor['name'] else: @@ -119,8 +120,8 @@ class SensorReading(object): 'Status', {}).get('Health', None), const.Health.Warning) self.states = [healthinfo.get('Status', {}).get('Health', 'Unknown')] - self.health = _healthmap[healthinfo['Status']['Health']] - self.states = [healthinfo['Status']['Health']] + if self.health == const.Health.Ok: + self.states = [] self.value = value self.state_ids = None self.imprecision = None From 40b1bf5e3b492ebbea5057db17b5d99f0fd3f6cb Mon Sep 17 00:00:00 2001 From: Markus Hilger Date: Thu, 24 Sep 2026 18:24:55 +0200 Subject: [PATCH 2/2] Report a missing redfish manager instead of raising TypeError ManagedBy is optional, and when a system does not link a manager get_bmcurl() answers None. Most callers passed that straight into a web request, which failed with "Constructor parameter should be str" from the url library. Route those callers through bmcinfo(), which now raises UnsupportedFunctionality so confluent reports it plainly. The event log falls through to its system and chassis fallback, and list_media lists nothing, since neither needs a manager. --- confluent_server/aiohmi/redfish/command.py | 31 ++++++++++--------- .../aiohmi/redfish/oem/generic.py | 24 ++++++++++---- confluent_server/aiohmi/redfish/oem/lookup.py | 3 +- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/confluent_server/aiohmi/redfish/command.py b/confluent_server/aiohmi/redfish/command.py index 35a58389..eace4d92 100644 --- a/confluent_server/aiohmi/redfish/command.py +++ b/confluent_server/aiohmi/redfish/command.py @@ -482,7 +482,9 @@ class Command(object): async def bmcinfo(self): bmcurl = await self.get_bmcurl() if not bmcurl: - raise exc.PyghmiException('Unable to identify BMC') + # ManagedBy is optional, and without it there is no manager to ask + raise exc.UnsupportedFunctionality( + 'Unable to identify the manager of this system') return await self._do_web_request(bmcurl) async def get_power(self): @@ -699,8 +701,8 @@ class Command(object): async def get_bmcurl(self): if not self._varbmcurl: sysinfo = await self.sysinfo() - self._varbmcurl = sysinfo.get('Links', {}).get( - 'ManagedBy', [{}])[0].get('@odata.id', None) + self._varbmcurl = (sysinfo.get('Links', {}).get( + 'ManagedBy') or [{}])[0].get('@odata.id', None) return self._varbmcurl async def get_bmcnicurl(self): @@ -709,8 +711,7 @@ class Command(object): return self._varbmcnicurl async def list_network_interface_names(self): - bmcurl = await self.get_bmcurl() - bmcinfo = await self._do_web_request(bmcurl) + bmcinfo = await self.bmcinfo() nicurl = bmcinfo.get('EthernetInterfaces', {}).get('@odata.id', None) if not nicurl: return @@ -722,7 +723,7 @@ class Command(object): yield curl.rsplit('/', 1)[1] async def _get_bmc_nic_url(self, name=None): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() nicurl = bmcinfo.get('EthernetInterfaces', {}).get('@odata.id', None) if not nicurl: # Also optional. The None went straight into a request and @@ -783,7 +784,7 @@ class Command(object): async def _bmcresetinfo(self): if not self._varresetbmcurl: - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() resetinf = bmcinfo.get('Actions', {}).get('#Manager.Reset', {}) url = resetinf.get('target', '') valid = resetinf.get('ResetType@Redfish.AllowableValues', []) @@ -948,7 +949,7 @@ class Command(object): return await oem.set_system_configuration(changeset, self) async def get_ntp_enabled(self): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() netprotocols = bmcinfo.get('NetworkProtocol', {}).get('@odata.id', None) if netprotocols: netprotoinfo = await self._do_web_request(netprotocols) @@ -957,7 +958,7 @@ class Command(object): return False async def set_ntp_enabled(self, enable): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() netprotocols = bmcinfo.get('NetworkProtocol', {}).get('@odata.id', None) if netprotocols: request = {'NTP':{'ProtocolEnabled': enable}} @@ -966,7 +967,7 @@ class Command(object): await self._do_web_request(netprotocols, cache=0) async def get_ntp_servers(self): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() netprotocols = bmcinfo.get('NetworkProtocol', {}).get('@odata.id', None) if not netprotocols: return [] @@ -974,7 +975,7 @@ class Command(object): return netprotoinfo.get('NTP', {}).get('NTPServers', []) async def set_ntp_server(self, server, index=None): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() netprotocols = bmcinfo.get('NetworkProtocol', {}).get('@odata.id', None) currntpservers = await self.get_ntp_servers() if index is None: @@ -1003,7 +1004,7 @@ class Command(object): In many cases, this may render remote network access impracticle or impossible." """ - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() rc = bmcinfo.get('Actions', {}).get('#Manager.ResetToDefaults', {}) actinf = rc.get('ResetType@Redfish.AllowableValues', []) if 'ResetAll' in actinf: @@ -1168,7 +1169,7 @@ class Command(object): {'HostName': hostname}, 'PATCH', etag='*') async def _netprotocolurl(self): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() netprotocols = bmcinfo.get('NetworkProtocol', {}).get('@odata.id', None) if not netprotocols: raise exc.UnsupportedFunctionality( @@ -1214,7 +1215,7 @@ class Command(object): method='PATCH', etag='*') async def get_remote_kvm_available(self): - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() gconsole = bmcinfo.get('GraphicalConsole', {}) return bool(gconsole.get('ServiceEnabled', False)) @@ -1700,7 +1701,7 @@ class Command(object): sysinfo = await self.sysinfo() vmcoll = sysinfo.get('VirtualMedia', {}).get('@odata.id', None) if not vmcoll: - bmcinfo = await self._do_web_request(await self.get_bmcurl()) + bmcinfo = await self.bmcinfo() vmcoll = bmcinfo.get('VirtualMedia', {}).get('@odata.id', None) if vmcoll: vmlist = await self._do_web_request(vmcoll) diff --git a/confluent_server/aiohmi/redfish/oem/generic.py b/confluent_server/aiohmi/redfish/oem/generic.py index 17799ca6..e30feb11 100644 --- a/confluent_server/aiohmi/redfish/oem/generic.py +++ b/confluent_server/aiohmi/redfish/oem/generic.py @@ -398,10 +398,17 @@ class OEMHandler(object): async def get_bmcurl(self): if not self._varbmcurl: - self._varbmcurl = (await self.sysinfo()).get('Links', {}).get( - 'ManagedBy', [{}])[0].get('@odata.id', None) + self._varbmcurl = ((await self.sysinfo()).get('Links', {}).get( + 'ManagedBy') or [{}])[0].get('@odata.id', None) return self._varbmcurl + async def _bmcinfo(self): + bmcurl = await self.get_bmcurl() + if not bmcurl: + raise exc.UnsupportedFunctionality( + 'Unable to identify the manager of this system') + return await self._do_web_request(bmcurl) + async def sysinfo(self): sysurl = await self.get_default_sysurl() @@ -554,7 +561,7 @@ class OEMHandler(object): await self._do_web_request(replacecerturl, certpayload) async def add_trusted_ca(self, pemdata): - mgrinfo = await self._do_web_request(await self.get_bmcurl()) + mgrinfo = await self._bmcinfo() secpolicy = mgrinfo.get('SecurityPolicy', {}).get('@odata.id', None) if secpolicy: secinfo = await self._do_web_request(secpolicy) @@ -568,7 +575,7 @@ class OEMHandler(object): raise exc.PyghmiException('Platform does not support adding trusted CAs') async def del_trusted_ca(self, certid): - mgrinfo = await self._do_web_request(await self.get_bmcurl()) + mgrinfo = await self._bmcinfo() secpolicy = mgrinfo.get('SecurityPolicy', {}).get('@odata.id', None) if secpolicy: secinfo = await self._do_web_request(secpolicy) @@ -585,7 +592,7 @@ class OEMHandler(object): raise exc.PyghmiException(f'No such certificate found: {certid}') async def get_trusted_cas(self): - mgrinfo = await self._do_web_request(await self.get_bmcurl()) + mgrinfo = await self._bmcinfo() secpolicy = mgrinfo.get('SecurityPolicy', {}).get('@odata.id', None) if secpolicy: secinfo = await self._do_web_request(secpolicy) @@ -654,7 +661,10 @@ class OEMHandler(object): return collections async def get_event_log(self, clear=False, fishclient=None, extraurls=[]): - bmcinfo = await self._do_web_request(await fishclient.get_bmcurl()) + bmcurl = await fishclient.get_bmcurl() + # ManagedBy is optional, and a system without it may still keep logs + # under itself or its chassis, which the fallback below looks for + bmcinfo = await self._do_web_request(bmcurl) if bmcurl else {} # A manager need not publish log services at all, and one that does not # is the clearest case of a platform keeping its event log elsewhere, so # carry on to the fallback below rather than answering with nothing @@ -1196,6 +1206,8 @@ class OEMHandler(object): async def list_media(self, fishclient, cache=True): bmcurl = await fishclient.get_bmcurl() + if not bmcurl: + return bmcinfo = await fishclient._do_web_request(bmcurl, cache=cache) vmcoll = bmcinfo.get('VirtualMedia', {}).get('@odata.id', None) if vmcoll: diff --git a/confluent_server/aiohmi/redfish/oem/lookup.py b/confluent_server/aiohmi/redfish/oem/lookup.py index d08e3c20..cc650896 100644 --- a/confluent_server/aiohmi/redfish/oem/lookup.py +++ b/confluent_server/aiohmi/redfish/oem/lookup.py @@ -61,7 +61,8 @@ async def get_oem_handler(sysinfo, sysurl, webclient, cache, cmd, rootinfo={}): bmcinfo = mgrinfo break else: - bmcinfo = await cmd.bmcinfo() + if await cmd.get_bmcurl(): + bmcinfo = await cmd.bmcinfo() for oem in bmcinfo.get('Oem', {}): if oem in OEMMAP: return await OEMMAP[oem].get_handler(sysinfo, sysurl, webclient, cache,