rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Decouple ServerStorageCheckJob from Sentinel sync (#7454)

Browse source
This commit is contained in:
Andras Bacsai 2025-12-03 10:29:10 +01:00 committed by GitHub
commit b55aaf34d3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 332 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
app/Jobs/ServerManagerJob.php
View file

@ -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

186
tests/Feature/ServerStorageCheckIndependenceTest.php Normal file
View file

@ -0,0 +1,186 @@
<?php
use App\Jobs\ServerCheckJob;
use App\Jobs\ServerManagerJob;
use App\Jobs\ServerStorageCheckJob;
use App\Models\Server;
use App\Models\Team;
use Carbon\Carbon;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Queue;
uses(RefreshDatabase::class);
beforeEach(function () {
Queue::fake();
});
afterEach(function () {
Carbon::setTestNow();
});
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([
'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);
});

121
tests/Unit/ServerManagerJobExecutionTimeTest.php Normal file
View file

@ -0,0 +1,121 @@
<?php
use Illuminate\Support\Carbon;
afterEach(function () {
Carbon::setTestNow();
});
it('does not mutate Carbon instance when using copy() before subSeconds()', function () {
// This test verifies the fix for the bug where subSeconds() was mutating executionTime
$originalTime = Carbon::parse('2024-12-02 12:00:00');
$originalTimeString = $originalTime->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');
});