From f1f7d5269e4d61cea167f7c3874c6b34f1532037 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:17:48 +0200 Subject: [PATCH] fix(docker-cleanup): fail executions orphaned by a killed worker Closes #11643 --- app/Jobs/DockerCleanupJob.php | 36 ++++++++++++++++++++++++++ app/Jobs/ScheduledJobManager.php | 1 + tests/Feature/DockerCleanupJobTest.php | 29 +++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/app/Jobs/DockerCleanupJob.php b/app/Jobs/DockerCleanupJob.php index 1a338f093f..f491f80030 100644 --- a/app/Jobs/DockerCleanupJob.php +++ b/app/Jobs/DockerCleanupJob.php @@ -41,6 +41,8 @@ class DockerCleanupJob implements ShouldBeEncrypted, ShouldQueue public ?DockerCleanupExecution $execution_log = null; + public const INTERRUPTED_MESSAGE = 'Docker cleanup was interrupted: its worker stopped before the cleanup finished.'; + /** * Shares the per-server lock with queued CleanupDocker runs, so two cleanups never run on * one server at once. A scheduled run that finds the lock held marks its occurrence @@ -231,6 +233,40 @@ class DockerCleanupJob implements ShouldBeEncrypted, ShouldQueue return $jobUuid ? 'docker-cleanup-execution:'.$jobUuid : null; } + /** + * Fails running executions that are older than the server lock. The worker kills a cleanup at the + * job timeout, which is shorter than the lock, so such a run lost its worker before failed() ran + * (for example a killed container) and would otherwise stay running. + */ + public static function failInterruptedExecutions(): void + { + $executions = DockerCleanupExecution::query() + ->with('server.team') + ->where('status', 'running') + ->whereNull('finished_at') + ->where('created_at', '<', now()->subSeconds(CleanupDocker::overlapLockExpiresAfter())) + ->get(); + + foreach ($executions as $execution) { + $updated = DockerCleanupExecution::query() + ->whereKey($execution->id) + ->where('status', 'running') + ->whereNull('finished_at') + ->update([ + 'status' => 'failed', + 'message' => self::INTERRUPTED_MESSAGE, + 'finished_at' => Carbon::now()->toImmutable(), + ]); + + if ($updated === 0) { + continue; + } + + event(new DockerCleanupDone($execution->refresh())); + $execution->server?->team?->notify(new DockerCleanupFailed($execution->server, 'Docker cleanup job failed with the following error: '.self::INTERRUPTED_MESSAGE)); + } + } + public function failed(?\Throwable $exception): void { if ($this->occurrenceUuid) { diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index c218a53652..7e137dadaa 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -191,6 +191,7 @@ class ScheduledJobManager implements ShouldQueue // Process Docker cleanups - don't let failures stop the job manager try { if ($this->includes('docker-cleanups')) { + DockerCleanupJob::failInterruptedExecutions(); $this->processDockerCleanups(); } } catch (\Throwable $e) { diff --git a/tests/Feature/DockerCleanupJobTest.php b/tests/Feature/DockerCleanupJobTest.php index 6b4780fc2b..c479fa21ea 100644 --- a/tests/Feature/DockerCleanupJobTest.php +++ b/tests/Feature/DockerCleanupJobTest.php @@ -3,6 +3,7 @@ use App\Actions\Server\CleanupDocker; use App\Events\DockerCleanupDone; use App\Jobs\DockerCleanupJob; +use App\Jobs\ScheduledJobManager; use App\Models\Application; use App\Models\DockerCleanupExecution; use App\Models\Environment; @@ -196,6 +197,34 @@ it('leaves a running scheduled cleanup untouched when a manual cleanup fails bef Event::assertNotDispatched(DockerCleanupDone::class); }); +it('fails a cleanup execution whose worker was killed once the server lock has expired, so it does not stay running', function () { + Carbon::setTestNow(Carbon::create(2026, 10, 16, 12, 0, 0, 'UTC')); + Queue::fake(); + Notification::fake(); + Event::fake([DockerCleanupDone::class]); + $server = dockerCleanupReachableServer(); + $server->team->emailNotificationSettings->update(['smtp_enabled' => true, 'docker_cleanup_failure_email_notifications' => true]); + $interrupted = DockerCleanupExecution::create(['server_id' => $server->id]); + Carbon::setTestNow(now()->addSeconds(CleanupDocker::overlapLockExpiresAfter() - 1)); + $live = DockerCleanupExecution::create(['server_id' => $server->id]); + $finished = DockerCleanupExecution::create(['server_id' => $server->id, 'status' => 'success', 'finished_at' => now()]); + + (new ScheduledJobManager('docker-cleanups'))->handle(); + + expect($interrupted->fresh()->status)->toBe('running'); + + Carbon::setTestNow(now()->addSeconds(2)); + (new ScheduledJobManager('docker-cleanups'))->handle(); + (new ScheduledJobManager('docker-cleanups'))->handle(); + + expect($interrupted->fresh()->only(['status', 'message']))->toBe(['status' => 'failed', 'message' => DockerCleanupJob::INTERRUPTED_MESSAGE]) + ->and($interrupted->fresh()->finished_at)->not->toBeNull() + ->and($live->fresh()->only(['status', 'finished_at']))->toBe(['status' => 'running', 'finished_at' => null]) + ->and($finished->fresh()->status)->toBe('success'); + Notification::assertSentToTimes($server->team, DockerCleanupFailed::class, 1); + Event::assertDispatchedTimes(DockerCleanupDone::class, 1); +}); + it('skips a scheduled cleanup occurrence while another cleanup holds the server lock, so it is not reported missed later', function () { Carbon::setTestNow(Carbon::create(2026, 9, 17, 0, 0, 0, 'UTC')); Notification::fake();