By copying, we leave the /etc/hosts with original ownership/permissions/etc.
Otherwise we create a new /etc/hosts, which is subject to new permissions and such.
Every error branch set exitcode and the script then ran off the end, so
a node that could not be read, or a log source that matched nothing,
still exited 0.
Every entry already says which log it came from, so -s narrows the output to
the ones asked for and "-s list" names what a node offers. Some of what a
platform keeps is noise: an AMI MegaRAC's event log is a list of redfish
sessions being opened and closed, while its useful records are elsewhere.
A selection cannot be cleared, since the platforms offer no such thing and
clearing more than was asked for is not something to do quietly.
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.
Asking for the mac addresses of a node whose inventory does not describe any,
which is every node reached over ipmi, printed absolutely nothing and exited
successfully, leaving no way to tell an empty answer from a broken command. Name
what was asked for instead. The exit code stays successful, since an inventory
that does not mention something is a valid answer rather than a failure.
A discrete sensor reports no value, and the unit was appended regardless, so a
watchdog came out as "Watchdog:% " and an event log sensor as "SEL:". The unit
belongs to a reading, so only print it when there is one. On the platform this
was seen on the units field is itself meaningless for such a sensor, carrying a
percent sign and a trailing space from the sdr.
A stray trailing comma made the update detail a one element tuple, so a
firmware error printed as a python tuple. A missing status printed the whole
response dict. A failure that named no node was dropped entirely, which is how
a service data request that the server refused came out as silence and a
success exit code, and nodestorage, nodelicense and nodesupport exited zero
even when they had reported an error.
nodeconsole crashed decoding an absent screenshot, and again on the terminal
calls behind a pipe, where a log replay crashed too; refuse the terminal only
modes cleanly and dump the log when there is no terminal to replay into.
nodedefine raised a ValueError on an argument without an equals sign, and
firmware for a category the target does not describe printed usage as though
the question had been malformed.
On the server side the readability check was applied to the path a download is
saved to, so asking for service data or saved licences at a path that does not
exist yet failed claiming the destination was not readable.
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.
nodeconsole spawned a task per input byte from the stdin reader callback
and kept no reference to it. Two of those tasks overlap as soon as one
parks in relay_keypresses waiting on the VNC connection, so keystrokes
can reach the node out of order and the escape sequence state (buffer,
inputcontext, modkeys) is mutated by more than one task at a time. With
the first keystroke relaying slowly, typing abcdef arrives as bcdefa.
Those tasks were also unreferenced, which asyncio documents as
collectable while still pending, so a keypress could be dropped.
Queue the bytes in the reader callback and process them from one
long-lived task instead. Keep a reference to the watch_input task as
well, since collecting that one takes the whole input handler with it.
Hand written rather than autofixed, since three of the four need the
surrounding code read to be sure they are equivalent:
- confetty: `powerstate == None` -> `is None`.
- nodeconfig: `setmode != True` / `!= False` -> `not setmode` / `setmode`.
Safe because setmode only ever holds None, True or False, and the two
lines above each test normalise None away first.
- pam: split two `if cond: stmt` one-liners.
- imgutil: `from shutil import copytree as copytree`, an alias that renames
nothing. Not a re-export marker, this is a script.
Entirely mechanical, produced by `ruff check --fix --select F401,F541,E713`
and reviewed rather than taken on faith: deleting an import is only safe if
nothing imports it for its side effects or re-exports it. None of the 19
removed names is referenced anywhere in its file, none appears in any string
literal, and none of the touched files uses eval, exec, globals() or
__import__, so there is no dynamic lookup that could reach them.
Each of these loops rebinds the name that holds the iterable. They work
today because the iterable is evaluated once before the loop starts, but
the name is then gone, so any later use reads a loop item instead of the
collection.
- nodeinventory: `for arg in args` / `for arg in arg.split(',')`.
- confignet (common and debian copies): iname holds the comma separated
interface list and is then reused for each interface in it.
- xcc _get_agentless_firmware: adata holds the adapter query response and
is then reused for each adapter.
No behaviour change, just distinct names for distinct things.
Every one of these raises NameError if its code path is reached:
- nodeapply: run_automation accumulated into an exitcode that only existed
in run(), so any automation error crashed instead of being reported. It
now keeps and returns its own, tracked separately from the exit code of
the ssh commands: the early exit after the spawn loop tests that one,
and folding automation failures into it would exit with children already
running and their pipes abandoned. Both are reported at the real exits.
- nodeconsole: redraw() reads firstnodename, which was local to
do_screenshot(); promote it to a module global like the other drawing
state.
- nodedeploy: the redeploy path appended to a lockednodes list that did not
exist yet. The block that follows re-reads the same lock state and acts
on it, so drop the dead duplicate.
- samples/nodeattrib_from_switch.py, misc/filterpasswd: missing import sys.
- xcc3: fixuuid was never imported. xcc imports xcc3, so take a local copy
the way the smm handler does instead of creating an import cycle.
- httpapi: the async session call still passed the WSGI-era env and an
extra argument to handle_async(), which has taken only querydict since
the aiohttp port. Calling it correctly exposed that handle_async()
registers an AsyncSession before raising on the discontinued long poll
path, so every request to it would leak a session that is never reaped.
It now only creates one when there is a websocket handler to yield it to.
- messages: the InputFirmwareUpdate.filename property checked
self.filebynode[node] with no node in scope. __init__ already validates
every expanded path and nodefile() rechecks per node, so drop the checks.
- pam: drop the python2 branches referencing unicode and raw_input. The
server has been python3 only since the asyncio port.
- cooltera: the sensor-name listing referenced a nonexistent sensors dict.
The available sensors depend on the model, which is only known after
reading the device, so list them from the same status data the readings
use.
- deltapdu, eatonpdu, geist: the not-implemented response in update() used
node outside the loop, unlike retrieve() in the same files and unlike
raritan/enlogic.
- confluentdbgcli: stray self. on a module-level socket connect.
gather propagates the first exception and leaves its siblings running,
so a transport level failure against one node ends the import with a
traceback while the rest of the batch is cancelled at loop shutdown.
The forked children used to contain such a failure to their own node.
assign_macs already reports an error response itself, so this is the
connection dropping rather than the server refusing the assignment.
Collect the exceptions instead, report each one and count it towards
the exit code. Schedule the assignments as tasks while doing so, since
the plain coroutines are left unawaited if defining a later node raises
before the gather is reached.
Replacing the forked children with a gather kept their fan-out: every
row of the import file gets a session of its own and they all start at
once, so a large file opens a local socket and a server side session
task per node simultaneously.
Hold a semaphore for the duration of each node's assignment instead, so
a finished node's session is dropped before the next one starts. Also
build that session once per node rather than once per MAC, and say why
the caller's session is not reused, which was self evident while this
ran in a forked child.
register_endpoint primes current but not total, so a first response
without a count field goes straight to
UnboundLocalError: local variable 'total' referenced before assignment
on the elif. Start at zero, which skips the progress line until the
server does report a count.
nodediscover's rescan poll and nodeconsole's screenshot refresh both slept
with time.sleep inside a coroutine. In nodeconsole --video that stops the
input handler and the VNC streaming tasks for the whole interval.
import_csv was left with several synchronous idioms:
- search_record is a coroutine function, but was called without await.
The returned coroutine is always truthy, so the rescan on incomplete
discovery data never happened, and iterating the result raised
TypeError: 'coroutine' object is not iterable
- the node creation loop iterated an async generator with plain for
- the per-node discovery assignment was forked off with os.fork() while
the event loop was running, and the child then built a fresh session
on the inherited selector
Assign discovery entries with asyncio.gather instead of a forked child,
which keeps the assignments concurrent and lets their exit codes
propagate. The forked child always ended in sys.exit(0), so its
accumulated errorcode was discarded.
register_endpoint and subscribe_discovery were left as plain functions
iterating the async client generators, so nodediscover register,
subscribe and unsubscribe all failed immediately with
TypeError: 'async_generator' object is not iterable
Neither tool detected when the attribute database produces conflicting
name/IP data, silently emitting the conflicts.
confluent2hosts now warns when the same hostname is generated for
multiple different addresses within one address family (dual-stack
IPv4+IPv6 pairs stay silent), which happens naturally in -a mode when a
node has several networks without distinct per-net hostnames.
confluent2dnsmasq now warns when generated reservations share a
hostname across different IPs, reserve the same IP more than once
(dnsmasq refuses to start on a duplicate dhcp-host IP), or reuse a MAC.
Replace input handling with an async, this
permitts screen updates while doing commands.
Implement 'send break' (sysrq) and focus move.
Indicate not-yet-active focus with titlebar color.
Jarrod Johnson
Markus Hilger
Jarrod Johnson