diff --git a/.ai/lessons.md b/.ai/lessons.md index 70e8451793..ec90c5ea28 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -14,6 +14,7 @@ - The checkout is shared with other sessions. When the full suite fails, rerun each failing file alone and compare with a clean `git archive HEAD` copy before you connect a failure to your change. The dev instance of the checkout runs its live files, so a live repro also runs the uncommitted edits of other sessions: run `git diff` on the code under test before you trust the result. - Run seeders and other artisan commands that write SSH keys in a dev container as `www-data` (`docker exec -u www-data`). Root-owned files in `storage/app/ssh` make every SSH call from the web process fail. - Files written on a server with `tee` get the server umask (027/077 on hardened hosts). Set the mode explicitly (`chmod 644`) for files that a non-root container user reads through a read-only bind mount. +- Each new (non-multiplexed) SSH connection opens a systemd-logind session on the server, and some systemd versions leak closed sessions until logins fail at 8192. Send repeated or scheduled commands through the mux connection; open a fresh connection only when the check must prove a new login. - `sshd` keeps the login shell of an open multiplexed SSH connection. Call `SshMultiplexingHelper::removeMuxFile()` before a live test of SSH or login-shell behavior. - Remote command batches run in `bash -se` (`sh -se` only when the host has no bash). A command that can fail without being an error (such as `cat` of an optional file) stops the batch unless it ends with `|| true`. `Process::fake()` cannot show this, so run such a command once in a real `bash -se` on the target host. - SSH retries replay the full command batch. Only exit code 255 is an SSH transport failure; other exit codes come from the remote command, so do not retry them. Keep batches safe to run twice (for example `docker rm -f || true` before `docker run --name `). diff --git a/CHANGELOG.md b/CHANGELOG.md index 36122766a4..fcdb7dbb1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- Open fewer SSH logins for the server connection check. The Docker and Docker Compose version checks now use the multiplexed SSH connection, so each check opens one new SSH login, not three. Each login makes a systemd-logind session on the server, and some systemd versions keep closed sessions in the `closing` state. After 8192 sessions the server refused new SSH logins with "Maximum number of sessions (8192) reached". To remove the stuck sessions, run `systemctl restart systemd-logind` on the server ([#9148](https://github.com/coollabsio/coolify/issues/9148)) - Fix the Docker install fallback on Ubuntu derivatives such as Pop!_OS and Linux Mint. `get.docker.com` does not support them, and the fallback used a Docker repository with the name of the distribution (`download.docker.com/linux/pop`), which does not exist, so the Docker install failed. The fallback now uses the Ubuntu repository with `UBUNTU_CODENAME` on these systems, and the Debian repository on Raspbian 13 (Trixie), because Docker has no Raspbian Trixie repository ([#9455](https://github.com/coollabsio/coolify/issues/9455)) - Ask for confirmation in a dialog before you change the name of a persistent volume of an application or a database. A new name does not rename the Docker volume: after the next restart, the resource uses a different volume, which Docker creates empty, and the data stays in the old volume. Before, Coolify saved the new name without a warning, so the data looked lost after the restart. The dialog tells you to copy the data into the new volume ([#8816](https://github.com/coollabsio/coolify/issues/8816)) - Show "Not in Compose file" on the applications and databases of a service that you removed from its Docker Compose file, and do not count them in the service status. Before, each removed part showed as "Exited" and the service stayed degraded. Coolify keeps the settings of a removed part: if you add the service back under the same name, Coolify uses the part again. To remove the part, delete it in its settings. A part that a Compose error created, for example a `ports:` block at the wrong indentation, is also marked. Existing parts are marked at the next deployment or save of the service ([#9591](https://github.com/coollabsio/coolify/issues/9591)) diff --git a/app/Jobs/ServerConnectionCheckJob.php b/app/Jobs/ServerConnectionCheckJob.php index a641dfdc11..fc4ff11ac1 100644 --- a/app/Jobs/ServerConnectionCheckJob.php +++ b/app/Jobs/ServerConnectionCheckJob.php @@ -50,8 +50,7 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue private ?string $ownerMarker = null; public function __construct( - public Server $server, - public bool $disableMux = true + public Server $server ) {} public function middleware(): array @@ -304,11 +303,14 @@ class ServerConnectionCheckJob implements ShouldBeEncrypted, ShouldQueue /** * Run one Docker command with a single attempt. Returns null when it fails or times out. + * The command goes through the multiplexed connection: the probe already proved a fresh login, + * and each new SSH connection opens a systemd-logind session on the host. Some systemd versions + * leak closed sessions until the host refuses new logins at 8192 sessions. */ private function runDockerCommand(string $command): ?string { try { - $process = $this->runSsh($command, self::DOCKER_TIMEOUT_SECONDS, disableMultiplexing: $this->disableMux); + $process = $this->runSsh($command, self::DOCKER_TIMEOUT_SECONDS); } catch (\Throwable $e) { Log::debug('ServerConnectionCheck: Docker command failed', [ 'server_id' => $this->server->id, diff --git a/app/Models/Server.php b/app/Models/Server.php index 5129fe5ad0..4e07d9818c 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -2241,7 +2241,7 @@ $siteAddress { return false; } - (new ServerConnectionCheckJob($this, disableMux: false))->handle(); + (new ServerConnectionCheckJob($this))->handle(); $this->settings->refresh(); return $this->isFunctional(); diff --git a/tests/Feature/DockerVersionStorageTest.php b/tests/Feature/DockerVersionStorageTest.php index 74a5a29b46..88cf0417f6 100644 --- a/tests/Feature/DockerVersionStorageTest.php +++ b/tests/Feature/DockerVersionStorageTest.php @@ -125,7 +125,7 @@ it('records docker and compose versions during connection checks', function () { ), ]); - (new ServerConnectionCheckJob($server, disableMux: false))->handle(); + (new ServerConnectionCheckJob($server))->handle(); $settings = $server->settings->fresh(); diff --git a/tests/Feature/ServerConnectionCheckIsolationTest.php b/tests/Feature/ServerConnectionCheckIsolationTest.php index 506b80937f..4d397ef1fa 100644 --- a/tests/Feature/ServerConnectionCheckIsolationTest.php +++ b/tests/Feature/ServerConnectionCheckIsolationTest.php @@ -40,7 +40,7 @@ it('never attempts SSH to a placeholder address', function (string $placeholderI Process::fake(); - (new ServerConnectionCheckJob($server, disableMux: false))->handle(); + (new ServerConnectionCheckJob($server))->handle(); Process::assertNothingRan(); expect($server->fresh()->unreachable_count)->toBe(0) @@ -78,7 +78,7 @@ it('does not call cloud provider APIs during an SSH connection check', function ), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); Http::assertNothingSent(); }); @@ -96,7 +96,7 @@ it('resets unreachable_count after a successful connection check', function () { ), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); expect($server->fresh()->unreachable_count)->toBe(0); }); @@ -111,8 +111,8 @@ it('logs the checking node and ssh error only when the connection state changes' '*' => Process::result(errorOutput: 'ssh: connect to host 203.0.113.10 port 22: Connection refused', exitCode: 255), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); $log = file_get_contents($logPath); @unlink($logPath); @@ -131,7 +131,7 @@ it('keeps the server online after one failed SSH check', function () { '*' => Process::result(errorOutput: 'Connection timed out', exitCode: 255), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); expect($server->fresh()->unreachable_count)->toBe(1) ->and((bool) $server->settings->fresh()->is_reachable)->toBeTrue() @@ -146,7 +146,7 @@ it('marks the server offline after consecutive failed SSH checks', function () { ]); for ($attempt = 0; $attempt < ServerConnectionCheckJob::UNREACHABLE_THRESHOLD; $attempt++) { - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); } expect($server->fresh()->unreachable_count)->toBe(ServerConnectionCheckJob::UNREACHABLE_THRESHOLD) @@ -165,7 +165,7 @@ it('does not mark the server offline when the check fails for a reason other tha // 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(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); Server::flushEventListeners(); expect($server->fresh()->unreachable_count)->toBe(1) @@ -186,7 +186,7 @@ it('retries a failed SSH attempt once within the same check', function () { '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); Sleep::assertSleptTimes(1); Process::assertRanTimes(fn ($process) => str_contains(remoteProcessScript($process), 'ls -la /'), 2); @@ -202,7 +202,7 @@ it('counts one failed check when both SSH attempts fail', function () { '*' => Process::result(errorOutput: 'Connection timed out', exitCode: 255), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); Sleep::assertSleptTimes(1); Process::assertRanTimes(fn ($process) => str_contains(remoteProcessScript($process), 'ls -la /'), 2); @@ -218,7 +218,7 @@ it('does not retry the Docker check within the same check', function () { '*' => Process::result(exitCode: 0), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); Process::assertRanTimes(fn ($process) => str_contains(remoteProcessScript($process), 'docker version'), 1); expect((bool) $server->settings->fresh()->is_reachable)->toBeTrue() @@ -232,7 +232,7 @@ it('keeps the server usable when only the compose version check fails', function '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); expect((bool) $server->settings->fresh()->is_usable)->toBeTrue(); }); @@ -256,7 +256,7 @@ it('stops managing a server when its owner marker names a newer claim of another '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); $server->refresh(); expect($server->isTransferredAway())->toBeTrue() @@ -276,7 +276,7 @@ it('keeps managing a server when its owner marker is its own claim', function () '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), ]); - (new ServerConnectionCheckJob($server->fresh(), disableMux: false))->handle(); + (new ServerConnectionCheckJob($server->fresh()))->handle(); $server->refresh(); expect($server->isTransferredAway())->toBeFalse() @@ -298,3 +298,21 @@ it('keeps the owner marker read elevated in the SSH probe on a non-root server', 'sudo cat '.ServerTransferClaimer::OWNER_MARKER_PATH.' 2>/dev/null || sudo true; ls -la / > /dev/null', ]); }); + +it('runs the Docker checks through the multiplexed connection and only the probe as a new login', function () { + config(['constants.ssh.mux_enabled' => true]); + $server = createServerForConnectionIsolationTest(['ip' => '203.0.113.10']); + fakeRemoteProcesses([ + '*compose version*' => Process::result(output: 'v2.32.4', exitCode: 0), + '*' => Process::result(output: '{"Server":{"Version":"29.4.3"}}', exitCode: 0), + ]); + + (new ServerConnectionCheckJob($server->fresh()))->handle(); + + $usesMux = fn (string $remoteCommand) => fn ($process) => str_contains(remoteProcessScript($process), $remoteCommand) + && str_contains(remoteProcessScript($process), 'ControlPath='); + Process::assertDidntRun($usesMux('ls -la /')); + Process::assertRan($usesMux('docker version --format json')); + Process::assertRan($usesMux('docker compose version --short')); + expect((bool) $server->settings->fresh()->is_usable)->toBeTrue(); +});