diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index a5f421da3d..145f08093c 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -329,7 +329,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, 'horizon_job_worker' => gethostname(), ]); - if ($this->server->isFunctional() === false) { + if ($this->server->isFunctionalAfterRecheck() === false) { $this->application_deployment_queue->addLogEntry('Server is not functional.'); $this->fail('Server is not functional.'); diff --git a/app/Jobs/ServerManagerJob.php b/app/Jobs/ServerManagerJob.php index 20b60b9fd2..a3710532b5 100644 --- a/app/Jobs/ServerManagerJob.php +++ b/app/Jobs/ServerManagerJob.php @@ -110,8 +110,11 @@ class ServerManagerJob implements ShouldBeEncrypted, ShouldQueue return; } - // Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity - if ($server->isSentinelEnabled() && $server->isSentinelLive()) { + // Skip SSH connection check if Sentinel is healthy — its heartbeat already proves connectivity. + // Never skip while the server is marked unreachable or unusable: the heartbeat does not + // restore these flags, so deployments would fail with "Server is not functional" forever. + if ($server->isSentinelEnabled() && $server->isSentinelLive() + && $server->settings->is_reachable && $server->settings->is_usable) { return; } if ($this->shouldSkipDueToBackoff($server)) { diff --git a/app/Models/Server.php b/app/Models/Server.php index 141b3c77c5..04683c8e2c 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -1630,6 +1630,25 @@ $siteAddress { return $isFunctional; } + /** + * Like isFunctional(), but runs a live SSH and Docker check when the server is marked + * unreachable. The flag can be stale after one failed scheduled check. + */ + public function isFunctionalAfterRecheck(): bool + { + if ($this->isFunctional()) { + return true; + } + if ($this->settings->is_reachable || $this->settings->force_disabled || $this->hasPlaceholderIp()) { + return false; + } + + (new ServerConnectionCheckJob($this, disableMux: false))->handle(); + $this->settings->refresh(); + + return $this->isFunctional(); + } + public function isLogDrainEnabled() { return $this->settings->is_logdrain_newrelic_enabled || $this->settings->is_logdrain_highlight_enabled || $this->settings->is_logdrain_axiom_enabled || $this->settings->is_logdrain_custom_enabled; diff --git a/tests/Feature/ServerFunctionalRecheckTest.php b/tests/Feature/ServerFunctionalRecheckTest.php new file mode 100644 index 0000000000..8e1b09bdc9 --- /dev/null +++ b/tests/Feature/ServerFunctionalRecheckTest.php @@ -0,0 +1,94 @@ +create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => $privateKey->id, + 'ip' => '203.0.113.10', + ]); + $server->settings->update($settings); + + return $server->refresh(); +} + +it('marks a server functional again when the live SSH and Docker checks pass', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => false, 'is_usable' => false]); + + Process::fake([ + '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), + ]); + + expect($server->isFunctionalAfterRecheck())->toBeTrue() + ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue() + ->and((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +}); + +it('keeps a server not functional when the live SSH check fails', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => false, 'is_usable' => false]); + + Process::fake([ + '*' => Process::result(errorOutput: 'Connection refused', exitCode: 255), + ]); + + expect($server->isFunctionalAfterRecheck())->toBeFalse() + ->and((bool) $server->settings->fresh()->is_reachable)->toBeFalse(); +}); + +it('does not connect to a reachable server that is marked unusable', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => true, 'is_usable' => false]); + + Process::fake(); + + expect($server->isFunctionalAfterRecheck())->toBeFalse(); + + Process::assertNothingRan(); +}); + +it('keeps a server not functional when Docker does not respond', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => false, 'is_usable' => false]); + + Process::fake([ + '*docker*' => Process::result(output: '', exitCode: 1), + '*' => Process::result(output: '', exitCode: 0), + ]); + + expect($server->isFunctionalAfterRecheck())->toBeFalse() + ->and((bool) $server->settings->fresh()->is_usable)->toBeFalse(); +}); + +it('does not connect to a server that is already functional', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => true, 'is_usable' => true]); + + Process::fake(); + + expect($server->isFunctionalAfterRecheck())->toBeTrue(); + + Process::assertNothingRan(); +}); + +it('does not connect to a disabled server', function () { + $server = createServerForFunctionalRecheckTest(['is_reachable' => false, 'is_usable' => false, 'force_disabled' => true]); + + Process::fake(); + + expect($server->isFunctionalAfterRecheck())->toBeFalse(); + + Process::assertNothingRan(); +}); diff --git a/tests/Unit/ServerManagerJobSentinelCheckTest.php b/tests/Unit/ServerManagerJobSentinelCheckTest.php index 5e87bcf7fe..df1b1c3ee6 100644 --- a/tests/Unit/ServerManagerJobSentinelCheckTest.php +++ b/tests/Unit/ServerManagerJobSentinelCheckTest.php @@ -35,6 +35,8 @@ function createSentinelCheckServer(Team $team, Carbon $sentinelUpdatedAt, Server 'server_timezone' => 'UTC', 'server_role' => $role, 'is_build_server' => $role === ServerRole::BUILD, + 'is_reachable' => true, + 'is_usable' => true, ]); return $server->refresh(); @@ -76,6 +78,21 @@ it('skips ServerConnectionCheckJob when sentinel is live', function () { Queue::assertNotPushed(ServerConnectionCheckJob::class); }); +it('dispatches ServerConnectionCheckJob when sentinel is live but the server is marked unusable', function (string $flag) { + // A Sentinel heartbeat does not restore these flags, so only the SSH check can recover them + $server = createSentinelCheckServer($this->team, Carbon::now()); + $server->settings->update([$flag => false]); + + expect($server->isSentinelLive())->toBeTrue(); + + $job = new ServerManagerJob; + $job->handle(); + + Queue::assertPushed(ServerConnectionCheckJob::class, function ($job) use ($server) { + return $job->server->id === $server->id; + }); +})->with(['is_reachable', 'is_usable']); + it('dispatches ServerConnectionCheckJob when sentinel is not live', function () { $server = createSentinelCheckServer($this->team, Carbon::now()->subMinutes(10));