diff --git a/app/Jobs/DockerCleanupJob.php b/app/Jobs/DockerCleanupJob.php index 78ef7f3a2..a8a3cb159 100644 --- a/app/Jobs/DockerCleanupJob.php +++ b/app/Jobs/DockerCleanupJob.php @@ -39,7 +39,9 @@ public function __construct( public bool $manualCleanup = false, public bool $deleteUnusedVolumes = false, public bool $deleteUnusedNetworks = false - ) {} + ) { + $this->onQueue('high'); + } public function handle(): void { diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index e68e3b613..ebcd229ed 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -350,7 +350,7 @@ private function shouldRunNow(string $frequency, string $timezone, ?string $dedu $baseTime = $this->executionTime ?? Carbon::now(); $executionTime = $baseTime->copy()->setTimezone($timezone); - // No dedup key → simple isDue check (used by docker cleanups) + // No dedup key → simple isDue check if ($dedupKey === null) { return $cron->isDue($executionTime); } @@ -411,7 +411,7 @@ private function processDockerCleanups(): void } // Use the frozen execution time for consistent evaluation - if ($this->shouldRunNow($frequency, $serverTimezone)) { + if ($this->shouldRunNow($frequency, $serverTimezone, "docker-cleanup:{$server->id}")) { DockerCleanupJob::dispatch( $server, false, diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 730ce547d..d56ff0a8c 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -14,6 +14,7 @@ use Illuminate\Queue\SerializesModels; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue @@ -80,7 +81,7 @@ private function getServers(): Collection private function dispatchConnectionChecks(Collection $servers): void { - if ($this->shouldRunNow($this->checkFrequency)) { + if ($this->shouldRunNow($this->checkFrequency, dedupKey: 'server-connection-checks')) { $servers->each(function (Server $server) { try { // Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity @@ -129,13 +130,13 @@ private function processServerTasks(Server $server): void if ($sentinelOutOfSync) { // Dispatch ServerCheckJob if Sentinel is out of sync - if ($this->shouldRunNow($this->checkFrequency, $serverTimezone)) { + if ($this->shouldRunNow($this->checkFrequency, $serverTimezone, "server-check:{$server->id}")) { ServerCheckJob::dispatch($server); } } $isSentinelEnabled = $server->isSentinelEnabled(); - $shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone); + $shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone, "sentinel-restart:{$server->id}"); // Dispatch Sentinel restart if due (daily for Sentinel-enabled servers) if ($shouldRestartSentinel) { @@ -149,7 +150,7 @@ private function processServerTasks(Server $server): void if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) { $serverDiskUsageCheckFrequency = VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency]; } - $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone); + $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone, "server-storage-check:{$server->id}"); if ($shouldRunStorageCheck) { ServerStorageCheckJob::dispatch($server); @@ -157,7 +158,7 @@ private function processServerTasks(Server $server): void } // Dispatch ServerPatchCheckJob if due (weekly) - $shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone); + $shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone, "server-patch-check:{$server->id}"); if ($shouldRunPatchCheck) { // Weekly on Sunday at midnight ServerPatchCheckJob::dispatch($server); @@ -167,7 +168,14 @@ private function processServerTasks(Server $server): void // Crash recovery is handled by sentinelOutOfSync → ServerCheckJob → CheckAndStartSentinelJob. } - private function shouldRunNow(string $frequency, ?string $timezone = null): bool + /** + * Determine if a cron schedule should run now. + * + * When a dedupKey is provided, uses getPreviousRunDate() + last-dispatch tracking + * instead of isDue(). This is resilient to queue delays — even if the job is delayed + * by minutes, it still catches the missed cron window. + */ + private function shouldRunNow(string $frequency, ?string $timezone = null, ?string $dedupKey = null): bool { $cron = new CronExpression($frequency); @@ -175,6 +183,29 @@ private function shouldRunNow(string $frequency, ?string $timezone = null): bool $baseTime = $this->executionTime ?? Carbon::now(); $executionTime = $baseTime->copy()->setTimezone($timezone ?? config('app.timezone')); - return $cron->isDue($executionTime); + if ($dedupKey === null) { + return $cron->isDue($executionTime); + } + + $previousDue = Carbon::instance($cron->getPreviousRunDate($executionTime, allowCurrentDate: true)); + + $lastDispatched = Cache::get($dedupKey); + + if ($lastDispatched === null) { + $isDue = $cron->isDue($executionTime); + if ($isDue) { + Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); + } + + return $isDue; + } + + if ($previousDue->gt(Carbon::parse($lastDispatched))) { + Cache::put($dedupKey, $executionTime->toIso8601String(), 86400); + + return true; + } + + return false; } } diff --git a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php index f820c3777..e445e9908 100644 --- a/tests/Feature/ScheduledJobManagerShouldRunNowTest.php +++ b/tests/Feature/ScheduledJobManagerShouldRunNowTest.php @@ -194,6 +194,55 @@ expect($result2)->toBeFalse(); }); +it('catches delayed docker cleanup when job runs past the cron minute', function () { + // Simulate a previous dispatch at :10 + Cache::put('docker-cleanup:42', Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC')->toIso8601String(), 86400); + + // Freeze time at :22 — job was delayed 2 minutes past the :20 cron window + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 22, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // isDue() would return false at :22, but getPreviousRunDate() = :20 + // lastDispatched = :10 → :20 > :10 → fires + $result = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:42'); + + expect($result)->toBeTrue(); +}); + +it('does not double-dispatch docker cleanup within same cron window', function () { + // First dispatch at :10 + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 10, 0, 'UTC')); + + $job = new ScheduledJobManager; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $first = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99'); + expect($first)->toBeTrue(); + + // Second run at :11 — should NOT dispatch (previousDue=:10, lastDispatched=:10) + Carbon::setTestNow(Carbon::create(2026, 2, 28, 10, 11, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + + $second = $method->invoke($job, '*/10 * * * *', 'UTC', 'docker-cleanup:99'); + expect($second)->toBeFalse(); +}); + it('respects server timezone for cron evaluation', function () { // UTC time is 22:00 Feb 28, which is 06:00 Mar 1 in Asia/Singapore (+8) Carbon::setTestNow(Carbon::create(2026, 2, 28, 22, 0, 0, 'UTC')); diff --git a/tests/Feature/ServerManagerJobShouldRunNowTest.php b/tests/Feature/ServerManagerJobShouldRunNowTest.php new file mode 100644 index 000000000..518f05c9c --- /dev/null +++ b/tests/Feature/ServerManagerJobShouldRunNowTest.php @@ -0,0 +1,102 @@ +toIso8601String(), 86400); + + // Job runs 3 minutes late at 00:03 + Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 3, 0, 'UTC')); + + $job = new ServerManagerJob; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + // isDue() would return false at 00:03, but getPreviousRunDate() = 00:00 today + // lastDispatched = yesterday 00:00 → today 00:00 > yesterday → fires + $result = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:1'); + + expect($result)->toBeTrue(); +}); + +it('catches delayed weekly patch check when job runs past the cron minute', function () { + // Simulate previous dispatch last Sunday at midnight + Cache::put('server-patch-check:1', Carbon::create(2026, 2, 22, 0, 0, 0, 'UTC')->toIso8601String(), 86400); + + // This Sunday at 00:02 — job was delayed 2 minutes + // 2026-03-01 is a Sunday + Carbon::setTestNow(Carbon::create(2026, 3, 1, 0, 2, 0, 'UTC')); + + $job = new ServerManagerJob; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $result = $method->invoke($job, '0 0 * * 0', 'UTC', 'server-patch-check:1'); + + expect($result)->toBeTrue(); +}); + +it('catches delayed storage check when job runs past the cron minute', function () { + // Simulate previous dispatch yesterday at 23:00 + Cache::put('server-storage-check:5', Carbon::create(2026, 2, 27, 23, 0, 0, 'UTC')->toIso8601String(), 86400); + + // Today at 23:04 — job was delayed 4 minutes + Carbon::setTestNow(Carbon::create(2026, 2, 28, 23, 4, 0, 'UTC')); + + $job = new ServerManagerJob; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $result = $method->invoke($job, '0 23 * * *', 'UTC', 'server-storage-check:5'); + + expect($result)->toBeTrue(); +}); + +it('does not double-dispatch within same cron window', function () { + Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 0, 0, 'UTC')); + + $job = new ServerManagerJob; + $reflection = new ReflectionClass($job); + + $executionTimeProp = $reflection->getProperty('executionTime'); + $executionTimeProp->setAccessible(true); + $executionTimeProp->setValue($job, Carbon::now()); + + $method = $reflection->getMethod('shouldRunNow'); + $method->setAccessible(true); + + $first = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10'); + expect($first)->toBeTrue(); + + // Next minute — should NOT dispatch again + Carbon::setTestNow(Carbon::create(2026, 2, 28, 0, 1, 0, 'UTC')); + $executionTimeProp->setValue($job, Carbon::now()); + + $second = $method->invoke($job, '0 0 * * *', 'UTC', 'sentinel-restart:10'); + expect($second)->toBeFalse(); +});