diff --git a/app/Actions/Server/StartSentinel.php b/app/Actions/Server/StartSentinel.php index 101f6b2d75..f58c995c96 100644 --- a/app/Actions/Server/StartSentinel.php +++ b/app/Actions/Server/StartSentinel.php @@ -4,12 +4,25 @@ namespace App\Actions\Server; use App\Events\SentinelRestarted; use App\Models\Server; +use Illuminate\Contracts\Cache\LockTimeoutException; +use Illuminate\Support\Facades\Cache; use Lorisleiva\Actions\Concerns\AsAction; class StartSentinel { use AsAction; + /** Longer than one start, including the image pull. */ + public const LOCK_SECONDS = 300; + + /** Shorter than the 120 second timeout of CheckAndStartSentinelJob. */ + public const LOCK_WAIT_SECONDS = 90; + + public static function lockKey(Server $server): string + { + return 'sentinel-start:'.$server->uuid; + } + /** * Sentinel and the proxy both mount this host path, and Sentinel reads the access log at the same path. */ @@ -74,11 +87,27 @@ class StartSentinel ]; } - public function handle(Server $server, bool $restart = false, ?string $latestVersion = null, ?string $customImage = null) + /** + * @throws LockTimeoutException when another start for the server does not finish in time + */ + public function handle(Server $server, bool $restart = false, ?string $latestVersion = null, ?string $customImage = null): void { if ($server->isSwarm() || $server->isBuildServer()) { return; } + + // Two starts at the same time both remove the container, and then the second `docker run` + // fails because the first one already uses the coolify-sentinel name. + Cache::lock(self::lockKey($server), self::LOCK_SECONDS)->block( + self::LOCK_WAIT_SECONDS, + fn () => $this->start($server, $restart, $latestVersion, $customImage), + ); + } + + private function start(Server $server, bool $restart, ?string $latestVersion, ?string $customImage): void + { + // A start that waited for the lock must use the settings saved in the meantime. + $server->refresh(); if ($restart) { StopSentinel::run($server); } diff --git a/app/Livewire/Server/Charts.php b/app/Livewire/Server/Charts.php index d4cf1c2ad9..c77662315a 100644 --- a/app/Livewire/Server/Charts.php +++ b/app/Livewire/Server/Charts.php @@ -69,7 +69,8 @@ class Charts extends Component try { $this->authorize('update', $this->server); $this->server->settings->is_metrics_enabled = ! $this->server->settings->is_metrics_enabled; - $this->server->settings->save(); + // Saved quietly: Sentinel is started below, so the settings hook must not start it a second time. + $this->server->settings->saveQuietly(); $this->auditSentinelUpdate(['is_metrics_enabled']); $this->server->refresh(); diff --git a/app/Livewire/Server/Sentinel.php b/app/Livewire/Server/Sentinel.php index 0de1ff766e..22a6074866 100644 --- a/app/Livewire/Server/Sentinel.php +++ b/app/Livewire/Server/Sentinel.php @@ -188,12 +188,15 @@ class Sentinel extends Component try { $this->authorize('update', $this->server); $changedFields = $this->syncData(true); - if ($changedFields !== []) { - auditLog('ui.server.sentinel.updated', $this->auditContext([ - 'changed_fields' => $changedFields, - ])); + if ($changedFields === []) { + return; } - $this->restartSentinel(); + auditLog('ui.server.sentinel.updated', $this->auditContext([ + 'changed_fields' => $changedFields, + ])); + // Saving the setting restarts Sentinel (ServerSetting::booted()). + $this->setSentinelRestarting(); + $this->dispatch('info', 'Restarting Sentinel.'); } catch (\Throwable $e) { return handleError($e, $this); } diff --git a/app/Livewire/Server/Show.php b/app/Livewire/Server/Show.php index 77b31ce211..d7420b7d9c 100644 --- a/app/Livewire/Server/Show.php +++ b/app/Livewire/Server/Show.php @@ -453,8 +453,8 @@ class Show extends Component public function updatedIsSentinelDebugEnabled($value) { try { + // Saving the setting restarts Sentinel (ServerSetting::booted()). $this->submit(); - $this->restartSentinel(); } catch (\Throwable $e) { return handleError($e, $this); } @@ -463,8 +463,8 @@ class Show extends Component public function updatedIsMetricsEnabled($value) { try { + // Saving the setting restarts Sentinel (ServerSetting::booted()). $this->submit(); - $this->restartSentinel(); } catch (\Throwable $e) { return handleError($e, $this); } diff --git a/app/Models/ServerSetting.php b/app/Models/ServerSetting.php index a4d51afdae..fab0d406ec 100644 --- a/app/Models/ServerSetting.php +++ b/app/Models/ServerSetting.php @@ -207,6 +207,8 @@ class ServerSetting extends Model if ( $settings->wasChanged('sentinel_token') || $settings->wasChanged('sentinel_custom_url') || + $settings->wasChanged('is_metrics_enabled') || + $settings->wasChanged('is_sentinel_debug_enabled') || $settings->wasChanged('sentinel_metrics_refresh_rate_seconds') || $settings->wasChanged('sentinel_metrics_history_days') || $settings->wasChanged('sentinel_push_interval_seconds') || diff --git a/tests/Feature/Api/ServerSubsystemsApiTest.php b/tests/Feature/Api/ServerSubsystemsApiTest.php index cdeb762170..37699bff1f 100644 --- a/tests/Feature/Api/ServerSubsystemsApiTest.php +++ b/tests/Feature/Api/ServerSubsystemsApiTest.php @@ -1,5 +1,6 @@ server->settings->update([ + // Saved quietly: a metrics change restarts Sentinel, which needs SSH. + $this->server->settings->forceFill([ 'is_sentinel_enabled' => true, 'is_metrics_enabled' => true, 'is_sentinel_debug_enabled' => false, - ]); + ])->saveQuietly(); $readToken = $this->user->createToken('server-subsystems-read', ['read'])->plainTextToken; @@ -225,8 +227,9 @@ describe('Sentinel API', function () { ->and($response->json())->not->toHaveKey('sentinel_custom_url'); }); - test('PATCH updates sentinel settings for own team server', function () { - // Only toggle fields that do not restart Sentinel (avoids remote StartSentinel on sync queue). + test('PATCH updates sentinel settings for own team server and restarts Sentinel once', function () { + Queue::fake(); + $this->withHeaders(serverSubsystemsHeaders()) ->patchJson("/api/v1/servers/{$this->server->uuid}/sentinel", [ 'is_metrics_enabled' => true, @@ -239,6 +242,9 @@ describe('Sentinel API', function () { $settings = $this->server->settings->fresh(); expect((bool) $settings->is_metrics_enabled)->toBeTrue() ->and((bool) $settings->is_sentinel_debug_enabled)->toBeTrue(); + // Sentinel reads both settings from its environment, so it must be recreated. + expect(Queue::pushed(JobDecorator::class, fn (JobDecorator $job): bool => $job->getAction() instanceof StartSentinel)) + ->toHaveCount(1); }); test('PATCH rejects disabling mandatory Sentinel', function () { diff --git a/tests/Feature/Server/ServerSettingSentinelRestartTest.php b/tests/Feature/Server/ServerSettingSentinelRestartTest.php index 8e2b2b32b0..ccedd9454f 100644 --- a/tests/Feature/Server/ServerSettingSentinelRestartTest.php +++ b/tests/Feature/Server/ServerSettingSentinelRestartTest.php @@ -192,3 +192,14 @@ it('does not detect changes when sentinel field is set to same value', function expect($changeDetected)->toBeFalse(); }); + +it('restarts sentinel when a setting in its environment changes while sentinel is enabled', function (string $field) { + $settings = $this->server->settings; + $settings->is_sentinel_enabled = true; + $settings->save(); + + $settings->{$field} = ! $settings->{$field}; + $settings->save(); + + Queue::assertPushed(JobDecorator::class, fn ($job) => isStartSentinelJob($job)); +})->with(['is_metrics_enabled', 'is_sentinel_debug_enabled']); diff --git a/tests/Feature/Server/StartSentinelLockTest.php b/tests/Feature/Server/StartSentinelLockTest.php new file mode 100644 index 0000000000..cec5438a45 --- /dev/null +++ b/tests/Feature/Server/StartSentinelLockTest.php @@ -0,0 +1,98 @@ + 0]); + config(['constants.ssh.mux_enabled' => false]); + + $this->user = User::factory()->create(); + $this->team = $this->user->teams()->first(); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $this->server = Server::factory()->create([ + 'team_id' => $this->team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $this->team->id])->id, + ]); + $this->server->settings->forceFill([ + 'is_sentinel_enabled' => true, + 'sentinel_custom_url' => 'https://coolify.example.com', + ])->saveQuietly(); +}); + +function queuedSentinelStarts(): int +{ + return Queue::pushed(JobDecorator::class, fn (JobDecorator $job): bool => $job->getAction() instanceof StartSentinel)->count(); +} + +it('holds the per-server lock while it replaces the Sentinel container', function () { + $lockHeldDuringDockerRun = null; + Process::fake(function ($process) use (&$lockHeldDuringDockerRun) { + $script = is_array($process->command) ? implode(' ', $process->command) : $process->command; + if (str_contains($script, 'docker run -d')) { + $lockHeldDuringDockerRun = ! Cache::lock(StartSentinel::lockKey($this->server), 1)->get(); + } + + return Process::result(output: ''); + }); + + StartSentinel::run($this->server, restart: true, latestVersion: '1.0.2'); + + expect($lockHeldDuringDockerRun)->toBeTrue() + ->and(Cache::lock(StartSentinel::lockKey($this->server), 1)->get())->toBeTrue(); +}); + +it('starts Sentinel with the settings saved after the server was loaded', function () { + $staleServer = Server::query()->findOrFail($this->server->id); + $staleServer->settings; + Server::query()->findOrFail($this->server->id)->settings->forceFill(['sentinel_push_interval_seconds' => 33])->saveQuietly(); + + $scripts = []; + Process::fake(function ($process) use (&$scripts) { + $scripts[] = is_array($process->command) ? implode(' ', $process->command) : $process->command; + + return Process::result(output: ''); + }); + + StartSentinel::run($staleServer, restart: true, latestVersion: '1.0.2'); + + expect(collect($scripts)->first(fn (string $script): bool => str_contains($script, 'docker run -d'))) + ->toContain('PUSH_INTERVAL_SECONDS=33'); +}); + +it('starts Sentinel once when the metrics page toggles metrics', function () { + StartSentinel::shouldRun()->once(); + + Livewire::test(Charts::class, ['server_uuid' => $this->server->uuid]) + ->call('toggleMetrics'); + + expect(queuedSentinelStarts())->toBe(0); +}); + +it('restarts Sentinel once when the Sentinel page changes debug logging', function () { + Livewire::test(Sentinel::class, ['server' => $this->server]) + ->set('isSentinelDebugEnabled', true) + ->call('instantSave'); + + expect(queuedSentinelStarts())->toBe(1); +});