From b47181c790010971ac78c3603a60b662006bf81f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Dec 2025 13:36:25 +0100 Subject: [PATCH 1/6] Decouple ServerStorageCheckJob from Sentinel sync status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server disk usage checks now run on their configured schedule regardless of Sentinel status, eliminating monitoring blind spots when Sentinel is offline, out of sync, or disabled. Storage checks now respect server timezone settings, consistent with patch checks. Changes: - Moved server timezone calculation to top of processServerTasks() - Extracted ServerStorageCheckJob dispatch from Sentinel conditional - Fixed default frequency to '0 23 * * *' (11 PM daily) - Added timezone parameter to storage check scheduling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Jobs/ServerManagerJob.php | 31 +-- .../ServerStorageCheckIndependenceTest.php | 188 ++++++++++++++++++ 2 files changed, 204 insertions(+), 15 deletions(-) create mode 100644 tests/Feature/ServerStorageCheckIndependenceTest.php diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 45ab1dde8..a618647eb 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -111,32 +111,33 @@ 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)); if ($sentinelOutOfSync) { - // Dispatch jobs if Sentinel is out of sync + // Dispatch ServerCheckJob if Sentinel is out of sync if ($this->shouldRunNow($this->checkFrequency)) { ServerCheckJob::dispatch($server); } - - // Dispatch ServerStorageCheckJob if due - $serverDiskUsageCheckFrequency = data_get($server->settings, 'server_disk_usage_check_frequency', '0 * * * *'); - if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) { - $serverDiskUsageCheckFrequency = VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency]; - } - $shouldRunStorageCheck = $this->shouldRunNow($serverDiskUsageCheckFrequency); - - if ($shouldRunStorageCheck) { - ServerStorageCheckJob::dispatch($server); - } } - $serverTimezone = data_get($server->settings, 'server_timezone', $this->instanceTimezone); - if (validate_timezone($serverTimezone) === false) { - $serverTimezone = config('app.timezone'); + // Dispatch ServerStorageCheckJob if due (independent of Sentinel status) + $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, $serverTimezone); + + if ($shouldRunStorageCheck) { + ServerStorageCheckJob::dispatch($server); } // Dispatch ServerPatchCheckJob if due (weekly) diff --git a/tests/Feature/ServerStorageCheckIndependenceTest.php b/tests/Feature/ServerStorageCheckIndependenceTest.php new file mode 100644 index 000000000..a6b18469d --- /dev/null +++ b/tests/Feature/ServerStorageCheckIndependenceTest.php @@ -0,0 +1,188 @@ +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', + ]); + + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', '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('dispatches storage check when sentinel is out of sync', function () { + // 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', + ]); + + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', '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 () { + // 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, + ]); + + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', '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('respects custom hourly storage check frequency', function () { + // Given: A server with hourly storage check frequency + $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 * * * *', + 'server_timezone' => 'UTC', + ]); + + // When: ServerManagerJob runs at the top of the hour (23:00) + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', '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', function () { + // Given: A server with 'hourly' string (should be converted to '0 * * * *') + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'sentinel_updated_at' => now(), + ]); + + $server->settings->update([ + 'server_disk_usage_check_frequency' => 'hourly', + 'server_timezone' => 'UTC', + ]); + + // When: ServerManagerJob runs at the top of the hour + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', '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', function () { + // Given: A server in America/New_York timezone (UTC-5) configured for 11 PM local time + $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' => 'America/New_York', + ]); + + // 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')); + $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 () { + // 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', + ]); + + // When: ServerManagerJob runs at 10 PM (not 11 PM) + Carbon::setTestNow(Carbon::parse('2025-01-15 22:00:00', 'UTC')); + $job = new ServerManagerJob; + $job->handle(); + + // Then: ServerStorageCheckJob should NOT be dispatched + Queue::assertNotPushed(ServerStorageCheckJob::class); +}); From ed5796739f277c07706fff3a8dadc708260750d7 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Dec 2025 15:27:17 +0100 Subject: [PATCH 2/6] Fix: Prevent ServerManagerJob executionTime mutation across server loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed a critical bug where $this->executionTime was being mutated during the server processing loop, causing incorrect scheduling calculations for subsequent servers. The issue occurred at line 123 where subSeconds() was called directly on the shared executionTime instance. This caused the baseline time to shift by waitTime seconds with each server iteration, resulting in compounding scheduling errors (e.g., 1680 seconds drift over 5 servers). Changed: - app/Jobs/ServerManagerJob.php:123 Added .copy() before .subSeconds() to prevent mutation Added comprehensive unit tests that verify: - Immutability when using .copy() - Demonstration of the bug without .copy() - Correct behavior across multiple iterations This follows the existing pattern in shouldRunNow() (line 167) and aligns with other jobs in the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Jobs/ServerManagerJob.php | 2 +- .../ServerManagerJobExecutionTimeTest.php | 97 +++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/ServerManagerJobExecutionTimeTest.php diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index a618647eb..61266473b 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -120,7 +120,7 @@ private function processServerTasks(Server $server): void // 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 ServerCheckJob if Sentinel is out of sync diff --git a/tests/Unit/ServerManagerJobExecutionTimeTest.php b/tests/Unit/ServerManagerJobExecutionTimeTest.php new file mode 100644 index 000000000..d32db6834 --- /dev/null +++ b/tests/Unit/ServerManagerJobExecutionTimeTest.php @@ -0,0 +1,97 @@ +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); +}); From 8ff83cc3d670993274eb52fbf2c5016fb5c2acd4 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 2 Dec 2025 16:58:43 +0100 Subject: [PATCH 3/6] Fix: Pass $serverTimezone to shouldRunNow() in ServerCheckJob dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass the server timezone parameter to shouldRunNow() call at line 127, ensuring ServerCheckJob dispatch respects the server's local timezone instead of falling back to the instance default. This aligns the behavior with other scheduled tasks in the same method: - ServerStorageCheckJob (line 137) - ServerPatchCheckJob (line 144) - Sentinel restart (line 152) All scheduled tasks in processServerTasks() now consistently use the server's configured timezone for cron evaluation. Added unit test to verify timezone-aware cron schedule evaluation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Jobs/ServerManagerJob.php | 2 +- .../ServerManagerJobExecutionTimeTest.php | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 61266473b..87458e8f2 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -124,7 +124,7 @@ private function processServerTasks(Server $server): void if ($sentinelOutOfSync) { // Dispatch ServerCheckJob if Sentinel is out of sync - if ($this->shouldRunNow($this->checkFrequency)) { + if ($this->shouldRunNow($this->checkFrequency, $serverTimezone)) { ServerCheckJob::dispatch($server); } } diff --git a/tests/Unit/ServerManagerJobExecutionTimeTest.php b/tests/Unit/ServerManagerJobExecutionTimeTest.php index d32db6834..56a3af7d2 100644 --- a/tests/Unit/ServerManagerJobExecutionTimeTest.php +++ b/tests/Unit/ServerManagerJobExecutionTimeTest.php @@ -95,3 +95,27 @@ // 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'); +}); From c982d58eee8b953db34d26e56cbf11288283e9e6 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 3 Dec 2025 09:21:55 +0100 Subject: [PATCH 4/6] Refactor: Move Sentinel restart logic into processServerTasks method --- app/Jobs/ServerManagerJob.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 87458e8f2..4a1cb05a3 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -129,6 +129,16 @@ private function processServerTasks(Server $server): void } } + $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 (independent of Sentinel status) $serverDiskUsageCheckFrequency = data_get($server->settings, 'server_disk_usage_check_frequency', '0 23 * * *'); if (isset(VALID_CRON_STRINGS[$serverDiskUsageCheckFrequency])) { @@ -147,15 +157,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 From 56a0143a25af3d2a040753637987c08a65bb3f09 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 3 Dec 2025 10:05:10 +0100 Subject: [PATCH 5/6] Fix: Prevent ServerStorageCheckJob duplication when Sentinel is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Sentinel is enabled and in sync, ServerStorageCheckJob was being dispatched from two locations causing unnecessary duplication: 1. PushServerUpdateJob (every ~30s with real-time filesystem data) 2. ServerManagerJob (scheduled cron check via SSH) This commit modifies ServerManagerJob to only dispatch ServerStorageCheckJob when Sentinel is out of sync or disabled. When Sentinel is active and in sync, PushServerUpdateJob provides real-time storage data, making the scheduled SSH check redundant. Benefits: - Eliminates duplicate storage checks when Sentinel is working - Reduces unnecessary SSH overhead - Storage checks still run as fallback when Sentinel fails - Maintains scheduled checks for servers without Sentinel Updated tests to reflect new behavior: - Storage check NOT dispatched when Sentinel is in sync - Storage check dispatched when Sentinel is out of sync or disabled - All timezone and frequency tests updated accordingly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Jobs/ServerManagerJob.php | 19 ++++++++------ .../ServerStorageCheckIndependenceTest.php | 26 +++++++++---------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 4a1cb05a3..53ee272bb 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -139,15 +139,18 @@ private function processServerTasks(Server $server): void }); } - // Dispatch ServerStorageCheckJob if due (independent of Sentinel status) - $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, $serverTimezone); + // 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, $serverTimezone); - if ($shouldRunStorageCheck) { - ServerStorageCheckJob::dispatch($server); + if ($shouldRunStorageCheck) { + ServerStorageCheckJob::dispatch($server); + } } // Dispatch ServerPatchCheckJob if due (weekly) diff --git a/tests/Feature/ServerStorageCheckIndependenceTest.php b/tests/Feature/ServerStorageCheckIndependenceTest.php index a6b18469d..d5b8b79f6 100644 --- a/tests/Feature/ServerStorageCheckIndependenceTest.php +++ b/tests/Feature/ServerStorageCheckIndependenceTest.php @@ -19,7 +19,7 @@ Carbon::setTestNow(); }); -it('dispatches storage check when sentinel is in sync', function () { +it('does not dispatch storage check when sentinel is in sync', function () { // Given: A server with Sentinel recently updated (in sync) $team = Team::factory()->create(); $server = Server::factory()->create([ @@ -37,10 +37,8 @@ $job = new ServerManagerJob; $job->handle(); - // Then: ServerStorageCheckJob should be dispatched - Queue::assertPushed(ServerStorageCheckJob::class, function ($job) use ($server) { - return $job->server->id === $server->id; - }); + // 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 () { @@ -93,12 +91,12 @@ }); }); -it('respects custom hourly storage check frequency', function () { - // Given: A server with hourly storage check frequency +it('respects custom hourly storage check frequency when sentinel is out of sync', function () { + // 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(), + 'sentinel_updated_at' => now()->subMinutes(10), ]); $server->settings->update([ @@ -117,12 +115,12 @@ }); }); -it('handles VALID_CRON_STRINGS mapping correctly', function () { - // Given: A server with 'hourly' string (should be converted to '0 * * * *') +it('handles VALID_CRON_STRINGS mapping correctly when sentinel is out of sync', function () { + // 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(), + 'sentinel_updated_at' => now()->subMinutes(10), ]); $server->settings->update([ @@ -141,12 +139,12 @@ }); }); -it('respects server timezone for storage checks', function () { - // Given: A server in America/New_York timezone (UTC-5) configured for 11 PM local time +it('respects server timezone for storage checks when sentinel is out of sync', function () { + // 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(), + 'sentinel_updated_at' => now()->subMinutes(10), ]); $server->settings->update([ From 74bb8f49cef67c21445e8e8f0cc8038a0254f99a Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 3 Dec 2025 10:22:09 +0100 Subject: [PATCH 6/6] Fix: Correct time inconsistency in ServerStorageCheckIndependenceTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move Carbon::setTestNow() to the beginning of each test before creating test data. Previously, tests created servers using now() (real current time) and only afterwards called Carbon::setTestNow(), making sentinel_updated_at inconsistent with the test clock. This caused staleness calculations to use different timelines: - sentinel_updated_at was based on real time (e.g., Dec 2024) - Test execution time was frozen at 2025-01-15 Now all timestamps use the same frozen test time, making staleness checks predictable and tests reliable regardless of when they run. Affected tests (all 7 test cases in the file): - does not dispatch storage check when sentinel is in sync - dispatches storage check when sentinel is out of sync - dispatches storage check when sentinel is disabled - respects custom hourly storage check frequency when sentinel is out of sync - handles VALID_CRON_STRINGS mapping correctly when sentinel is out of sync - respects server timezone for storage checks when sentinel is out of sync - does not dispatch storage check outside schedule 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../ServerStorageCheckIndependenceTest.php | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/Feature/ServerStorageCheckIndependenceTest.php b/tests/Feature/ServerStorageCheckIndependenceTest.php index d5b8b79f6..57b392e2f 100644 --- a/tests/Feature/ServerStorageCheckIndependenceTest.php +++ b/tests/Feature/ServerStorageCheckIndependenceTest.php @@ -20,6 +20,9 @@ }); it('does not dispatch storage check when sentinel is in sync', function () { + // When: ServerManagerJob runs at 11 PM + Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); + // Given: A server with Sentinel recently updated (in sync) $team = Team::factory()->create(); $server = Server::factory()->create([ @@ -31,9 +34,6 @@ 'server_disk_usage_check_frequency' => '0 23 * * *', 'server_timezone' => 'UTC', ]); - - // When: ServerManagerJob runs at 11 PM - Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle(); @@ -42,6 +42,9 @@ }); 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([ @@ -53,9 +56,6 @@ 'server_disk_usage_check_frequency' => '0 23 * * *', 'server_timezone' => 'UTC', ]); - - // When: ServerManagerJob runs at 11 PM - Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle(); @@ -67,6 +67,9 @@ }); 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([ @@ -79,9 +82,6 @@ 'server_timezone' => 'UTC', 'is_metrics_enabled' => false, ]); - - // When: ServerManagerJob runs at 11 PM - Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle(); @@ -92,6 +92,9 @@ }); 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([ @@ -103,9 +106,6 @@ 'server_disk_usage_check_frequency' => '0 * * * *', 'server_timezone' => 'UTC', ]); - - // When: ServerManagerJob runs at the top of the hour (23:00) - Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle(); @@ -116,6 +116,9 @@ }); 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([ @@ -127,9 +130,6 @@ 'server_disk_usage_check_frequency' => 'hourly', 'server_timezone' => 'UTC', ]); - - // When: ServerManagerJob runs at the top of the hour - Carbon::setTestNow(Carbon::parse('2025-01-15 23:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle(); @@ -140,6 +140,9 @@ }); 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([ @@ -151,9 +154,6 @@ 'server_disk_usage_check_frequency' => '0 23 * * *', 'server_timezone' => 'America/New_York', ]); - - // 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')); $job = new ServerManagerJob; $job->handle(); @@ -164,6 +164,9 @@ }); 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([ @@ -175,9 +178,6 @@ 'server_disk_usage_check_frequency' => '0 23 * * *', 'server_timezone' => 'UTC', ]); - - // When: ServerManagerJob runs at 10 PM (not 11 PM) - Carbon::setTestNow(Carbon::parse('2025-01-15 22:00:00', 'UTC')); $job = new ServerManagerJob; $job->handle();