sidecar: typed streaming transport + detached exec, and a fail-safe SYSTEM>UPDATE - #1903
sidecar: typed streaming transport + detached exec, and a fail-safe SYSTEM>UPDATE#1903colinmcardell wants to merge 7 commits into
Conversation
0a58a29 to
8f35747
Compare
|
force pushed lua lint fixes to keep things tidy. |
|
wow!! thank you! i have a couple hectic days but review and testing coming shortly. looking forward to a rollout |
| local x = 64 - (screen.text_extents(pre) + 4 + screen.text_extents(step)) / 2 | ||
| screen.level(10) | ||
| screen.move(x,40) | ||
| screen.text(pre) |
There was a problem hiding this comment.
likely easier to do string concatenation here ie
screen.text(pre .. " " .. step)
and then use text_center instead of pre-calculating width with text_extents.
there was some concern with text_extents some years ago, i need to refresh my memory on the problem.
| int main(int argc, char **argv) { | ||
| // update.sh gates the final reboot on this. exiting 0 before any | ||
| // fork/jack/hardware init proves the new binary links | ||
| if (argc > 1 && strcmp(argv[1], "--check") == 0) { |
There was a problem hiding this comment.
curious if --version would be a micro feature that could double as this check
There was a problem hiding this comment.
that's a good idea.
tehn
left a comment
There was a problem hiding this comment.
reviewed the code, looks good, skimmed the sidecar section.
will be able to test on hardware before the end of the week.
thank you for the thorough notes!






why
with the current converged binary, updates and system commands such as restart are delegated to the sidecar. update fails completely and restart fails often... requiring manual reboot of the device.
update, restart, and shutdown all need privileged, detached execution. the existing update flow runs
tarandupdate.shas concurrent fire-and-forget shell calls, racing each other, then shows "done" before either has finished. when an update step stalls, the screen is stuck on its last message forever.since the sidecar is norns' child process, any command that restarts norns also kills the sidecar and with it the command doing the restarting.
what
this PR reworks the sidecar around a typed, streaming transport with message framing (
norns/sidecar_msg), line streaming (norns/sidecar_lines), and supervised execution (norns/sidecar_shell) as pure, separately tested pieces, andsidecar.cppkeeping the NNG glue. additionally, the sidecar gains the ability to launch detached system commands is added, which keep running after norns stops.the SYSTEM>UPDATE flow is then reworked (using the sidecar transport improvements) into a fail-safe / fail-loud updater that replaces the norns binary and reboots only once the new binary proves launchable, rolling back otherwise. and importantly no longer stalls and fails spectacularly :).
sidecar's protocol can now launch work that outlives norns replacing/restarting itself, so we also got a much more reliable restart mechanism.
highlights
norns/sidecar_shell). commands run with limits. a command that goes quiet or floods output is cut off (each request sets its cap), stderr streams back alongside stdout, and stopping a command asks it to exit before forcing it.os.executeandio.popenare reimplemented over the sidecar, closing the lastfork()opportunities in the JACK-holding converged binary process.os.executekeeps the exact lua contract.io.popenhands back a read-only handle (read/lines/close, see notes below). captured output is delivered through lua'sprint, so command output still lands in maiden.releases.txtreply, which previously left the checking screen spinning forever.norns --checkbefore the device reboots into it..newand keeps.old, so a failed check rolls back to the working install.failed:<step>is the format) used to present messaging on-screen, and returns to a clean pre-update state, rather than booting a broken build.m.releases_urloverride variable for local update testing (see test running the update locally below).verification
failed:<step>, no reboot, on-screen failure)./update.sh)test-updateCI job.update.sh, aimed at a scratch tree through itsNORNS_UPDATE_ROOT/NORNS_UPDATE_SUDOtest-time overrides.the rest of this pr is largely the cleanup that fell out of making this all possible:
sudo -n systemd-run. lua only ever supplies the action name, so no new shell execution capability is exposed to scripts.passdonecopy inlua/core/menu/system.lua(settings and wifi each own their own live copy).test running the update locally
the SYSTEM>UPDATE menu page can now read its release manifest from an
m.releases_urloverride variable, settable at runtime over the repl. otherwise it defaults to the monomereleases.txton github. with this override, the real download -> verify -> swap -> reboot path can be driven against a local http server for testing:1. serve a manifest + bundle.
releases.txtis lua that the menuload()s. it sets a globalreleases:the bundle it points at is a release tarball which is a top dir named for the version, holding
update.sh, the builtnorns/tree (build/norns/norns+build/maiden-repl/maiden-repl),maiden/, theconfig/systemd units,version.txt, andchangelog.txt.update/pack.sh <version>tars that dir and writes the matching checksum:update/pack.sh 999999 # tars ./999999/ -> norns999999.tgz + norns999999.sha256the quickest valid bundle is a copy of an installed release tree with its version bumped. serve
releases.txt, the.tgz, and the.sha256from one dir over the LAN —python3 -m http.server 8000.2. point the device at it over the repl (any restart resets the override to upstream):
note the menu still pings
github.com(and wants 400M free on disk) before it reads the override, so the device needs working internet even though the release itself is served from the LAN.3. open SYSTEM>UPDATE and confirm. the success bundle downloads, verifies, swaps, and reboots into the new version. for the fail-safe paths you don't even need a valid bundle — a corrupt
.tgz, a wrong.sha256, or a bundle missingupdate.sheach surfaces itsfailed:<step>on screen with no reboot.notes
io.popenis partially regressed... "w" mode intentionally fails with a loud error, and reads hand back the finished command's whole output rather than streaming it. this should be fixed in a follow-up PR. it requires additional changes to the sidecar to support it, and i'm intentionally not putting that work here, to keep this already massive amount of change as contained as possible at this point. the system cmd and sidecar work from this PR is already an excellent prefactoring for the changes required to getio.popenfully working.crone_cleanup()→jack_client_closeonly runs when the event loop exits on its own. restart delivers SIGTERM, and with no handler installed the process dies before reaching it, so every restart is an unclean JACK client death. the fix should be a SIGTERM handler that routes into the existing teardown path.RemoveIPC=yes), destroying jackd's shm while it runs. the next restart then fails until a power cycle. the fix here is likely aRemoveIPC=nologind addition. this belongs in norns-image for fresh flashes, but it should likely also ship throughupdate.sh, so existing devices would pick it up on their next update without a reflash.norns.system_cmdnow has a ceiling upstream does not have. a command that stays silent for more than 4 minutes is killed and reported failed. the timeout bounds silence, not total runtime, so a command that keeps producing output can run as long as it needs.os.executeandio.popenpass no ceiling at all, keeping stdlib duration behavior for scripts that use them (this is on the script author).update.shhas indeed outgrown its bash beginnings at ~500 lines... with a rather silly (but kind of fun to write) hand-rolled test rig as an additional ~500 lines. this is fine, but the bash scripting seems to have reached its limit. it is fully functional and tested work, but further expansion should consider moving to python IMO.