mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(server): stop marking servers offline after one failed check
A single failed SSH attempt or any unrelated exception in ServerConnectionCheckJob marked the server unreachable and unusable, so deployments failed with "Server is not functional" while the server was online. - Retry a failed SSH attempt once after 3 seconds within the same check. - Mark the server unreachable only after UNREACHABLE_THRESHOLD (2) consecutive failed checks; notifications use the same threshold. - Keep the stored state when the job fails for a reason other than SSH or Docker (database, cache, bugs). - Raise the job timeout to 60 seconds and the overlap lock above it. The old 15 second timeout killed checks before they wrote a result. - Run the Docker and Compose version commands once with a 15 second timeout. A failed Compose check no longer marks the server unusable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bcc07dd47e
commit
69c809f4a2
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user