From c2f63c8f10046cb4c6a765398ad2d69af2ed6f6c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:51:58 +0200 Subject: [PATCH] fix(server): remove redundant SSH work from server checks - ServerCheckJob uses the stored reachability state instead of a second SSH probe. One failed probe no longer marks the server unreachable without UNREACHABLE_THRESHOLD or marks all its resources exited. - Do not dispatch ServerCheckJob for build servers and Swarm workers. - Check cloud provider state every 5 minutes instead of every minute. - ServerPatchCheckJob uses isFunctional() and drops the unused retries. - Delete the unused ServerCleanupMux job. - Disable SSH multiplexing per command. The global config change stayed in the queue worker, so later jobs (also deployments) ran without mux. --- app/Jobs/ServerCheckJob.php | 8 +- app/Jobs/ServerCleanupMux.php | 40 ----- app/Jobs/ServerConnectionCheckJob.php | 14 +- app/Jobs/ServerManagerJob.php | 14 +- app/Jobs/ServerPatchCheckJob.php | 4 +- app/Livewire/Boarding/Index.php | 11 +- app/Models/Server.php | 60 ++++---- app/Services/ConfigurationRepository.php | 5 - .../Server/ServerCheckReachabilityTest.php | 145 ++++++++++++++++++ 9 files changed, 196 insertions(+), 105 deletions(-) delete mode 100644 app/Jobs/ServerCleanupMux.php create mode 100644 tests/Feature/Server/ServerCheckReachabilityTest.php diff --git a/app/Jobs/ServerCheckJob.php b/app/Jobs/ServerCheckJob.php index f978685411..a4fa961345 100644 --- a/app/Jobs/ServerCheckJob.php +++ b/app/Jobs/ServerCheckJob.php @@ -52,7 +52,13 @@ class ServerCheckJob implements ShouldBeEncrypted, ShouldQueue public function handle() { try { - if ($this->server->serverStatus() === false) { + // ServerConnectionCheckJob owns reachability. Do not probe SSH again here: one failed + // probe would mark the server unreachable without UNREACHABLE_THRESHOLD. + if ($this->server->isFunctional() === false) { + if ($this->server->settings->is_reachable === false) { + $this->server->markResourcesAsExited(); + } + return 'Server is not reachable or not ready.'; } diff --git a/app/Jobs/ServerCleanupMux.php b/app/Jobs/ServerCleanupMux.php deleted file mode 100644 index b793c3eca6..0000000000 --- a/app/Jobs/ServerCleanupMux.php +++ /dev/null @@ -1,40 +0,0 @@ -server->serverStatus() === false) { - return 'Server is not reachable or not ready.'; - } - SshMultiplexingHelper::removeMuxFile($this->server); - } catch (\Throwable $e) { - return handleError($e); - } - } -} diff --git a/app/Jobs/ServerConnectionCheckJob.php b/app/Jobs/ServerConnectionCheckJob.php index 886c659379..ab8d56a98a 100644 --- a/app/Jobs/ServerConnectionCheckJob.php +++ b/app/Jobs/ServerConnectionCheckJob.php @@ -5,7 +5,6 @@ namespace App\Jobs; use App\Events\ServerReachabilityChanged; use App\Helpers\SshMultiplexingHelper; use App\Models\Server; -use App\Services\ConfigurationRepository; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -55,12 +54,6 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue return [(new WithoutOverlapping('server-connection-check-'.$this->server->uuid))->expireAfter($this->timeout + 30)->dontRelease()]; } - private function disableSshMux(): void - { - $configRepository = app(ConfigurationRepository::class); - $configRepository->disableSshMux(); - } - public function handle(): void { if ($this->server->hasPlaceholderIp()) { @@ -87,11 +80,6 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue return; } - // Temporarily disable mux if requested - if ($this->disableMux) { - $this->disableSshMux(); - } - // Check basic connectivity first $isReachable = $this->checkConnection(); @@ -291,7 +279,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue private function runDockerCommand(string $command): ?string { try { - $process = Process::timeout(self::DOCKER_TIMEOUT_SECONDS)->run($this->sshCommand($command)); + $process = Process::timeout(self::DOCKER_TIMEOUT_SECONDS)->run($this->sshCommand($command, disableMultiplexing: $this->disableMux)); } catch (\Throwable $e) { Log::debug('ServerConnectionCheck: Docker command failed', [ 'server_id' => $this->server->id, diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 3d9bae6cad..95cb20fb1c 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -31,6 +31,12 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue private string $checkFrequency = '* * * * *'; + /** + * Provider state seldom changes, and each check is one API call per server. + * Hetzner allows 3600 requests per hour for each token. + */ + private const CLOUD_PROVIDER_STATUS_CHECK_CRON = '*/5 * * * *'; + /** * Create a new job instance. */ @@ -86,7 +92,7 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue private function dispatchCloudProviderStatusChecks(Collection $servers): void { - if (! shouldRunCronNow($this->checkFrequency, $this->instanceTimezone, 'server-cloud-provider-status-checks', $this->executionTime)) { + if (! shouldRunCronNow(self::CLOUD_PROVIDER_STATUS_CHECK_CRON, $this->instanceTimezone, 'server-cloud-provider-status-checks', $this->executionTime)) { return; } @@ -161,7 +167,11 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue $waitTime = $server->waitBeforeDoingSshCheck(); $sentinelOutOfSync = Carbon::parse($lastSentinelUpdate)->isBefore($this->executionTime->copy()->subSeconds($waitTime)); - if ($sentinelOutOfSync) { + // Build servers and Swarm workers have no containers to sync, and ServerConnectionCheckJob + // already checks their connection. + $hasContainersToSync = ! $server->isBuildServer() && ! $server->isSwarmWorker(); + + if ($sentinelOutOfSync && $hasContainersToSync) { // Dispatch ServerCheckJob if Sentinel is out of sync if (shouldRunCronNow($this->checkFrequency, $serverTimezone, "server-check:{$server->id}", $this->executionTime)) { if (! $this->shouldSkipDueToBackoff($server)) { diff --git a/app/Jobs/ServerPatchCheckJob.php b/app/Jobs/ServerPatchCheckJob.php index 42caa3861c..a383a962a3 100644 --- a/app/Jobs/ServerPatchCheckJob.php +++ b/app/Jobs/ServerPatchCheckJob.php @@ -18,8 +18,6 @@ class ServerPatchCheckJob implements ShouldBeEncrypted, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; - public $tries = 3; - public $timeout = 600; public function middleware(): array @@ -35,7 +33,7 @@ class ServerPatchCheckJob implements ShouldBeEncrypted, ShouldQueue public function handle(): void { try { - if ($this->server->serverStatus() === false) { + if ($this->server->isFunctional() === false) { return; } diff --git a/app/Livewire/Boarding/Index.php b/app/Livewire/Boarding/Index.php index 38484ef6f3..ca305905a3 100644 --- a/app/Livewire/Boarding/Index.php +++ b/app/Livewire/Boarding/Index.php @@ -7,7 +7,6 @@ use App\Models\PrivateKey; use App\Models\Project; use App\Models\Server; use App\Models\Team; -use App\Services\ConfigurationRepository; use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; @@ -341,10 +340,8 @@ class Index extends Component $this->authorizeCreatedServer(); try { - $this->disableSshMux(); - // EC2 does not have `uptime` command, lol - instant_remote_process(['ls /'], $this->createdServer, true); + instant_remote_process(['ls /'], $this->createdServer, true, disableMultiplexing: true); $this->createdServer->settings()->update([ 'is_reachable' => true, @@ -524,12 +521,6 @@ class Index extends Component ['private' => $this->privateKey, 'public' => $this->publicKey] = generateSSHKey(); } - private function disableSshMux(): void - { - $configRepository = app(ConfigurationRepository::class); - $configRepository->disableSshMux(); - } - private function authorizeCreatedServer(): void { $this->createdServer = Server::findOrFail($this->createdServer?->id); diff --git a/app/Models/Server.php b/app/Models/Server.php index d80a924ae6..cf32ce702a 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -18,7 +18,6 @@ use App\Jobs\ServerConnectionCheckJob; use App\Livewire\Server\Proxy; use App\Notifications\Server\Reachable; use App\Notifications\Server\Unreachable; -use App\Services\ConfigurationRepository; use App\Services\DigitalOceanService; use App\Services\HetznerService; use App\Services\VultrService; @@ -1050,7 +1049,6 @@ $siteAddress { $this->settings->save(); $sshKeyFileLocation = "id.root@{$this->uuid}"; Storage::disk('ssh-keys')->delete($sshKeyFileLocation); - $this->disableSshMux(); } public function sentinelHeartbeat(bool $isReset = false) @@ -1785,26 +1783,7 @@ $siteAddress { { ['uptime' => $uptime] = $this->validateConnection(); if ($uptime === false) { - foreach ($this->applications() as $application) { - $application->status = 'exited'; - $application->save(); - } - foreach ($this->databases() as $database) { - $database->status = 'exited'; - $database->save(); - } - foreach ($this->services() as $service) { - $apps = $service->applications()->get(); - $dbs = $service->databases()->get(); - foreach ($apps as $app) { - $app->status = 'exited'; - $app->save(); - } - foreach ($dbs as $db) { - $db->status = 'exited'; - $db->save(); - } - } + $this->markResourcesAsExited(); return false; } @@ -1812,6 +1791,33 @@ $siteAddress { return true; } + /** + * Mark all resources on this server as exited, because their containers cannot be checked. + */ + public function markResourcesAsExited(): void + { + foreach ($this->applications() as $application) { + $application->status = 'exited'; + $application->save(); + } + foreach ($this->databases() as $database) { + $database->status = 'exited'; + $database->save(); + } + foreach ($this->services() as $service) { + $apps = $service->applications()->get(); + $dbs = $service->databases()->get(); + foreach ($apps as $app) { + $app->status = 'exited'; + $app->save(); + } + foreach ($dbs as $db) { + $db->status = 'exited'; + $db->save(); + } + } + } + public function isReachableChanged() { $this->refresh(); @@ -1845,13 +1851,11 @@ $siteAddress { public function validateConnection(bool $justCheckingNewKey = false) { - $this->disableSshMux(); - if ($this->skipServer()) { return ['uptime' => false, 'error' => 'Server skipped.']; } try { - instant_remote_process(['ls /'], $this); + instant_remote_process(['ls /'], $this, disableMultiplexing: true); if ($this->settings->is_reachable === false) { $this->settings->is_reachable = true; $this->settings->save(); @@ -2144,12 +2148,6 @@ $siteAddress { $this->services()->count() == 0; } - private function disableSshMux(): void - { - $configRepository = app(ConfigurationRepository::class); - $configRepository->disableSshMux(); - } - /** * Return the server's CA certificate, generating it first when it does not exist yet. */ diff --git a/app/Services/ConfigurationRepository.php b/app/Services/ConfigurationRepository.php index ead3e4a21c..660191859e 100644 --- a/app/Services/ConfigurationRepository.php +++ b/app/Services/ConfigurationRepository.php @@ -61,9 +61,4 @@ class ConfigurationRepository Mail::purge(); } } - - public function disableSshMux(): void - { - $this->config->set('constants.ssh.mux_enabled', false); - } } diff --git a/tests/Feature/Server/ServerCheckReachabilityTest.php b/tests/Feature/Server/ServerCheckReachabilityTest.php new file mode 100644 index 0000000000..7d350f826d --- /dev/null +++ b/tests/Feature/Server/ServerCheckReachabilityTest.php @@ -0,0 +1,145 @@ + true]); + Carbon::setTestNow('2026-07-11 12:00:00'); + InstanceSettings::forceCreate(['id' => 0, 'instance_timezone' => 'UTC']); + $this->team = Team::factory()->create(); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->server->settings->update([ + 'server_timezone' => 'UTC', + 'server_role' => ServerRole::BOTH, + 'is_reachable' => true, + 'is_usable' => true, + ]); + $this->server->refresh(); +}); + +afterEach(function () { + Carbon::setTestNow(); +}); + +function createServerCheckReachabilityApplication(Server $server): Application +{ + $project = Project::factory()->create(['team_id' => $server->team_id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $destination = StandaloneDocker::factory()->create([ + 'server_id' => $server->id, + 'name' => 'dest-'.fake()->unique()->word(), + 'network' => 'net-'.fake()->unique()->word(), + ]); + + return Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'status' => 'running:healthy', + ]); +} + +it('marks resources exited on an unreachable server without an SSH probe', function () { + Process::fake(); + $application = createServerCheckReachabilityApplication($this->server); + $this->server->settings->update(['is_reachable' => false, 'is_usable' => false]); + + (new ServerCheckJob($this->server->refresh()))->handle(); + + Process::assertNothingRan(); + expect($application->refresh()->status)->toStartWith('exited'); +}); + +it('does not mark a reachable server unreachable when it is not usable', function () { + Process::fake(); + $application = createServerCheckReachabilityApplication($this->server); + $this->server->settings->update(['is_usable' => false]); + + (new ServerCheckJob($this->server->refresh()))->handle(); + + Process::assertNothingRan(); + expect($application->refresh()->status)->toBe('running:healthy') + ->and($this->server->settings->refresh()->is_reachable)->toBeTrue(); +}); + +it('skips the patch check on a server that is not functional without an SSH probe', function () { + Process::fake(); + $this->server->settings->update(['is_reachable' => false, 'is_usable' => false]); + + (new ServerPatchCheckJob($this->server->refresh()))->handle(); + + Process::assertNothingRan(); +}); + +it('does not dispatch ServerCheckJob for build servers and Swarm workers', function (array $settings) { + Queue::fake(); + $this->server->settings->update($settings); + + (new ServerManagerJob)->handle(); + + Queue::assertNotPushed(ServerCheckJob::class); +})->with([ + 'build server' => [['server_role' => ServerRole::BUILD]], + 'swarm worker' => [['is_swarm_worker' => true]], +]); + +it('dispatches ServerCheckJob for a deployment server when Sentinel is out of sync', function () { + Queue::fake(); + + (new ServerManagerJob)->handle(); + + Queue::assertPushed(ServerCheckJob::class, fn (ServerCheckJob $job) => $job->server->is($this->server)); +}); + +it('checks the cloud provider status every five minutes', function () { + Queue::fake(); + $token = CloudProviderToken::create([ + 'team_id' => $this->team->id, + 'provider' => 'hetzner', + 'token' => 'test-hetzner-token', + 'name' => 'Hetzner', + ]); + $this->server->update(['cloud_provider_token_id' => $token->id, 'hetzner_server_id' => 123]); + + $pushedAt = collect(['12:00', '12:01', '12:04', '12:05'])->filter(function (string $time) { + Carbon::setTestNow("2026-07-11 {$time}:00"); + Server::flushIdentityMap(); + $before = Queue::pushed(ServerCloudProviderStatusCheckJob::class)->count(); + + (new ServerManagerJob)->handle(); + + return Queue::pushed(ServerCloudProviderStatusCheckJob::class)->count() > $before; + })->values()->all(); + + expect($pushedAt)->toBe(['12:00', '12:05']); +}); + +it('does not disable SSH multiplexing for later jobs in the same worker', function () { + Process::fake(); + config(['constants.ssh.mux_enabled' => true]); + + (new ServerConnectionCheckJob($this->server))->handle(); + $this->server->validateConnection(); + + expect(config('constants.ssh.mux_enabled'))->toBeTrue(); +});