From a4626d25697171167bb1d74b1a97c053c6296ddb Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 15:43:54 +0200 Subject: [PATCH] Stop root workers breaking every page, and let the panel be closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ── The 500 ───────────────────────────────────────────────────────────────── "touch(): Utime failed: Operation not permitted", from BladeCompiler, on every request for the affected view. Blade compiles a view and then touch()es the compiled file to the source's mtime. touch() with an explicit time needs ownership of the file. A compose service without `user:` runs as ROOT — and queue, scheduler and reverb had none — so the first worker to render a view wrote a root-owned compiled file that the web process, as www-data, could never refresh again. The queue is what renders mails, which is why this surfaced now. queue, reverb and scheduler run as www-data. queue-provisioning stays root and says why: it brings wg0 up and runs `wg set` for every peer change, which needs NET_ADMIN on the running process. It renders no mail. And update.sh normalises ownership at the END of a run as well as at the start. The first call heals what a previous run left; the second heals what this one made — `git checkout` rewrites the tree as the service account while the old build is still serving. ── The panel that would not go away ───────────────────────────────────────── The same 500 is why it kept coming back: every poll failed, the watcher treats a failed request as "still restarting" (which it normally is), and the overlay stayed up over a console nobody could then reach to find out why. A restart is seconds. After two minutes of nothing the panel now says so and offers a way out — for an operator only, behind the same flag as the step and the log, so a customer on the 503 page is not shown a button suggesting they can call the deployment off. A successful answer clears it again: one bad minute must not leave a "something is wrong" notice sitting there for the rest of the run. Not an always-present close button. The deployment does not stop because somebody dismissed a panel, and offering that at the wrong moment is a lie. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- deploy/update.sh | 8 +++ docker-compose.yml | 21 +++++++ lang/de/updating.php | 3 + lang/en/updating.php | 3 + resources/js/app.js | 26 +++++++++ .../views/partials/updating-panel.blade.php | 18 ++++++ tests/Feature/Admin/UpdateButtonTest.php | 58 +++++++++++++++++++ 8 files changed, 138 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 085c0f2..5bdcf5c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.14 +1.3.15 diff --git a/deploy/update.sh b/deploy/update.sh index fbf00b2..fe819a3 100755 --- a/deploy/update.sh +++ b/deploy/update.sh @@ -449,6 +449,14 @@ if [[ $proxy_running -eq 1 ]]; then done fi +# Again, at the end. The first call heals what a previous run left; this one +# heals what THIS run made — `git checkout` rewrites the tree as the service +# account, and anything a root process wrote into storage while the old build +# was serving would otherwise stay root-owned. A compiled Blade view in that +# state answers every request for it with 500: touch() with an explicit mtime +# needs ownership, and the framework does exactly that on recompile. +normalise_ownership + phase restart "Restarting services" docker compose up -d # Workers hold their PHP classes for the life of the process; without this they diff --git a/docker-compose.yml b/docker-compose.yml index 37fecc3..98a517d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -42,6 +42,9 @@ services: reverb: image: clupilot-app:dev restart: unless-stopped + # www-data, for the same reason as the queue above: a Blade view compiled + # by a root process can never be refreshed by the web process again. + user: "www-data" command: php artisan reverb:start --host=0.0.0.0 --port=8080 volumes: - .:/var/www/html @@ -54,6 +57,16 @@ services: queue: image: clupilot-app:dev restart: unless-stopped + # As www-data, like php-fpm in the app container — NOT as root, which is + # what a compose service without `user:` gets. + # + # Blade compiles a view the first time it is rendered and then touch()es the + # compiled file to the source's mtime. touch() with an explicit time needs + # ownership, so a compiled file written by a ROOT worker cannot be refreshed + # afterwards by the web process: every request for that view answered 500 + # with "touch(): Utime failed: Operation not permitted". Reported from live + # right after a deployment, and it is the queue that renders mails. + user: "www-data" command: php artisan queue:work redis --tries=3 --timeout=90 volumes: - .:/var/www/html @@ -73,6 +86,11 @@ services: restart: unless-stopped # Bring up wg0 (from the mounted config, once the operator has installed it) # before starting the worker, so LocalWireguardHub can manage peers. + # The one worker that stays root: it brings wg0 up and runs `wg set` for + # every peer change, which needs NET_ADMIN on the running process. It + # renders no mail, so it is not the one that was writing compiled views — + # and update.sh normalises ownership at the end of every deployment, which + # heals it if it ever does. command: sh -c 'wg-quick up wg0 2>/dev/null || true; exec php artisan queue:work provisioning --queue=provisioning --tries=1 --timeout=2100 --sleep=3' cap_add: - NET_ADMIN @@ -160,6 +178,9 @@ services: scheduler: image: clupilot-app:dev restart: unless-stopped + # www-data, for the same reason as the queue above: a Blade view compiled + # by a root process can never be refreshed by the web process again. + user: "www-data" command: php artisan schedule:work volumes: - .:/var/www/html diff --git a/lang/de/updating.php b/lang/de/updating.php index e510c94..a939374 100644 --- a/lang/de/updating.php +++ b/lang/de/updating.php @@ -5,4 +5,7 @@ return [ 'title' => 'Gleich wieder da.', 'body' => 'Wir spielen gerade ein Update ein. Ihre Nextcloud läuft davon unberührt weiter — nur dieses Portal ist für einen Moment nicht erreichbar.', 'auto' => 'Diese Seite lädt sich von selbst neu.', + + 'stuck' => 'Der Server antwortet seit einigen Minuten nicht mehr. Das ist länger als ein Neustart dauert — vermutlich ist die Aktualisierung durch und etwas anderes stimmt nicht.', + 'dismiss' => 'Anzeige schließen', ]; diff --git a/lang/en/updating.php b/lang/en/updating.php index 11d2b6c..54248b7 100644 --- a/lang/en/updating.php +++ b/lang/en/updating.php @@ -5,4 +5,7 @@ return [ 'title' => 'Back in a moment.', 'body' => 'We are applying an update. Your Nextcloud keeps running — only this portal is briefly unavailable.', 'auto' => 'This page reloads itself.', + + 'stuck' => 'The server has not answered for several minutes. That is longer than a restart takes — the update has probably finished and something else is wrong.', + 'dismiss' => 'Close this notice', ]; diff --git a/resources/js/app.js b/resources/js/app.js index b499a09..033ee5b 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -214,6 +214,16 @@ document.addEventListener('alpine:init', () => { // has not got there yet. serverConfirmed: running, unconfirmedPolls: 0, + // Consecutive failed requests. Expected during the restart — that IS + // the event — but not forever: if the application comes back broken, + // every request fails and the overlay used to stay up over a console + // nobody could then reach to find out why. Reported exactly that way, + // together with the 500 that caused it. + failedPolls: 0, + // Shown once the failures stop looking like a restart. Not a "close" + // button that is always there: the run does not stop because somebody + // dismissed a dialog, and offering that at the wrong moment is a lie. + stuck: false, // Already translated server-side ("Schritt: …", "Läuft seit …") — the // client only ever displays these, never assembles them, so German // text stays out of JavaScript entirely. @@ -280,6 +290,8 @@ document.addEventListener('alpine:init', () => { // Stay open while we are waiting for the agent to confirm. this.wasRunning = state.running || (this.wasRunning && !this.serverConfirmed); + this.failedPolls = 0; + this.stuck = false; this.step = state.step ?? null; this.runningSince = state.running_since ?? null; this.log = state.log ?? null; @@ -287,11 +299,25 @@ document.addEventListener('alpine:init', () => { // Expected while the containers are down. Keep asking: this is // the middle of the very event being watched, not a fault. this.wasRunning = true; + + // A deployment restart is seconds. Two minutes of nothing is + // not a restart any more — it is an installation that came back + // broken, and the operator needs the page rather than a panel + // telling them to wait. + if (++this.failedPolls > 40) { + this.stuck = true; + } } this.schedule(); }, + // The way out, offered only once `stuck` is true. + dismiss() { + this.wasRunning = false; + clearTimeout(this.timer); + }, + destroy() { clearTimeout(this.timer); }, diff --git a/resources/views/partials/updating-panel.blade.php b/resources/views/partials/updating-panel.blade.php index 54140cf..78d08aa 100644 --- a/resources/views/partials/updating-panel.blade.php +++ b/resources/views/partials/updating-panel.blade.php @@ -77,6 +77,12 @@ color: var(--cpu-muted); } .cpu-foot { margin-top: 2.25rem; font-family: "IBM Plex Mono", "Plex Mono", ui-monospace, monospace; font-size: 0.7rem; text-transform: uppercase; letter-spacing: 0.07em; color: var(--cpu-muted); } + .cpu-stuck { margin-top: 1.5rem; padding-top: 1.25rem; border-top: 1px solid var(--cpu-line); } + .cpu-stuck p { margin: 0; font-size: 0.85rem; color: var(--cpu-muted); } + .cpu-dismiss { margin-top: 0.75rem; padding: 0.5rem 1rem; border: 1px solid var(--cpu-line); + border-radius: 0.6rem; background: var(--cpu-card); color: var(--cpu-fg); + font: inherit; font-size: 0.85rem; font-weight: 500; cursor: pointer; } + .cpu-dismiss:hover { background: var(--cpu-card-2); } .cpu-detail { margin-top: 2rem; text-align: left; } .cpu-step { margin: 0; padding: 0.8rem 1rem; border: 1px solid var(--cpu-line); border-radius: 0.7rem; background: var(--cpu-card); font-size: 0.9rem; font-weight: 500; } @@ -125,5 +131,17 @@ @endif

{{ __('updating.auto') }}

+ + {{-- Only when the server has stopped answering for long enough that + this is no longer a restart. An always-present close button would + be a lie — the deployment does not stop because somebody dismissed + a panel — but being held behind one while the application is broken + is worse. --}} + @if ($detail ?? false) +
+

{{ __('updating.stuck') }}

+ +
+ @endif diff --git a/tests/Feature/Admin/UpdateButtonTest.php b/tests/Feature/Admin/UpdateButtonTest.php index b95c8dc..3076494 100644 --- a/tests/Feature/Admin/UpdateButtonTest.php +++ b/tests/Feature/Admin/UpdateButtonTest.php @@ -893,3 +893,61 @@ it('reproduces the gap the console used to reload on', function () { // …and has not written its status yet. expect(app(UpdateChannel::class)->state()['running'])->toBeFalse(); }); + +it('does not hold the console behind the panel when the application comes back broken', function () { + // Reported together with a 500: after the update every request failed, the + // watcher treated each failure as "still restarting", and the overlay + // stayed up over a console nobody could reach to find out why. + // + // A restart is seconds. Two minutes of nothing is an installation that came + // back broken, and then the operator needs the page. + $watcher = Illuminate\Support\Facades\File::get(resource_path('js/app.js')); + + expect($watcher)->toContain('this.failedPolls > 40') + ->toContain('this.stuck = true') + // A successful answer clears it: one bad minute must not leave a + // "something is wrong" notice sitting there for the rest of the run. + ->toContain('this.failedPolls = 0'); +}); + +it('offers the way out only once it is true, and only to an operator', function () { + // An always-present close button would be a lie: the deployment does not + // stop because somebody dismissed a panel. And the 503 page has no Alpine + // at all, so the affordance lives behind the same operator flag as the step + // and the log. + $panel = Illuminate\Support\Facades\File::get(resource_path('views/partials/updating-panel.blade.php')); + + expect($panel)->toContain('x-show="stuck"') + ->toContain('@click="dismiss()"'); + + // Two operator-only blocks, both gated by the same flag: the deployment + // detail and this. The 503 page passes no `detail`, so a customer sees + // neither the step nor a button suggesting they can call the update off. + expect(substr_count($panel, "@if (\$detail ?? false)"))->toBe(2); +}); + +it('runs the workers as the same user as the web process', function () { + // Blade compiles a view and then touch()es the compiled file to the + // source's mtime. touch() with an explicit time needs ownership — so a view + // compiled by a ROOT worker can never be refreshed by the web process + // again, and every request for it answers 500 with "touch(): Utime failed". + // A compose service without `user:` runs as root, and the queue is what + // renders mails. + $compose = Illuminate\Support\Facades\File::get(base_path('docker-compose.yml')); + + foreach (['queue', 'reverb', 'scheduler'] as $service) { + // From the service key to the next one at the same indentation. An + // earlier version cut at the first "\n ", which is the end of the + // block's own first line. + preg_match('/\n '.$service.':\n(.*?)(?=\n \S|\z)/s', $compose, $match); + + expect($match[1] ?? '')->toContain('user: "www-data"'); + } + + // And the deployment repairs whatever a root process left behind, at the + // end of the run as well as at the start. + expect(substr_count( + Illuminate\Support\Facades\File::get(base_path('deploy/update.sh')), + "\nnormalise_ownership\n", + ))->toBe(2); +});