OpsDeck/module_updates
Watch
2
0
Fork
You've already forked module_updates
 0
generated from OpsDeck/module_template
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

feat: restart the host, now or on a delay #3

Merged
thisilike merged 2 commits from feat/restart-host into main 2026-10-19 10:38:50 +02:00
Conversation 3 Commits 2 Files changed 10 +483 -14
thisilike commented 2026-09-21 18:35:52 +02:00
Owner

The brief said not to add a reboot action unless asked for; it has now been asked for. The page could already say a reboot was needed and was the one place that could not do anything about it. README.md's "There is no reboot action, by design" goes with it.

Rebased onto main after the task-runs rework landed, so the gates below speak the current model (isBusy("apt"), not the old running boolean).

What it looks like

A form rather than an action, because the delay is the decision:

control does
Restart host — When: Now (default), 1 / 5 / 15 / 30 / 60 minutes shutdown -r now|+N "OpsDeck: restart requested by <user>"
Cancel scheduled restart (only while one is pending) shutdown -c

"Now" restarts on one confirmation; anything else schedules a restart that the cancel action calls off until it fires. The wall message names the admin who asked, and both routes are role: "admin" and confirmed.

The Host block gains Restart (the capability, or why not) and Scheduled restart (relative time).

Not a task run

shutdown returns as soon as systemd has accepted the request, so there is no work to follow and nothing for Activity to hold — unlike the apt actions, these two answer directly. What is worth watching is the host, and that is on the page: <hostRoot>/run/systemd/shutdown/scheduled, which systemd writes when it accepts a shutdown and removes on shutdown -c. So a restart somebody scheduled over SSH shows up too, and asking costs no host command. parseScheduledShutdown is a pure parser in apt.ts with tests, like everything else there.

Two gates

  • Host PID namespace. shutdown asks systemd over the bus and systemd authenticates bus peers by pid, so a mount strategy cannot substitute. host.ts gained the probe (host /proc/self/stat vs Deno.pid — the read that does not need Deno's --allow-all); without it the routes answer 503 naming pid: host, and the page says the same instead of failing at the click. The compose example already sets it.
  • isBusy("apt"). Restart is the one action still refused while the lock is busy: it does not queue behind it the way a second upgrade does, and a dist-upgrade cut in half leaves dpkg wanting --configure -a on a host that is now also rebooting. isBusy is "not terminal", so a queued run is refused too — that is exactly the run a reboot would strand before it started. Cancelling is never refused; mid-run is when it is wanted.

Worth a reviewer's eye: this is the one place that deliberately does not follow "gate on host access alone". canRestart goes dark while a run is live, because a button that reboots the host is the wrong place to learn about a 409.

Known rough edge

An immediate restart takes OpsDeck down with the host, so that response races the machine going away — the page usually shows the connection dropping rather than a result. That is the reason the delayed path exists, and it is in the README.

Verified

deno check, deno lint, deno fmt --check, 58 tests pass (56 existing + 2 new parser tests; alerts_test.ts's state fixture gained shutdown: null). Ran against a fixture host tree: schema loads valid, the form and cancel action render, shutdownAtMs parses, the gates answer 503 with their reason. The actual shutdown call is untested — it needs a Debian host with pid: host.

🤖 Generated with Claude Code

The brief said not to add a reboot action unless asked for; it has now been asked for. The page could already say a reboot was needed and was the one place that could not do anything about it. `README.md`'s "There is **no reboot action**, by design" goes with it. Rebased onto `main` after the task-runs rework landed, so the gates below speak the current model (`isBusy("apt")`, not the old `running` boolean). ## What it looks like A **form** rather than an action, because the delay is the decision: | control | does | |---|---| | Restart host — When: **Now** (default), 1 / 5 / 15 / 30 / 60 minutes | `shutdown -r now\|+N "OpsDeck: restart requested by <user>"` | | Cancel scheduled restart (only while one is pending) | `shutdown -c` | "Now" restarts on one confirmation; anything else schedules a restart that the cancel action calls off until it fires. The wall message names the admin who asked, and both routes are `role: "admin"` and confirmed. The Host block gains **Restart** (the capability, or why not) and **Scheduled restart** (relative time). ## Not a task run `shutdown` returns as soon as systemd has accepted the request, so there is no work to follow and nothing for Activity to hold — unlike the apt actions, these two answer directly. What is worth watching is the host, and that is on the page: `<hostRoot>/run/systemd/shutdown/scheduled`, which systemd writes when it accepts a shutdown and removes on `shutdown -c`. So a restart somebody scheduled over SSH shows up too, and asking costs no host command. `parseScheduledShutdown` is a pure parser in `apt.ts` with tests, like everything else there. ## Two gates - **Host PID namespace.** `shutdown` asks systemd over the bus and systemd authenticates bus peers *by pid*, so a mount strategy cannot substitute. `host.ts` gained the probe (host `/proc/self/stat` vs `Deno.pid` — the read that does not need Deno's `--allow-all`); without it the routes answer 503 naming `pid: host`, and the page says the same instead of failing at the click. The compose example already sets it. - **`isBusy("apt")`.** Restart is the one action still refused while the lock is busy: it does not queue behind it the way a second upgrade does, and a dist-upgrade cut in half leaves dpkg wanting `--configure -a` on a host that is now also rebooting. `isBusy` is "not terminal", so a *queued* run is refused too — that is exactly the run a reboot would strand before it started. Cancelling is never refused; mid-run is when it is wanted. Worth a reviewer's eye: this is the one place that deliberately does **not** follow "gate on host access alone". `canRestart` goes dark while a run is live, because a button that reboots the host is the wrong place to learn about a 409. ## Known rough edge An immediate restart takes OpsDeck down with the host, so that response races the machine going away — the page usually shows the connection dropping rather than a result. That is the reason the delayed path exists, and it is in the README. ## Verified `deno check`, `deno lint`, `deno fmt --check`, **58 tests pass** (56 existing + 2 new parser tests; `alerts_test.ts`'s state fixture gained `shutdown: null`). Ran against a fixture host tree: schema loads valid, the form and cancel action render, `shutdownAtMs` parses, the gates answer 503 with their reason. **The actual `shutdown` call is untested** — it needs a Debian host with `pid: host`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
The brief said not to add a reboot action unless asked for; it has now been
asked for. The page could already say a reboot was needed and was the one
place that could not do anything about it.

A form rather than an action, because the delay is the decision: "Now"
restarts on one confirmation, anything else schedules a restart that "Cancel
scheduled restart" calls off until it fires. Both run `shutdown` on the host,
whose wall message names the admin who asked.

What is scheduled is read back from /run/systemd/shutdown/scheduled — systemd
writes it when it accepts a shutdown and removes it on `shutdown -c` — so a
restart somebody scheduled over SSH shows up too, and asking costs no host
command.

Two gates. `shutdown` asks systemd over the bus and systemd authenticates bus
peers by pid, so the routes require the host PID namespace and the page says
so when the container lacks it, rather than failing at the click; host.ts
gained the probe for that. And a restart is refused while an update run is in
progress: a dist-upgrade cut in half leaves dpkg wanting `--configure -a` on a
host that is now also rebooting.

An immediate restart takes OpsDeck down with the host, so that response races
the machine going away and the client may never see it. Nothing to be done
about it, but it is why the delayed path exists.
thisilike force-pushed feat/restart-host from 11d67e8d58 to 9de3df4208 2026-09-21 18:44:47 +02:00 Compare
julian requested changes 2026-09-22 12:08:40 +02:00
Dismissed
julian left a comment

Reviewed at 9de3df4. The design (form + cancel, pid-namespace probe, isBusy gate on restart) holds up; two things need fixing before merge, the rest are small.

Blocking

backend/mod.ts — starter() / /upgrade/*: an upgrade can still be started while a restart is scheduled.
The isBusy("apt") gate closes one direction (no restart while a run is live), but the other direction is open: schedule a restart in 30 minutes, then click "Install all updates". The run starts, the reboot fires mid dist-upgrade, and the host is in exactly the dpkg --configure -a state the PR text says the gate exists to prevent. Refuse /upgrade/security and /upgrade/all with a 409 while s.shutdown !== null (a middleware next to pidNamespaceRequired is enough), and drop canUpgradeSecurity / canUpgradeAll in /summary for the same condition so the buttons go dark rather than 409. /refresh can stay open — an interrupted apt-get update is harmless.

backend/mod.ts — /restart body parsing coerces bad input to an immediate reboot.
Math.floor(Number(body.delayMin ?? 0)) turns "", null, false, and [] into 0, which is shutdown -r now. A client that sends an empty or malformed body (or a select that submits "") reboots the host right away instead of getting a 400. Require delayMin to be present and either a non-negative integer or a /^\d+$/ string; anything else is a 400. The form's default of "0" still gets "Now" on the happy path.

Non-blocking

  • backend/mod.ts /summary: restartText says available while a run is busy, but canRestart is false so the form's submit is hidden. Add a branch for that case (blocked — update run in progress) so the Host block matches the controls.
  • backend/host.ts probePidNamespace: a failed read (hostProc not mounted, Deno permission) becomes false, and the UI then tells the operator to add pid: host — wrong advice when the compose file already has it. Keep the failure reason and surface it separately from "own PID namespace".
  • backend/mod.ts /restart: the isBusy check and the shutdown call are not atomic; a run started in between proceeds. Small window, fine to accept, but worth a comment.
  • backend/mod.ts /restart: systemctl that believes it is in a chroot prints Running in chroot, ignoring request. and exits 0. Only reachable if the pid-namespace probe false-positives, but the response would then say "restarting now" with nothing happening. Cheap defence: treat stderr containing ignoring request as a failure.
  • backend/mod.ts UI schema: "Cancel scheduled restart" and canCancelRestart are also true for a MODE=poweroff / halt scheduled over SSH. Either use s.shutdown.mode in the label or gate on mode === "reboot".

Checked and fine: core's relativeMs renders future times ("in 5m") and null as "—"; the compose file sets pid: host and OPSDECK_HOST_PROC=/host/root/proc, so the /host/root/proc/self/stat probe is legitimate; the wall message goes through argv (chroot/nsenter), not a shell, so the user name is safe.

Reviewed at 9de3df4. The design (form + cancel, pid-namespace probe, `isBusy` gate on restart) holds up; two things need fixing before merge, the rest are small. ## Blocking **`backend/mod.ts` — `starter()` / `/upgrade/*`: an upgrade can still be started while a restart is scheduled.** The `isBusy("apt")` gate closes one direction (no restart while a run is live), but the other direction is open: schedule a restart in 30 minutes, then click "Install all updates". The run starts, the reboot fires mid dist-upgrade, and the host is in exactly the `dpkg --configure -a` state the PR text says the gate exists to prevent. Refuse `/upgrade/security` and `/upgrade/all` with a 409 while `s.shutdown !== null` (a middleware next to `pidNamespaceRequired` is enough), and drop `canUpgradeSecurity` / `canUpgradeAll` in `/summary` for the same condition so the buttons go dark rather than 409. `/refresh` can stay open — an interrupted `apt-get update` is harmless. **`backend/mod.ts` — `/restart` body parsing coerces bad input to an immediate reboot.** `Math.floor(Number(body.delayMin ?? 0))` turns `""`, `null`, `false`, and `[]` into `0`, which is `shutdown -r now`. A client that sends an empty or malformed body (or a select that submits `""`) reboots the host right away instead of getting a 400. Require `delayMin` to be present and either a non-negative integer or a `/^\d+$/` string; anything else is a 400. The form's default of `"0"` still gets "Now" on the happy path. ## Non-blocking - `backend/mod.ts` `/summary`: `restartText` says `available` while a run is busy, but `canRestart` is `false` so the form's submit is hidden. Add a branch for that case (`blocked — update run in progress`) so the Host block matches the controls. - `backend/host.ts` `probePidNamespace`: a failed read (hostProc not mounted, Deno permission) becomes `false`, and the UI then tells the operator to add `pid: host` — wrong advice when the compose file already has it. Keep the failure reason and surface it separately from "own PID namespace". - `backend/mod.ts` `/restart`: the `isBusy` check and the `shutdown` call are not atomic; a run started in between proceeds. Small window, fine to accept, but worth a comment. - `backend/mod.ts` `/restart`: `systemctl` that believes it is in a chroot prints `Running in chroot, ignoring request.` and exits 0. Only reachable if the pid-namespace probe false-positives, but the response would then say "restarting now" with nothing happening. Cheap defence: treat stderr containing `ignoring request` as a failure. - `backend/mod.ts` UI schema: "Cancel scheduled restart" and `canCancelRestart` are also true for a `MODE=poweroff` / `halt` scheduled over SSH. Either use `s.shutdown.mode` in the label or gate on `mode === "reboot"`. Checked and fine: core's `relativeMs` renders future times ("in 5m") and `null` as "—"; the compose file sets `pid: host` and `OPSDECK_HOST_PROC=/host/root/proc`, so the `/host/root/proc/self/stat` probe is legitimate; the wall message goes through argv (chroot/nsenter), not a shell, so the user name is safe.
Review follow-up on the restart form.

The isBusy gate closed one direction only. Scheduling a restart in 30
minutes and then installing updates let the reboot land mid dist-upgrade,
the dpkg --configure -a state the gate exists to prevent. /upgrade/* now
answer 409 while any shutdown is scheduled (ours or one from SSH, any mode),
reading the systemd file fresh rather than the 10-second cache, and
canUpgradeSecurity/canUpgradeAll go false with it. /refresh stays open.
The two gates exclude each other in-process: /restart sets a flag and the
upgrade gate a counter, each checked before the other's first await, so the
check-then-act window the review accepted with a comment is closed instead.

/restart coerced "", null, false, [] and a missing or unparsable body to 0,
which is `shutdown -r now`. delayMin must now be a non-negative integer or a
digit string; anything else is a 400. The parse lives in restart.ts, pure
and tested.

Smaller:
- a failed PID namespace probe no longer reads as "own namespace"; the
  reason is kept and surfaced instead of the `pid: host` advice
- restartText says "blocked — update run in progress" when the form is hidden
- systemctl's exit-0 "Running in chroot, ignoring request." is a failure
- the cancel action says "shutdown": it calls off any mode, and a poweroff
  scheduled over SSH now blocks the upgrades, so it stays offered for them
thisilike commented 2026-09-23 10:45:52 +02:00
Author
Owner

Thanks, all seven points checked against the code. Five fixed as you suggested. The isBusy/shutdown race is closed rather than commented, and the cancel gate is fixed a different way; reasons for both are below. Everything is in 9826480.

Blocking

Upgrade while a restart is scheduled. Confirmed, fixed. /upgrade/security and /upgrade/all now go through a noShutdownScheduled middleware that returns 409 while /run/systemd/shutdown/scheduled exists. It checks for any mode, not only reboot, because a poweroff scheduled over SSH cuts a dist-upgrade off just as hard. It reads the file fresh on each request, not from the 10 s load() cache, since the cache could miss a restart scheduled a second earlier. canUpgradeSecurity / canUpgradeAll also go false while one is scheduled. /refresh stays open.

delayMin coercion. Confirmed: "", null, false, [], a missing field and an unparsable body (.catch(() => ({}))) all ended up as shutdown -r now. It is now parseRestartDelay in a new pure restart.ts. It accepts a non-negative integer or a /^\d+$/ string up to the max, and returns 400 for anything else. restart_test.ts covers those cases plus true, "1.5", " 5", "0x10", "1e2" and over-max.

Non-blocking

  • restartText while busy: fixed, it now says blocked — update run in progress.
  • probePidNamespace read failure: fixed. The host now exposes pidNamespaceError. When it is set, restartText and the 503 show the read error and point at OPSDECK_HOST_PROC instead of recommending pid: host. The log level also went from debug to warn.
  • isBusy / shutdown not atomic: I closed this instead of adding a comment, because once the first blocking fix is in, the race exists in both directions. Now /restart sets a restarting flag and the upgrade gate raises an upgradesStarting counter. Each one checks the other side's value synchronously, with no await between the check and its own set, so the single-threaded event loop makes them exclude each other. /restart also parses the body before the busy check now, so that check and setting the flag happen with no await between them. This does not cover a restart scheduled over SSH at the same moment someone clicks upgrade, and nothing inside the process can.
  • chroot ignoring request exit 0: fixed for both /restart and /restart/cancel: shutdownError now treats exit 0 with ignoring request in its output as a failure.
  • Cancel label for poweroff/halt: partly disagree. The label was wrong, but I don't think the action should be limited to mode === "reboot". shutdown -c cancels any mode, and because of the first blocking fix, a poweroff scheduled over SSH now also blocks the upgrades. If we hid the cancel for that case, the page would show "poweroff scheduled", upgrade buttons that are greyed out, and no way to clear it except a shell. So the action is now labelled "Cancel scheduled shutdown", its confirmation names all three modes, and the field reads "Scheduled shutdown".

Checks: deno lint and deno check are clean, and deno test passes (61 passed, 0 failed), all run inside a core checkout. I did not reboot a real host with it, because neither restart route can run against a fixture tree. deno fmt --line-width 100 reports one hunk in mod.ts (L398) that was already there before this PR; I left it alone.

Thanks, all seven points checked against the code. Five fixed as you suggested. The isBusy/shutdown race is closed rather than commented, and the cancel gate is fixed a different way; reasons for both are below. Everything is in 9826480. ## Blocking **Upgrade while a restart is scheduled.** Confirmed, fixed. `/upgrade/security` and `/upgrade/all` now go through a `noShutdownScheduled` middleware that returns 409 while `/run/systemd/shutdown/scheduled` exists. It checks for any mode, not only `reboot`, because a poweroff scheduled over SSH cuts a dist-upgrade off just as hard. It reads the file fresh on each request, not from the 10 s `load()` cache, since the cache could miss a restart scheduled a second earlier. `canUpgradeSecurity` / `canUpgradeAll` also go false while one is scheduled. `/refresh` stays open. **`delayMin` coercion.** Confirmed: `""`, `null`, `false`, `[]`, a missing field and an unparsable body (`.catch(() => ({}))`) all ended up as `shutdown -r now`. It is now `parseRestartDelay` in a new pure `restart.ts`. It accepts a non-negative integer or a `/^\d+$/` string up to the max, and returns 400 for anything else. `restart_test.ts` covers those cases plus `true`, `"1.5"`, `" 5"`, `"0x10"`, `"1e2"` and over-max. ## Non-blocking - **`restartText` while busy**: fixed, it now says `blocked — update run in progress`. - **`probePidNamespace` read failure**: fixed. The host now exposes `pidNamespaceError`. When it is set, `restartText` and the 503 show the read error and point at `OPSDECK_HOST_PROC` instead of recommending `pid: host`. The log level also went from debug to warn. - **isBusy / shutdown not atomic**: I closed this instead of adding a comment, because once the first blocking fix is in, the race exists in both directions. Now `/restart` sets a `restarting` flag and the upgrade gate raises an `upgradesStarting` counter. Each one checks the other side's value synchronously, with no await between the check and its own set, so the single-threaded event loop makes them exclude each other. `/restart` also parses the body *before* the busy check now, so that check and setting the flag happen with no await between them. This does not cover a restart scheduled over SSH at the same moment someone clicks upgrade, and nothing inside the process can. - **chroot `ignoring request` exit 0**: fixed for both `/restart` and `/restart/cancel`: `shutdownError` now treats exit 0 with `ignoring request` in its output as a failure. - **Cancel label for poweroff/halt**: partly disagree. The label was wrong, but I don't think the action should be limited to `mode === "reboot"`. `shutdown -c` cancels any mode, and because of the first blocking fix, a poweroff scheduled over SSH now also blocks the upgrades. If we hid the cancel for that case, the page would show "poweroff scheduled", upgrade buttons that are greyed out, and no way to clear it except a shell. So the action is now labelled "Cancel scheduled shutdown", its confirmation names all three modes, and the field reads "Scheduled shutdown". Checks: `deno lint` and `deno check` are clean, and `deno test` passes (61 passed, 0 failed), all run inside a core checkout. I did not reboot a real host with it, because neither restart route can run against a fixture tree. `deno fmt --line-width 100` reports one hunk in `mod.ts` (L398) that was already there before this PR; I left it alone.
julian approved these changes 2026-09-23 15:09:02 +02:00
julian left a comment

Re-reviewed at 9826480. Every point from the previous review is fixed.

  • noShutdownScheduled reads the scheduled file fresh on every request and accepts any mode. /upgrade/* returns 409 and canUpgrade* goes false.
  • parseRestartDelay returns 400 for malformed input and never falls back to an immediate reboot. restart_test.ts covers the coercion cases.
  • The restarting flag and the upgradesStarting counter exclude each other. /restart parses the body first, then checks isBusy || upgradesStarting || restarting and sets the flag with no await in between. The upgrade middleware checks the flag after its only await and increments before next(). Both reset in finally.
  • pidNamespaceError is surfaced separately from the pid: host advice. shutdownIgnored() catches the chroot exit-0 case. The cancel is relabelled "Cancel scheduled shutdown", and your reason for not gating it on mode === "reboot" makes sense.

One non-blocking nit: if shutdown hits REBOOT_TIMEOUT_MS, the flag clears and the response reports failure, but systemd may already have accepted the request. For a delayed restart that is harmless, because the scheduled file then blocks upgrades anyway. It may be worth wording that error as "outcome unknown" rather than "failed".

The actual reboot path is still untested on a real host, as the PR says. Smoke-test it on prod after deploy.

Re-reviewed at 9826480. Every point from the previous review is fixed. - `noShutdownScheduled` reads the scheduled file fresh on every request and accepts any mode. `/upgrade/*` returns 409 and `canUpgrade*` goes false. - `parseRestartDelay` returns 400 for malformed input and never falls back to an immediate reboot. `restart_test.ts` covers the coercion cases. - The `restarting` flag and the `upgradesStarting` counter exclude each other. `/restart` parses the body first, then checks `isBusy || upgradesStarting || restarting` and sets the flag with no await in between. The upgrade middleware checks the flag after its only await and increments before `next()`. Both reset in `finally`. - `pidNamespaceError` is surfaced separately from the `pid: host` advice. `shutdownIgnored()` catches the chroot exit-0 case. The cancel is relabelled "Cancel scheduled shutdown", and your reason for not gating it on `mode === "reboot"` makes sense. One non-blocking nit: if `shutdown` hits `REBOOT_TIMEOUT_MS`, the flag clears and the response reports failure, but systemd may already have accepted the request. For a delayed restart that is harmless, because the scheduled file then blocks upgrades anyway. It may be worth wording that error as "outcome unknown" rather than "failed". The actual reboot path is still untested on a real host, as the PR says. Smoke-test it on prod after deploy.
thisilike deleted branch feat/restart-host 2026-10-19 10:38:51 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
julian
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
OpsDeck/module_updates!3
No description provided.