mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(docker-cleanup): fail executions orphaned by a killed worker
Closes #11643
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user