The per-MAC 90 second log throttle in proxydhcp has been inert: the
`skiplogging = True` reset sat in relay_proxydhcp, where it is a dead
local, while the loop in proxydhcp only ever assigns False. Once the
first packet is handled the flag stays False for the life of the
process, so every retransmitted boot request logs again even though
ignoredisco is updated to suppress it.
Reset the flag at the top of each loop iteration instead, next to the
timestamp check it belongs to, and drop the dead assignment.
reply_dhcp4 declines to answer a PXE boot request unless
deployment.useinsecureprotocols is set to firmware or always, but
proxydhcp had no such check. A node left at the default of never was
therefore still offered a TFTP bootfile and a plain http boot.ipxe URL
whenever the request arrived on port 4011 rather than port 67, so the
attribute silently did nothing in ProxyDHCP deployments alongside an
independent DHCP server.
Apply the same gate, including the UEFI HTTP boot exemption, and log the
same remediation hint. The node attributes are now fetched once and
passed through to get_deployment_profile instead of being looked up
again there.
Requests whose architecture could not be determined are ignored rather
than falling through to the reply. opts_to_dict stops parsing before the
client architecture option whenever the message type is not a request,
and such a packet would otherwise reach the iPXE branch and be handed a
plain http boot.ipxe URL without ever passing the gate.
The default 'cp' is /lbin/cp, but that fails, use full path to the cp that works.
Perform the hmac registration of api key that was missing.
Remove assumption that the ip will be ipv6, wrapping it only if a : is present in address.
The rsync push carried no preservation flags, so files arrived with their
special permission bits explicitly disabled and a setuid/setgid entry could
only be honored by the permissions= chmod on the client side.
Preservation was turned on once before in e52a9ff70f ("Have syncfiles
attempt to preserve more") and rolled back the same day in c0287e93ed
("Roll back rsync ownership"), because rsync also applied the staging copy's
attributes to the parent directories it merely traversed on the way to the
synced files, clobbering the permissions of system directories such as /etc.
Naming every staged file explicitly through --files-from and adding
--no-implied-dirs confines preservation to the content actually being
synchronized, leaving traversed directories alone and creating missing ones
with default attributes.
Two details follow from the way the staging tree is built. Files are staged as
symlinks, so rsync reads their attributes through to the real file, but
directories are staged as directories and need the source attributes copied
onto them for the otherwise empty ones that have to be named explicitly.
Ownership is mapped from the account the daemon runs as to root, since that
account generally does not exist on the node and would otherwise arrive as a
meaningless numeric id.
--xattrs from that earlier attempt is deliberately left out: with --copy-links
rsync reads xattrs off the symlink rather than its referent, so it transfers
nothing here while adding a failure mode on hosts without xattr support.
chown() clears the setuid bit of a file on Linux (and its setgid bit, if
the file is group-executable), even when run by root and even when the
owner/group are unchanged. Since the owner/group chown ran after the
permissions chmod, any syncfiles entry combining owner=/group= with a
setuid/setgid permissions= value silently lost the special bits.
get_syncresult() caught the sync task's exception, logged a repr server
side and returned 200 OK with a null body. The node then called
.get('options') on that null resulted in:
c1: 'NoneType' object has no attribute 'get'
and syncfileclient still exited 0 as if syncing had succeeded.
Return the error to the requestor as a 500 with an error payload. On
the node, unwrap the body that grab_url_with_status raises for a
non-success status, print it once and exit non-zero. Only a failure the
server deliberately reported for this sync is terminal. Anything else,
such as a dropped connection, is re-raised so the existing retry loop
handles it as before. The same case now reports
c1: Error performing syncfiles: Syncing failed due to unreadable files: /etc/dangling.conf
c1: 'syncfileclient' exited with code 1
nodeattrib/nodegroupattrib -e replaced '.' with '_' in the attribute name
before handing it to the server, not just when looking up the environment
variable. Any attribute with a dot in it was therefore rejected, e.g.
$ export info_note=test
$ nodeattrib -e gpu1 info.note
Traceback (most recent call last):
File "/opt/confluent/bin/nodeattrib", line 97, in <module>
exitcode=client.updateattrib(session,args,nodetype, noderange, options, argassign)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/confluent/lib/python/confluent/client.py", line 688, in updateattrib
key, os.environ[key.upper()])
~~~~~~~~~~^^^^^^^^^^^^^
File "<frozen os>", line 714, in __getitem__
KeyError: 'INFO_NOTE'
$ export INFO_NOTE=test
$ nodeattrib -e gpu1 info.note
Error: Bad Request - info_note attribute on node gpu1 is invalid
Keep the attribute name intact and derive the environment variable name
from it separately. A missing environment variable now reports which
variable names were looked for instead of raising a bare KeyError
traceback.
source_remote imageboot.sh is the last thing the diskless cmdline hook
runs, so returning early on a failed extraction ended the hook and left
dracut to time out. Falling through instead reaches the existing
/sysroot/sbin/init guard, which reports the failure and holds the node so
it stays reachable over ssh, as it did before extraction was checked.
imageboot falls back to cp when unsquashfs is unavailable, but the build
side did not: a bare dracut_install/copy_exec aborts initramfs generation
when the binary is absent, and the capture prerequisite check refused to
capture the image at all.
Mark the initramfs copies optional and report the missing package as an
advisory rather than a hard prerequisite, so such images still build and
capture, just without the faster extraction path. Widen the EL check to
every release past el8 so future ones inherit it.
The copy ran after the cd to the repo root, so it read ../LICENSE from
outside the checkout and never placed the file in confluent_osdeploy/. The
tarball went out without it and the spec's %install, which does
"cp LICENSE" after %setup cds into the unpacked directory, failed.
imgutil/buildrpm already copies before its cd; do the same here.
The aarch64 spec has its LICENSE lines commented out, so only the x86_64
build broke, but the copy was equally wrong in both scripts.
Release tags do not live on master: 3.15.2 through 3.15.6 were tagged on branch
3.15, so git describe reaches only 3.15.1 and dev builds were stamped
3.15.2.dev<n>. Besides being confusing, rpm and dpkg both rank the released
3.15.6 above that, so a dev package will not install over a released one.
Add a top-level VERSION file naming the release the branch is working toward
(4.0.0 on master) and a mkversion helper that stamps packages from it, keeping
the tag-derived value as a floor so a forgotten bump cannot go backwards.
mkversion also replaces the block copy-pasted into seven build scripts, and
makesetup no longer writes a per-package VERSION file, so the stale checked-in
confluent_common/VERSION goes with it.
confluent_imgutil does not depend on confluent_server, and the yaml
import is optional too, yet both capture and pack dereference osimage
and yaml unconditionally when writing manifest.yaml. With
confluent_osdeploy present but the server absent that raises rather
than producing a profile.
Guard the manifest on both being importable and say so, since rebase
is what the manifest exists for. The yaml fallback now binds None
instead of leaving the name undefined.
The two call sites carried the manifest write verbatim in both, so fold
them into one function rather than duplicate the guard as well.
osimport polls import progress from a coroutine, so a blocking sleep
between reads stalls the whole client loop. It was the only use of time
in the script, so the import goes with it.
Both loops that read the importer's output test for a percentage first,
so an ERROR: line whose text carries a % takes the percentage branch and
float() raises instead of the error being reported. The import target
name can carry one too, and that one is user supplied. importmedia runs
as a bare task, so the exception is swallowed and the client polls a
phase that never advances.
Test for ERROR: first and treat an unparsable percentage as no
percentage. Set percent on the error path of the second loop as well,
as the first already does.
The loop that drains the importer's remaining output reads a byte at a
time but clears currline on every iteration, one level out from where
the earlier loop clears it. currline is therefore never longer than a
single byte, so the percentage and ERROR: branches can never match and
the tail of an import is silently discarded.
Clear it only once a line has been consumed, as the earlier loop does.
scan_iso walks an entire ISO with blocking libarchive reads, yielding
only once per entry, and the header-sum branch of fingerprint reads the
whole file with no yield at all. Both run in the daemon, reached from
MediaImporter.init on every fingerprint and importing request.
The scan costs about 8us per entry and is indifferent to media size,
since libarchive seeks past file data rather than reading it: measured
at 80ms for 10k entries whether the image is 0.2 GB or 8.8 GB, and at
310ms for 40k. The header-sum branch is the one that scales with size,
reading a multi-gigabyte image end to end.
Make the pair plain functions and hand them to a thread instead.
Nothing unwound the loop device and dm-crypt mapping when the copy loop
raised, so interrupting a pack stranded both, still holding the profile's
rootimg.sfs.
Tear them down from a finally. The retry loop moves with them, so also
honour its tries counter, as unpack_image already does; spinning forever
inside a finally would hang the interrupt it is meant to clean up after.
A bounded retry loop can also give up, and the detach that follows would
then fail with EBUSY and, raising from a finally, replace the exception
that brought us here. Warn and leave both in place instead.
Both functions became coroutines solely to await one get_hashes call,
but their bodies are long stretches of blocking work: mksquashfs, the
encrypt_image copy loop, rsync, ssh and osdeploy.
From Python 3.11 on, asyncio.run installs a SIGINT handler that cancels
the main task and returns rather than raising, so an interrupt is only
noticed at the next await. Interrupting a pack during mksquashfs
surfaced as a CalledProcessError from the dying child instead of a
KeyboardInterrupt, and with a base profile, where nothing is ever
awaited, pack carried on and published the profile before exiting.
Run the loop only around the call that needs it.
check_output unwraps a single list argument, check_call never did, so
callers passing a list hit a TypeError out of create_subprocess_exec.
Two callers do: the genisoimage run behind Windows profile imports,
where an except Exception swallows the failure and the boot.iso is
silently missing, and the nodeconfig run in discovery, which takes out
automatic node configuration on discovery outright.
capture and pack hash the whole profile directory, which by that point
holds rootimg.sfs, the kernel and the distribution initramfs. rebase
only ever looks up entries that came from the profile source directory,
so the image blobs cost gigabytes of hashing for nothing.
Pass the source directory as the filter, as generate_stock_profiles
already does. Older manifests keep working, since rebase reads their
entries with a default.
The asyncio port added an await between every 2048 byte read, which
roughly doubled the cost of hashing. imgutil runs entire packed images
through this, and the server pays it on rebase and media import.
Read a megabyte per iteration instead. That still yields hundreds of
times per gigabyte, so the event loop stays responsive, and sha512 is
independent of the read size, so existing manifests remain valid.
grab_response_with_status hands back bytes, so every failure path put a
b'<status>error</status>' repr in front of the operator rather than what
the SMM said. Decode at the raise, replacing rather than failing on a
body that is not valid utf8. The bodies still reach fromstring() as
bytes, which is what lxml wants when the xml carries an encoding
declaration.
The poll loop spends its retry budget on a poll that goes unanswered but
aborted the update on the first non-200, even though an SMM restarting
its web service part way through the apply keeps answering, with
whatever its httpd has to say, before it stops answering at all. Give a
bad status the same budget as a dead connection.
Every other /data call checks the status, this one discarded the
response, so an SMM that refused the user record was reported to the
caller as a successful privilege change.
Staleness is judged by age alone, so a session the SMM ended on its own
reached the operator as a raw error body instead of being retried.
Route the /data calls through a helper that logs back in and retries
once on a 401, which is how the SMM answers once a session is gone.
A hostname or domain write does not end the session, measured on a
DW612S at firmware 1.18, so this covers what the chassis drops by
itself, not a self-inflicted loss.
The counter is there to ride out a few unanswered progress polls, but
nothing ever cleared it, so three failures spread across a long apply
exhausted it and aborted an update that was still making progress.
A firmware update posts on one session for the minutes its apply loop
runs, and an FFDC collection downloads on the session it acquired, but
wc() judges a session by its age alone, so a settings call arriving
thirty seconds in logged that session out from underneath them. Flag
the long operations the way the IMM and XCC handlers already do and
leave their session in place.
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.
Markus Hilger
Jarrod Johnson
Jarrod Johnson