diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 45ab1dde8..53ee272bb 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -111,34 +111,48 @@ private function processScheduledTasks(Collection $servers): void private function processServerTasks(Server $server): void { + // Get server timezone (used for all scheduled tasks) + $serverTimezone = data_get($server->settings, 'server_timezone', $this->instanceTimezone); + if (validate_timezone($serverTimezone) === false) { + $serverTimezone = config('app.timezone'); + } + // Check if we should run sentinel-based checks $lastSentinelUpdate = $server->sentinel_updated_at; $waitTime = $server->waitBeforeDoingSshCheck(); - $sentinelOutOfSync = Carbon::parse($lastSentinelUpdate)->isBefore($this->executionTime->subSeconds($waitTime)); + $sentinelOutOfSync = Carbon::parse($lastSentinelUpdate)->isBefore($this->executionTime->copy()->subSeconds($waitTime)); if ($sentinelOutOfSync) { - // Dispatch jobs if Sentinel is out of sync - if ($this->shouldRunNow($this->checkFrequency)) { + // Dispatch ServerCheckJob if Sentinel is out of sync + if ($this->shouldRunNow($this->checkFrequency, $serverTimezone)) { ServerCheckJob::dispatch($server); } + } - // Dispatch ServerStorageCheckJob if due - $serverDiskUsageCheckFrequency = data_get($server->settings, 'server_disk_usage_check_frequency', '0 * * * *'); + $isSentinelEnabled = $server->isSentinelEnabled(); + $shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone); + // Dispatch Sentinel restart if due (daily for Sentinel-enabled servers) + + if ($shouldRestartSentinel) { + dispatch(function () use ($server) { + $server->restartContainer('coolify-sentinel'); + }); + } + + // Dispatch ServerStorageCheckJob if due (only when Sentinel is out of sync or disabled) + // When Sentinel is active, PushServerUpdateJob handles storage checks with real-time data + if ($sentinelOutOfSync) { + $serverDiskUsageCheckFrequency = data_get($server->settings, 'server_disk_usage_check_frequency', '0 23 * * *'); if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) { $serverDiskUsageCheckFrequency = VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency]; } - $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency); + $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency, $serverTimezone); if ($shouldRunStorageCheck) { ServerStorageCheckJob::dispatch($server); } } - $serverTimezone = data_get($server->settings, 'server_timezone', $this->instanceTimezone); - if (validate_timezone($serverTimezone) === false) { - $serverTimezone = config('app.timezone'); - } - // Dispatch ServerPatchCheckJob if due (weekly) $shouldRunPatchCheck = $this->shouldRunNow('0 0 * * 0', $serverTimezone); @@ -146,15 +160,6 @@ private function processServerTasks(Server $server): void ServerPatchCheckJob::dispatch($server); } - // Dispatch Sentinel restart if due (daily for Sentinel-enabled servers) - $isSentinelEnabled = $server->isSentinelEnabled(); - $shouldRestartSentinel = $isSentinelEnabled && $this->shouldRunNow('0 0 * * *', $serverTimezone); - - if ($shouldRestartSentinel) { - dispatch(function () use ($server) { - $server->restartContainer('coolify-sentinel'); - }); - } } private function shouldRunNow(string $frequency, ?string $timezone = null): bool diff --git a/tests/Feature/ServerStorageCheckIndependenceTest.php b/tests/Feature/ServerStorageCheckIndependenceTest.php new file mode 100644 index 000000000..57b392e2f --- /dev/null +++ b/tests/Feature/ServerStorageCheckIndependenceTest.php @@ -0,0 +1,186 @@ +create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now(), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 23 * * *', + 'server_timezone' => 'UTC', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should NOT be dispatched (Sentinel handles it via PushServerUpdateJob) + Queue::assertNotPushed(ServerStorageCheckJob::class); +}); + +it('dispatches storage check when sentinel is out of sync', function () { + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); + + // Given: A server with Sentinel out of sync (last update 10 minutes ago) + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now()->subMinutes(10), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 23 * * *', + 'server_timezone' => 'UTC', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: Both ServerCheckJob and ServerStorageCheckJob should be dispatched + Queue::assertPushed(ServerCheckJob::class); + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('dispatches storage check when sentinel is disabled', function () { + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); + + // Given: A server with Sentinel disabled + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now()->subHours(24), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 23 * * *', + 'server_timezone' => 'UTC', + 'is_metrics_enabled' => false, + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should be dispatched + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('respects custom hourly storage check frequency when sentinel is out of sync', function () { + // When: ServerManagerJob runs at the top of the hour (23:00) + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); + + // Given: A server with hourly storage check frequency and Sentinel out of sync + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now()->subMinutes(10), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 * * * *', + 'server_timezone' => 'UTC', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should be dispatched + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('handles VALID_CRON_STRINGS mapping correctly when sentinel is out of sync', function () { + // When: ServerManagerJob runs at the top of the hour + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); + + // Given: A server with 'hourly' string (should be converted to '0 * * * *') and Sentinel out of sync + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now()->subMinutes(10), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => 'hourly', + 'server_timezone' => 'UTC', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should be dispatched (hourly was converted to cron) + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('respects server timezone for storage checks when sentinel is out of sync', function () { + // When: ServerManagerJob runs at 11 PM New York time (4 AM UTC next day) + Carbon::setTestNow(Carbon::parse('2025-01-16 04:00:00', 'UTC')); + + // Given: A server in America/New_York timezone (UTC-5) configured for 11 PM local time and Sentinel out of sync + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now()->subMinutes(10), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 23 * * *', + 'server_timezone' => 'America/New_York', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should be dispatched + Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +}); + +it('does not dispatch storage check outside schedule', function () { + // When: ServerManagerJob runs at 10 PM (not 11 PM) + Carbon::setTestNow(Carbon::parse('2025-01-15 22:00:00', 'UTC')); + + // Given: A server with daily storage check at 11 PM + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now(), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => '0 23 * * *', + 'server_timezone' => 'UTC', + ]); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should NOT be dispatched + Queue::assertNotPushed(ServerStorageCheckJob::class); +}); diff --git a/tests/Unit/ServerManagerJobExecutionTimeTest.php b/tests/Unit/ServerManagerJobExecutionTimeTest.php new file mode 100644 index 000000000..56a3af7d2 --- /dev/null +++ b/tests/Unit/ServerManagerJobExecutionTimeTest.php @@ -0,0 +1,121 @@ +toDateTimeString(); + + // Simulate what happens in processServerTasks() with the FIX applied + $waitTime = 360; + $threshold = $originalTime->copy()->subSeconds($waitTime); + + // The original time should remain unchanged after using copy() + expect($originalTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold->toDateTimeString())->toBe('2024-12-02 11:54:00'); +}); + +it('demonstrates mutation bug when not using copy()', function () { + // This test shows what would happen WITHOUT the fix (the bug) + $originalTime = Carbon::parse('2024-12-02 12:00:00'); + + // Simulate what would happen WITHOUT copy() (the bug) + $waitTime = 360; + $threshold = $originalTime->subSeconds($waitTime); + + // Without copy(), the original time is mutated! + expect($originalTime->toDateTimeString())->toBe('2024-12-02 11:54:00'); + expect($threshold->toDateTimeString())->toBe('2024-12-02 11:54:00'); + expect($originalTime)->toBe($threshold); // They're the same object +}); + +it('preserves executionTime across multiple subSeconds calls with copy()', function () { + // Simulate processing multiple servers with different wait times + $executionTime = Carbon::parse('2024-12-02 12:00:00'); + $originalTimeString = $executionTime->toDateTimeString(); + + // Server 1: waitTime = 360s + $threshold1 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold1->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // Server 2: waitTime = 300s (should still use original time) + $threshold2 = $executionTime->copy()->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold2->toDateTimeString())->toBe('2024-12-02 11:55:00'); + + // Server 3: waitTime = 360s (should still use original time) + $threshold3 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold3->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // Server 4: waitTime = 300s (should still use original time) + $threshold4 = $executionTime->copy()->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold4->toDateTimeString())->toBe('2024-12-02 11:55:00'); + + // Server 5: waitTime = 360s (should still use original time) + $threshold5 = $executionTime->copy()->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe($originalTimeString); + expect($threshold5->toDateTimeString())->toBe('2024-12-02 11:54:00'); + + // The executionTime should STILL be exactly the original time + expect($executionTime->toDateTimeString())->toBe('2024-12-02 12:00:00'); +}); + +it('demonstrates compounding bug without copy() across multiple calls', function () { + // This shows the compounding bug that happens WITHOUT the fix + $executionTime = Carbon::parse('2024-12-02 12:00:00'); + + // Server 1: waitTime = 360s + $threshold1 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:54:00'); // MUTATED! + + // Server 2: waitTime = 300s (uses already-mutated time) + $threshold2 = $executionTime->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:49:00'); // Further mutated! + + // Server 3: waitTime = 360s (uses even more mutated time) + $threshold3 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:43:00'); // Even more mutated! + + // Server 4: waitTime = 300s + $threshold4 = $executionTime->subSeconds(300); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:38:00'); + + // Server 5: waitTime = 360s + $threshold5 = $executionTime->subSeconds(360); + expect($executionTime->toDateTimeString())->toBe('2024-12-02 11:32:00'); + + // The executionTime is now 1680 seconds (28 minutes) earlier than it should be! + expect($executionTime->diffInSeconds(Carbon::parse('2024-12-02 12:00:00')))->toEqual(1680); +}); + +it('respects server timezone when evaluating cron schedules', function () { + // This test verifies that timezone parameter affects cron evaluation + // Set a fixed test time at 23:00 UTC + Carbon::setTestNow('2024-12-02 23:00:00', 'UTC'); + + $executionTime = Carbon::now(); + $cronExpression = new \Cron\CronExpression('0 23 * * *'); // Every day at 11 PM + + // Test 1: UTC timezone at 23:00 - should match + $timeInUTC = $executionTime->copy()->setTimezone('UTC'); + expect($cronExpression->isDue($timeInUTC))->toBeTrue(); + + // Test 2: America/New_York timezone - 23:00 UTC is 18:00 EST, should not match 23:00 cron + $timeInEST = $executionTime->copy()->setTimezone('America/New_York'); + expect($cronExpression->isDue($timeInEST))->toBeFalse(); + + // Test 3: Asia/Tokyo timezone - 23:00 UTC is 08:00 JST next day, should not match 23:00 cron + $timeInJST = $executionTime->copy()->setTimezone('Asia/Tokyo'); + expect($cronExpression->isDue($timeInJST))->toBeFalse(); + + // Test 4: Verify copy() preserves the original time + expect($executionTime->toDateTimeString())->toBe('2024-12-02 23:00:00'); +});