diff --git a/app/Jobs/ServerConnectionCheckJob.php b/app/Jobs/ServerConnectionCheckJob.php index 182164b35e..886c659379 100644 --- a/app/Jobs/ServerConnectionCheckJob.php +++ b/app/Jobs/ServerConnectionCheckJob.php @@ -16,6 +16,7 @@ use Illuminate\Queue\SerializesModels; use Illuminate\Queue\TimeoutExceededException; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Process; +use Illuminate\Support\Sleep; use Illuminate\Support\Str; class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue @@ -24,7 +25,23 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue public $tries = 1; - public $timeout = 15; + public $timeout = 60; + + /** + * Consecutive failed SSH checks before the server is marked unreachable. + * One failed attempt is often a short network problem, not an offline server. + */ + public const UNREACHABLE_THRESHOLD = 2; + + /** Each check makes this many SSH attempts before it counts as failed. */ + public const SSH_ATTEMPTS = 2; + + public const SSH_TIMEOUT_SECONDS = 10; + + public const SSH_RETRY_DELAY_SECONDS = 3; + + /** Docker and Compose version commands each get one attempt with this timeout. */ + public const DOCKER_TIMEOUT_SECONDS = 15; private ?string $connectionError = null; @@ -35,7 +52,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue public function middleware(): array { - return [(new WithoutOverlapping('server-connection-check-'.$this->server->uuid))->expireAfter(25)->dontRelease()]; + return [(new WithoutOverlapping('server-connection-check-'.$this->server->uuid))->expireAfter($this->timeout + 30)->dontRelease()]; } private function disableSshMux(): void @@ -79,16 +96,22 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue $isReachable = $this->checkConnection(); if (! $isReachable) { - $this->server->settings->update([ - 'is_reachable' => false, - 'is_usable' => false, - ]); $this->server->increment('unreachable_count'); Log::warning('ServerConnectionCheck: Server not reachable', [ 'server_id' => $this->server->id, 'server_name' => $this->server->name, 'server_ip' => $this->server->ip, + 'unreachable_count' => $this->server->unreachable_count, + ]); + + if ($this->server->unreachable_count < self::UNREACHABLE_THRESHOLD) { + return; + } + + $this->server->settings->update([ + 'is_reachable' => false, + 'is_usable' => false, ]); $this->logConnectionStateChange($wasReachable, $wasUsable, false, false, $this->connectionError); @@ -116,21 +139,12 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, true); } catch (\Throwable $e) { - + // SSH and Docker failures are handled above. An error here (database, cache, a bug) + // says nothing about the server, so the stored connection state stays as it is. Log::error('ServerConnectionCheckJob failed', [ 'error' => $e->getMessage(), 'server_id' => $this->server->id, ]); - $this->server->settings->update([ - 'is_reachable' => false, - 'is_usable' => false, - ]); - $this->server->increment('unreachable_count'); - $this->logConnectionStateChange($wasReachable, $wasUsable, false, false, $e->getMessage()); - - $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, false); - - return; } } @@ -166,7 +180,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue } /** - * Fire ServerReachabilityChanged when state crosses the unreachable threshold (count >= 2) + * Fire ServerReachabilityChanged when state crosses UNREACHABLE_THRESHOLD * or when a previously-notified server recovers. Skips noise from single transient flaps. */ private function dispatchReachabilityChangedIfNeeded(bool $wasReachable, bool $wasNotified, bool $isReachable): void @@ -179,24 +193,44 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue return; } - if ($this->server->unreachable_count >= 2 && ! $wasNotified) { + if ($this->server->unreachable_count >= self::UNREACHABLE_THRESHOLD && ! $wasNotified) { ServerReachabilityChanged::dispatch($this->server); } } + /** + * Worst-case run time of one check: every SSH attempt and delay, then the Docker and Compose commands. + */ + public static function maximumCheckSeconds(): int + { + return self::SSH_ATTEMPTS * self::SSH_TIMEOUT_SECONDS + + (self::SSH_ATTEMPTS - 1) * self::SSH_RETRY_DELAY_SECONDS + + 2 * self::DOCKER_TIMEOUT_SECONDS; + } + + /** + * A short retry inside the check absorbs brief network problems. Longer outages are + * handled across checks with unreachable_count and UNREACHABLE_THRESHOLD. + */ private function checkConnection(): bool { - try { - // Single SSH attempt without SshRetryHandler — retries waste time for connectivity checks. - // Backoff is managed at the dispatch level via unreachable_count. - $commands = ['ls -la /']; - if ($this->server->isNonRoot()) { - $commands = parseCommandsByLineForSudo(collect($commands), $this->server); + for ($attempt = 1; $attempt <= self::SSH_ATTEMPTS; $attempt++) { + if ($attempt > 1) { + Sleep::for(self::SSH_RETRY_DELAY_SECONDS)->seconds(); } - $commandString = implode("\n", $commands); + if ($this->attemptConnection()) { + return true; + } + } - $sshCommand = SshMultiplexingHelper::generateSshCommand($this->server, $commandString, true); - $process = Process::timeout(10)->run($sshCommand); + return false; + } + + private function attemptConnection(): bool + { + try { + $process = Process::timeout(self::SSH_TIMEOUT_SECONDS) + ->run($this->sshCommand('ls -la /', disableMultiplexing: true)); if ($process->exitCode() !== 0) { $this->connectionError = 'ssh exit '.$process->exitCode().': '.Str::limit(trim($process->errorOutput()), 300); } @@ -216,13 +250,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue private function checkDockerAvailability(): bool { try { - // Use instant_remote_process to check Docker - // The function will automatically handle sudo for non-root users - $output = instant_remote_process_with_timeout( - ['docker version --format json'], - $this->server, - false // don't throw error - ); + $output = $this->runDockerCommand('docker version --format json'); if ($output === null) { return false; @@ -237,11 +265,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue $this->server->rememberDockerVersion($dockerVersion); } - $composeOutput = instant_remote_process_with_timeout( - ['docker compose version --short'], - $this->server, - false - ); + $composeOutput = $this->runDockerCommand('docker compose version --short'); $composeVersion = parseDockerEngineVersion($composeOutput); if ($composeVersion !== null) { $this->server->rememberComposeVersion($composeVersion); @@ -260,4 +284,34 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue return false; } } + + /** + * Run one Docker command with a single attempt. Returns null when it fails or times out. + */ + private function runDockerCommand(string $command): ?string + { + try { + $process = Process::timeout(self::DOCKER_TIMEOUT_SECONDS)->run($this->sshCommand($command)); + } catch (\Throwable $e) { + Log::debug('ServerConnectionCheck: Docker command failed', [ + 'server_id' => $this->server->id, + 'command' => $command, + 'error' => $e->getMessage(), + ]); + + return null; + } + + return $process->successful() ? sanitize_utf8_text(trim($process->output())) : null; + } + + private function sshCommand(string $command, bool $disableMultiplexing = false): string + { + $commands = [$command]; + if ($this->server->isNonRoot()) { + $commands = parseCommandsByLineForSudo(collect($commands), $this->server); + } + + return SshMultiplexingHelper::generateSshCommand($this->server, implode("\n", $commands), $disableMultiplexing); + } } diff --git a/app/Models/Server.php b/app/Models/Server.php index f4004713da..141b3c77c5 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -14,6 +14,7 @@ use App\Helpers\SslHelper; use App\Jobs\CheckAndStartSentinelJob; use App\Jobs\CheckTraefikVersionForServerJob; use App\Jobs\RegenerateSslCertJob; +use App\Jobs\ServerConnectionCheckJob; use App\Livewire\Server\Proxy; use App\Notifications\Server\Reachable; use App\Notifications\Server\Unreachable; @@ -1801,7 +1802,7 @@ $siteAddress { return; } - if ($this->unreachable_count >= 2 && ! $unreachableNotificationSent) { + if ($this->unreachable_count >= ServerConnectionCheckJob::UNREACHABLE_THRESHOLD && ! $unreachableNotificationSent) { $this->sendUnreachableNotification(); } } diff --git a/tests/Feature/ServerConnectionCheckIsolationTest.php b/tests/Feature/ServerConnectionCheckIsolationTest.php index e5431417c3..771c4a832a 100644 --- a/tests/Feature/ServerConnectionCheckIsolationTest.php +++ b/tests/Feature/ServerConnectionCheckIsolationTest.php @@ -10,6 +10,7 @@ use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Process; use Illuminate\Support\Facades\Storage; +use Illuminate\Support\Sleep; uses(RefreshDatabase::class); @@ -26,6 +27,7 @@ function createServerForConnectionIsolationTest(array $attributes = []): Server beforeEach(function () { Storage::fake('ssh-keys'); + Sleep::fake(); }); it('never attempts SSH to a placeholder address', function (string $placeholderIp) { @@ -120,3 +122,124 @@ it('logs the checking node and ssh error only when the connection state changes' ->toContain('ssh exit 255: ssh: connect to host 203.0.113.10 port 22: Connection refused') ->toContain('"host":"'.gethostname().'"'); }); + +it('keeps the server online after one failed SSH check', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + Process::fake([ + '*' => Process::result(errorOutput: 'Connection timed out', exitCode: 255), + ]); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + expect($server->fresh()->unreachable_count)->toBe(1) + ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue() + ->and((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +}); + +it('marks the server offline after consecutive failed SSH checks', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + Process::fake([ + '*' => Process::result(errorOutput: 'Connection timed out', exitCode: 255), + ]); + + for ($attempt = 0; $attempt < ServerConnectionCheckJob::UNREACHABLE_THRESHOLD; $attempt++) { + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + } + + expect($server->fresh()->unreachable_count)->toBe(ServerConnectionCheckJob::UNREACHABLE_THRESHOLD) + ->and((bool) $server->settings->fresh()->is_reachable)->toBeFalse() + ->and((bool) $server->settings->fresh()->is_usable)->toBeFalse(); +}); + +it('does not mark the server offline when the check fails for a reason other than SSH or Docker', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + $server->unreachable_count = 1; + $server->save(); + Process::fake([ + '*' => Process::result(output: '{"Server":{"Version":"27.0.0"}}', exitCode: 0), + ]); + // Fails the unreachable_count reset, after SSH and Docker checks passed. + Server::saving(fn () => throw new RuntimeException('database connection lost')); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + Server::flushEventListeners(); + expect($server->fresh()->unreachable_count)->toBe(1) + ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue() + ->and((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +}); + +it('retries a failed SSH attempt once within the same check', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => false, 'is_usable' => false]); + $server->unreachable_count = 3; + $server->save(); + Process::fake([ + '*ls -la /*' => Process::sequence() + ->push(Process::result(errorOutput: 'Connection timed out', exitCode: 255)) + ->push(Process::result(exitCode: 0)), + '*compose*' => Process::result(output: 'v2.32.4', exitCode: 0), + '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), + ]); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + Sleep::assertSleptTimes(1); + Process::assertRanTimes(fn ($process) => str_contains($process->command, 'ls -la /'), 2); + expect($server->fresh()->unreachable_count)->toBe(0) + ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue() + ->and((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +}); + +it('counts one failed check when both SSH attempts fail', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + Process::fake([ + '*' => Process::result(errorOutput: 'Connection timed out', exitCode: 255), + ]); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + Sleep::assertSleptTimes(1); + Process::assertRanTimes(fn ($process) => str_contains($process->command, 'ls -la /'), 2); + expect($server->fresh()->unreachable_count)->toBe(1) + ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue(); +}); + +it('does not retry the Docker check within the same check', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + $server->settings->update(['is_reachable' => true, 'is_usable' => true]); + Process::fake([ + '*docker version*' => Process::result(errorOutput: 'Cannot connect to the Docker daemon', exitCode: 1), + '*' => Process::result(exitCode: 0), + ]); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + Process::assertRanTimes(fn ($process) => str_contains($process->command, 'docker version'), 1); + expect((bool) $server->settings->fresh()->is_reachable)->toBeTrue() + ->and((bool) $server->settings->fresh()->is_usable)->toBeFalse(); +}); + +it('keeps the server usable when only the compose version check fails', function () { + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + Process::fake([ + '*compose*' => Process::result(errorOutput: 'docker: compose is not a docker command', exitCode: 1), + '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), + ]); + + (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + + expect((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +}); + +it('gives the job enough time for every connection check step', function () { + $job = new ServerConnectionCheckJob(new Server); + $lock = collect($job->middleware())->first(); + + expect($job->timeout)->toBeGreaterThanOrEqual(ServerConnectionCheckJob::maximumCheckSeconds()) + ->and($lock->expiresAfter)->toBeGreaterThan($job->timeout); +}); diff --git a/tests/Unit/ServerBackoffTest.php b/tests/Unit/ServerBackoffTest.php index 11a0c606af..114283a35e 100644 --- a/tests/Unit/ServerBackoffTest.php +++ b/tests/Unit/ServerBackoffTest.php @@ -8,12 +8,14 @@ use App\Models\Server; use Illuminate\Queue\TimeoutExceededException; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Sleep; use Tests\TestCase; uses(TestCase::class); beforeEach(function () { Carbon::setTestNow('2025-01-15 12:00:00'); + Sleep::fake(); }); afterEach(function () { @@ -127,7 +129,7 @@ describe('shouldSkipDueToBackoff', function () { }); describe('ServerConnectionCheckJob unreachable_count', function () { - it('marks servers unreachable when SSH is unavailable', function () { + it('marks servers unreachable when SSH fails at the threshold', function () { Event::fake([ServerReachabilityChanged::class]); $settings = Mockery::mock(); @@ -145,7 +147,7 @@ describe('ServerConnectionCheckJob unreachable_count', function () { 'ip' => '203.0.113.10', 'id' => 1, 'name' => 'test-server', - 'unreachable_count' => 1, + 'unreachable_count' => ServerConnectionCheckJob::UNREACHABLE_THRESHOLD, default => null, }); $server->shouldReceive('increment')->with('unreachable_count')->once();