Two places where the code that exists to explain a failure fails instead, and
the caller is shown the second failure rather than the first.
_do_web_request builds its message from the response body when that body is
not the JSON error document the spec asks for. An html 404 page is exactly
that, the body is bytes, and str + bytes raised TypeError. The status and the
body never reached anyone.
LenovoFirmwareConfig raised a bare Exception when python-lxml and
python-eficompressor are absent. Confluent has no handler for one, so a
missing dependency showed as "Unexpected Error" and hid a message that
already said what to install.
Neither changes what fails, only what the caller is told.
PCIeDevices and PCIeFunctions are optional, and get_health indexed both
without checking. _get_adp_urls in the same file already spells it
.get('PCIeDevices', []), so these two were the outliers.
Power and cooling equipment has no PCIe at all. The KeyError escaped the
health read and reached the user as "Unexpected Error" with a traceback
behind it. Reproduces offline against DMTF's public-rackmount1 mockup.
A read only ever looked at the manager's log services, and fell back to the
system's when the manager published none. A platform that keeps an event log
in both places had the second one invisible: an AMI MegaRAC keeps power unit
and thermal events in a chassis log that nothing read, 103 records that no
command could reach.
Clearing deliberately does not follow. It stays where it was, so a log that
only a read reaches is never destroyed by one, and clearing a platform that
keeps its only event log on the system still works.
The name test now ignores spacing, since a build that calls its post code log
"BIOS POST Code Log" was read as an event log and merged 2719 post codes in.
Reading an event log skipped every log service whose id or name said journal,
dump, post code, host logger or crash, on every implementation. Those names
are bmcweb's: the AMI and Lenovo bmcs call theirs SEL, EventLog, AuditLog and
PlatformLog. bmcweb does need the distinction, keeping its event log under the
system while a clear would destroy its dumps, so it gets a handler that names
the words and generic names none, reading whatever a platform publishes.
Generic added an UpdateParameters part to every multipart firmware push,
because the specification has one carried. Only the AMI firmware was seen to
insist on it, so name it in that handler and let generic send what the caller
passed.
The deadline was set when monitoring began, so a flash that kept
answering for longer than the grace period had already spent it by the
time the reboot it covers arrived, and reported a working update as a
failure.
Some will not take an image without being told which kind it is, and the
only way to find out was to attempt an update and read the error, which
writes to the bmc before it gets that far.
Processor inventory carried a single field, the model, so a platform that
does not give one had a processor in the listing with nothing in it, and
the client, which skips empty values, showed no processor at all. This
bmc names the manufacturer, the socket and the core and thread counts, and
gives no model.
Carry those, along with the speed, serial and part number where a platform
offers them, and treat a processor as missing only when the bmc says its
state is absent, rather than whenever it does not describe a state.
The redfish event log was taken from every log service the manager
advertises, whatever those turned out to be. On a bmc that keeps its
systemd journal there, nodeeventlog answered with a thousand lines of
kernel probe failures and daemon chatter, and the log the user asked for
was never read at all, because this implementation keeps it under the
system. Clearing was worse: of the services it did find, the ones with a
clear action were the dumps, so a clear destroyed diagnostic data, left
the event log untouched, and reported success.
Judge a log service before reading it. A service whose id or name says
journal, dump, post code, host logger or crash is not an event log, and
both reading and clearing skip it, so a clear can no longer take out
something that was never asked for.
If that leaves the manager with no event log at all, look under the
system, where such an implementation keeps it. Only then: a bmc that has
one under the manager is served exactly as before, from the same requests,
so this cannot change what an implementation that already worked reports.
The list of services was also being extended in place, and it belongs to
whatever the url cache is holding, so an extra log added by an oem handler
accumulated on every call within the cache window.
Deleting a user gave up after one attempt and reported why the fallback of
blanking the name failed rather than why the delete did. MegaRAC reports a
timeout for a delete that a second ask completes, so retry, check whether the
account went away, and keep the original error.
Attaching media judged a device free by ConnectedVia, which describes how the
device is wired to the host rather than whether anything is in it. Every
device on this bmc reports a fixed value there, so nothing was ever selected
and the attach reported success having done nothing. Judge by whether an image
is loaded, fall back to the properties when an advertised insert action is not
served, and say so when no device would take the image.
The firmware category was passed to the library and ignored, so core,
adapters, disks and misc all returned the same full list. Classify by what
each entry is related to, and answer only for core when a platform says
nothing about where its firmware belongs.
reseat_bay reached for a hardcoded Nvidia action on Chassis_0, so it failed
with a not found for that url instead of saying reseat is unsupported.
A bare UnsupportedFunctionality() left the user with an error containing no
text at all, or with no output and a success exit code, so asking a platform
for something it does not implement looked like nothing had happened.
Name what is unsupported at each raise, treat it as its own case in the
plugins so it reads as a limitation rather than an unexpected error and does
not log a traceback, and fall back to naming the exception when an exception
still arrives with nothing to say. The generic redfish
get_extended_bmc_configuration was also declared without async while the
caller awaits it.
Three things stopped a redfish firmware update on MegaRAC. The AMI handler
opened by asking the bmc to preserve fourteen named settings, and a build that
knows a different set rejects the whole request, which aborted the update
before anything was uploaded; send only the keys the bmc advertises. The
multipart push carried the image alone, and the specification has it carry an
UpdateParameters part too, which this firmware enforces. AMI also wants an
OemParameters part naming the kind of image, and nothing was supplying one.
The kind of image is asked for rather than worked out from the file. The
extension is vendor habit rather than format, and the leading bytes answer just
as confidently about an image they have never seen, while being wrong means a
bmc flashed with a bios image. So nodefirmware takes --type, it travels as far
as the handler that wants it, and where the bmc publishes the types it accepts,
an unknown one is refused with the list, as is asking with none. A platform
that reads the kind of firmware out of the image itself refuses the option
rather than dropping it, so nobody aims an update somewhere they did not mean
to. A parameter file still wins, since it can carry more than the image type.
Updating the bmc takes the bmc, and the task being watched, away for minutes.
That is the update working rather than the monitoring failing, so wait a
bounded while for it to answer again instead of reporting a successful flash
as an error.
MegaRAC refuses a PATCH with no If-Match header, so setting the bmc hostname,
ntp, the bmc network configuration, a location and ejecting media all failed
with a precondition error. set_identify already passed etag=*; do the same at
the call sites that did not, including the firmware push busy flag.
get_identify indexed the sysinfo method object rather than awaiting it, so
reading the identify state raised a KeyError naming a bound method on every
redfish system. Read the indicator from the chassis that owns the physical
led, since some implementations leave the copy on the system stale, and say
plainly when a platform does not report one.
The generic get_description took no fishclient while every other handler and
the caller pass one, so the description resource raised a TypeError on any
non Lenovo bmc.
Apply ruff's safe autofixes.
The changes are mechanical and behaviour-preserving. Issues fixed:
- F401: remove unused imports.
- F841: drop unused local variables and assignments, including discarded
await/return values, unused "except ... as e" bindings, and unused
"with ... as name" targets.
- F541: remove the f prefix from f-strings that contain no placeholders.
- E711: compare against None with "is"/"is not" instead of "=="/"!=".
- E712: test truthiness directly instead of comparing to True.
- E713: use "x not in y" instead of "not x in y".
- E714: use "is not" instead of "not ... is".
- E731: convert lambdas bound to a name into def statements.
- W291/W293: trim trailing whitespace on touched lines.
Jarrod Johnson
Jarrod Johnson
Markus Hilger