Fix SSH multiplexing contention for concurrent scheduled tasks (#6736) (#7503)

This commit is contained in:
Andras Bacsai 2025-12-05 11:07:43 +01:00 committed by GitHub
commit b55415f7e6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 135 additions and 19 deletions

View file

@ -149,7 +149,7 @@ public static function generateScpCommand(Server $server, string $source, string
return $scp_command;
}
public static function generateSshCommand(Server $server, string $command)
public static function generateSshCommand(Server $server, string $command, bool $disableMultiplexing = false)
{
if ($server->settings->force_disabled) {
throw new \RuntimeException('Server is disabled.');
@ -168,7 +168,7 @@ public static function generateSshCommand(Server $server, string $command)
$ssh_command = "timeout $timeout ssh ";
$multiplexingSuccessful = false;
if (self::isMultiplexingEnabled()) {
if (! $disableMultiplexing && self::isMultiplexingEnabled()) {
try {
$multiplexingSuccessful = self::ensureMultiplexedConnection($server);
if ($multiplexingSuccessful) {

View file

@ -121,7 +121,7 @@ public function handle(): void
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env | grep POSTGRES_";
$envs = instant_remote_process($commands, $this->server);
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
$user = $envs->filter(function ($env) {
@ -152,7 +152,7 @@ public function handle(): void
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env | grep MYSQL_";
$envs = instant_remote_process($commands, $this->server);
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
$rootPassword = $envs->filter(function ($env) {
@ -175,7 +175,7 @@ public function handle(): void
$this->container_name = "{$this->database->name}-$serviceUuid";
$this->directory_name = $serviceName.'-'.$this->container_name;
$commands[] = "docker exec $this->container_name env";
$envs = instant_remote_process($commands, $this->server);
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$envs = str($envs)->explode("\n");
$rootPassword = $envs->filter(function ($env) {
return str($env)->startsWith('MARIADB_ROOT_PASSWORD=');
@ -217,7 +217,7 @@ public function handle(): void
try {
$commands = [];
$commands[] = "docker exec $this->container_name env | grep MONGO_INITDB_";
$envs = instant_remote_process($commands, $this->server);
$envs = instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
if (filled($envs)) {
$envs = str($envs)->explode("\n");
@ -508,7 +508,7 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi
}
}
}
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout);
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
if ($this->backup_output === '') {
$this->backup_output = null;
@ -537,7 +537,7 @@ private function backup_standalone_postgresql(string $database): void
}
$commands[] = $backupCommand;
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout);
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
if ($this->backup_output === '') {
$this->backup_output = null;
@ -560,7 +560,7 @@ private function backup_standalone_mysql(string $database): void
$escapedDatabase = escapeshellarg($database);
$commands[] = "docker exec $this->container_name mysqldump -u root -p\"{$this->database->mysql_root_password}\" $escapedDatabase > $this->backup_location";
}
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout);
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
if ($this->backup_output === '') {
$this->backup_output = null;
@ -583,7 +583,7 @@ private function backup_standalone_mariadb(string $database): void
$escapedDatabase = escapeshellarg($database);
$commands[] = "docker exec $this->container_name mariadb-dump -u root -p\"{$this->database->mariadb_root_password}\" $escapedDatabase > $this->backup_location";
}
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout);
$this->backup_output = instant_remote_process($commands, $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->backup_output = trim($this->backup_output);
if ($this->backup_output === '') {
$this->backup_output = null;
@ -614,7 +614,7 @@ private function add_to_error_output($output): void
private function calculate_size()
{
return instant_remote_process(["du -b $this->backup_location | cut -f1"], $this->server, false);
return instant_remote_process(["du -b $this->backup_location | cut -f1"], $this->server, false, false, null, disableMultiplexing: true);
}
private function upload_to_s3(): void
@ -637,9 +637,9 @@ private function upload_to_s3(): void
$fullImageName = $this->getFullImageName();
$containerExists = instant_remote_process(["docker ps -a -q -f name=backup-of-{$this->backup_log_uuid}"], $this->server, false);
$containerExists = instant_remote_process(["docker ps -a -q -f name=backup-of-{$this->backup_log_uuid}"], $this->server, false, false, null, disableMultiplexing: true);
if (filled($containerExists)) {
instant_remote_process(["docker rm -f backup-of-{$this->backup_log_uuid}"], $this->server, false);
instant_remote_process(["docker rm -f backup-of-{$this->backup_log_uuid}"], $this->server, false, false, null, disableMultiplexing: true);
}
if (isDev()) {
@ -661,7 +661,7 @@ private function upload_to_s3(): void
$commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc alias set temporary {$escapedEndpoint} {$escapedKey} {$escapedSecret}";
$commands[] = "docker exec backup-of-{$this->backup_log_uuid} mc cp $this->backup_location temporary/$bucket{$this->backup_dir}/";
instant_remote_process($commands, $this->server);
instant_remote_process($commands, $this->server, true, false, null, disableMultiplexing: true);
$this->s3_uploaded = true;
} catch (\Throwable $e) {
@ -670,7 +670,7 @@ private function upload_to_s3(): void
throw $e;
} finally {
$command = "docker rm -f backup-of-{$this->backup_log_uuid}";
instant_remote_process([$command], $this->server);
instant_remote_process([$command], $this->server, true, false, null, disableMultiplexing: true);
}
}

View file

@ -139,7 +139,9 @@ public function handle(): void
if (count($this->containers) == 1 || str_starts_with($containerName, $this->task->container.'-'.$this->resource->uuid)) {
$cmd = "sh -c '".str_replace("'", "'\''", $this->task->command)."'";
$exec = "docker exec {$containerName} {$cmd}";
$this->task_output = instant_remote_process([$exec], $this->server, true, false, $this->timeout);
// Disable SSH multiplexing to prevent race conditions when multiple tasks run concurrently
// See: https://github.com/coollabsio/coolify/issues/6736
$this->task_output = instant_remote_process([$exec], $this->server, true, false, $this->timeout, disableMultiplexing: true);
$this->task_log->update([
'status' => 'success',
'message' => $this->task_output,

View file

@ -118,7 +118,7 @@ function () use ($server, $command_string) {
);
}
function instant_remote_process(Collection|array $command, Server $server, bool $throwError = true, bool $no_sudo = false, ?int $timeout = null): ?string
function instant_remote_process(Collection|array $command, Server $server, bool $throwError = true, bool $no_sudo = false, ?int $timeout = null, bool $disableMultiplexing = false): ?string
{
$command = $command instanceof Collection ? $command->toArray() : $command;
@ -129,8 +129,8 @@ function instant_remote_process(Collection|array $command, Server $server, bool
$effectiveTimeout = $timeout ?? config('constants.ssh.command_timeout');
return \App\Helpers\SshRetryHandler::retry(
function () use ($server, $command_string, $effectiveTimeout) {
$sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string);
function () use ($server, $command_string, $effectiveTimeout, $disableMultiplexing) {
$sshCommand = SshMultiplexingHelper::generateSshCommand($server, $command_string, $disableMultiplexing);
$process = Process::timeout($effectiveTimeout)->run($sshCommand);
$output = trim($process->output());

View file

@ -0,0 +1,114 @@
<?php
namespace Tests\Unit;
use App\Helpers\SshMultiplexingHelper;
use Tests\TestCase;
/**
* Tests for SSH multiplexing disable functionality.
*
* These tests verify the parameter signatures for the disableMultiplexing feature
* which prevents race conditions when multiple scheduled tasks run concurrently.
*
* @see https://github.com/coollabsio/coolify/issues/6736
*/
class SshMultiplexingDisableTest extends TestCase
{
public function test_generate_ssh_command_method_exists()
{
$this->assertTrue(
method_exists(SshMultiplexingHelper::class, 'generateSshCommand'),
'generateSshCommand method should exist'
);
}
public function test_generate_ssh_command_accepts_disable_multiplexing_parameter()
{
$reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'generateSshCommand');
$parameters = $reflection->getParameters();
// Should have at least 3 parameters: $server, $command, $disableMultiplexing
$this->assertGreaterThanOrEqual(3, count($parameters));
$disableMultiplexingParam = $parameters[2] ?? null;
$this->assertNotNull($disableMultiplexingParam);
$this->assertEquals('disableMultiplexing', $disableMultiplexingParam->getName());
$this->assertTrue($disableMultiplexingParam->isDefaultValueAvailable());
$this->assertFalse($disableMultiplexingParam->getDefaultValue());
}
public function test_disable_multiplexing_parameter_is_boolean_type()
{
$reflection = new \ReflectionMethod(SshMultiplexingHelper::class, 'generateSshCommand');
$parameters = $reflection->getParameters();
$disableMultiplexingParam = $parameters[2] ?? null;
$this->assertNotNull($disableMultiplexingParam);
$type = $disableMultiplexingParam->getType();
$this->assertNotNull($type);
$this->assertEquals('bool', $type->getName());
}
public function test_instant_remote_process_accepts_disable_multiplexing_parameter()
{
$this->assertTrue(
function_exists('instant_remote_process'),
'instant_remote_process function should exist'
);
$reflection = new \ReflectionFunction('instant_remote_process');
$parameters = $reflection->getParameters();
// Find the disableMultiplexing parameter
$disableMultiplexingParam = null;
foreach ($parameters as $param) {
if ($param->getName() === 'disableMultiplexing') {
$disableMultiplexingParam = $param;
break;
}
}
$this->assertNotNull($disableMultiplexingParam, 'disableMultiplexing parameter should exist');
$this->assertTrue($disableMultiplexingParam->isDefaultValueAvailable());
$this->assertFalse($disableMultiplexingParam->getDefaultValue());
}
public function test_instant_remote_process_disable_multiplexing_is_boolean_type()
{
$reflection = new \ReflectionFunction('instant_remote_process');
$parameters = $reflection->getParameters();
// Find the disableMultiplexing parameter
$disableMultiplexingParam = null;
foreach ($parameters as $param) {
if ($param->getName() === 'disableMultiplexing') {
$disableMultiplexingParam = $param;
break;
}
}
$this->assertNotNull($disableMultiplexingParam);
$type = $disableMultiplexingParam->getType();
$this->assertNotNull($type);
$this->assertEquals('bool', $type->getName());
}
public function test_multiplexing_is_skipped_when_disabled()
{
// This test verifies the logic flow by checking the code path
// When disableMultiplexing is true, the condition `! $disableMultiplexing && self::isMultiplexingEnabled()`
// should evaluate to false, skipping multiplexing entirely
// We verify the condition logic:
// disableMultiplexing = true -> ! true = false -> condition is false -> skip multiplexing
$disableMultiplexing = true;
$this->assertFalse(! $disableMultiplexing, 'When disableMultiplexing is true, negation should be false');
// disableMultiplexing = false -> ! false = true -> condition may proceed
$disableMultiplexing = false;
$this->assertTrue(! $disableMultiplexing, 'When disableMultiplexing is false, negation should be true');
}
}