From 2466ccb8fd76f726f23c528fdade49f1a6ac8414 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 9 Nov 2026 10:18:51 +0100 Subject: [PATCH] fix(service): check images before stopping a service on restart Closes #12485 --- ...es-stayed-down-after-restart-image-pull.md | 6 + app/Actions/Service/StartService.php | 37 +++++- app/Actions/Service/StopService.php | 22 +++- .../Feature/ServiceRestartImageCheckTest.php | 124 ++++++++++++++++++ 4 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 .changes/services-stayed-down-after-restart-image-pull.md create mode 100644 tests/Feature/ServiceRestartImageCheckTest.php diff --git a/.changes/services-stayed-down-after-restart-image-pull.md b/.changes/services-stayed-down-after-restart-image-pull.md new file mode 100644 index 0000000000..63a93215cd --- /dev/null +++ b/.changes/services-stayed-down-after-restart-image-pull.md @@ -0,0 +1,6 @@ +--- +type: fixed +--- +Services that stopped and stayed down after a `Restart` when an image could not be pulled ([#12485](https://github.com/coollabsio/coolify/issues/12485)) + +- Coolify pulls the missing images before it stops the containers. If an image cannot be pulled, the restart fails and the service keeps running diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index ac286d9619..c33c3ac845 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -37,9 +37,6 @@ class StartService // Compose recreates the containers, so a status check can find them stopped for a moment. $service->applications()->get()->concat($service->databases()->get()) ->each(fn (ServiceApplication|ServiceDatabase $part) => NotifyResourceStoppedJob::expectStop($part)); - if ($this->shouldStopBeforeStarting($pullLatestImages, $stopBeforeStart)) { - StopService::run(service: $service, dockerCleanup: false); - } $service->saveComposeConfigs(); $service->isConfigurationChanged(save: true); $service->applications()->get()->each->resetRestartLimit(); @@ -68,7 +65,12 @@ class StartService $commands[] = "echo 'Creating Docker network.'"; $commands[] = "docker network inspect $service->uuid >/dev/null 2>&1 || docker network create --attachable $service->uuid"; } - $commands = array_merge($commands, self::removeContainersWithRemovedVolumesCommands($service)); + if ($this->shouldStopBeforeStarting($pullLatestImages, $stopBeforeStart)) { + // The stop removes every container, so no container keeps a deleted volume. + $commands = array_merge($commands, self::stopAfterImageCheckCommands($service, $compose)); + } else { + $commands = array_merge($commands, self::removeContainersWithRemovedVolumesCommands($service)); + } $commands[] = 'echo Starting service.'; if (data_get($service, 'connect_to_docker_network')) { // Connect the created containers to the predefined network before they start. A container @@ -119,6 +121,33 @@ class StartService return "{$connectedNetworks} | grep -qxF {$network} || docker network connect --alias {$containerName} {$network} {$containerName} || echo {$warning}"; } + /** + * Stops and removes the containers of the service, but only after Compose has every image on the + * server. `up --no-start --no-recreate` pulls (or builds) only the missing images and does not touch + * the running containers. If an image cannot be fetched, the script stops there and the service + * keeps running. + * + * @return list + */ + public static function stopAfterImageCheckCommands(Service $service, string $compose): array + { + $containers = $service->applications()->pluck('name') + ->concat($service->databases()->pluck('name')) + ->map(fn (string $name): string => "{$name}-{$service->uuid}") + ->values() + ->all(); + if ($containers === []) { + return []; + } + + return [ + "echo 'Checking that all images are on the server.'", + "{$compose} up --no-start --no-recreate < /dev/null", + "echo 'Stopping service.'", + ...array_map(fn (string $command): string => "{$command} || true", StopService::stopContainersCommands($containers, $service->server)), + ]; + } + /** * Removes the containers that `docker compose up` would give a volume that the user deleted * in Coolify (see composeContainersWithRemovedVolumes()). diff --git a/app/Actions/Service/StopService.php b/app/Actions/Service/StopService.php index 155e39f706..a9e8c53215 100644 --- a/app/Actions/Service/StopService.php +++ b/app/Actions/Service/StopService.php @@ -79,15 +79,25 @@ class StopService private function stopContainersInParallel(array $containersToStop, Server $server): void { - $timeout = count($containersToStop) > 5 ? 10 : 30; - $commands = []; - $containerList = implode(' ', array_map('escapeshellarg', $containersToStop)); - $commands[] = dockerStopCommand($timeout, $containerList, $server); - $commands[] = "docker rm -f $containerList"; instant_remote_process( - command: $commands, + command: self::stopContainersCommands($containersToStop, $server), server: $server, throwError: false ); } + + /** + * @param list $containersToStop + * @return list + */ + public static function stopContainersCommands(array $containersToStop, Server $server): array + { + $timeout = count($containersToStop) > 5 ? 10 : 30; + $containerList = implode(' ', array_map('escapeshellarg', $containersToStop)); + + return [ + dockerStopCommand($timeout, $containerList, $server), + "docker rm -f $containerList", + ]; + } } diff --git a/tests/Feature/ServiceRestartImageCheckTest.php b/tests/Feature/ServiceRestartImageCheckTest.php new file mode 100644 index 0000000000..b9391821c7 --- /dev/null +++ b/tests/Feature/ServiceRestartImageCheckTest.php @@ -0,0 +1,124 @@ + 0]); + config([ + 'app.maintenance.store' => 'array', + 'constants.ssh.mux_enabled' => false, + ]); + $this->stubDirectory = sys_get_temp_dir().'/service-restart-image-check-'.bin2hex(random_bytes(6)); +}); + +afterEach(function () { + File::deleteDirectory($this->stubDirectory); +}); + +function serviceRestartImageCheckService(string $sshUser): Service +{ + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'user' => $sshUser, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + return Service::factory()->create([ + 'environment_id' => $environment->id, + 'server_id' => $server->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'docker_compose_raw' => "services:\n web:\n image: busybox:latest\n oneshot:\n image: local/oneshot:1\n restart: 'no'\n", + ]); +} + +/** + * Runs the start script the way SSH does (`bash -se` with the script on stdin). The docker stub + * fails the image check (`compose up --no-start`) when $imageMissing is true, as Compose does + * when it cannot pull a missing image. + * + * @return array{successful: bool, calls: list} + */ +function runServiceRestartScript(string $script, string $stubDirectory, bool $imageMissing): array +{ + File::ensureDirectoryExists($stubDirectory); + $log = "{$stubDirectory}/calls.log"; + File::put("{$stubDirectory}/docker", <<<'SH' +#!/bin/sh +echo "docker $*" >> "$STUB_LOG" +case " $* " in + *" compose "*" up --no-start "*) + if [ "$STUB_IMAGE_MISSING" = 1 ]; then echo "pull access denied for local/oneshot" >&2; exit 1; fi + ;; +esac +exit 0 +SH); + File::put("{$stubDirectory}/touch", "#!/bin/sh\nexit 0\n"); + File::put("{$stubDirectory}/sudo", "#!/bin/sh\nexec env \"\$@\"\n"); + foreach (['docker', 'touch', 'sudo'] as $stub) { + chmod("{$stubDirectory}/{$stub}", 0755); + } + + $process = new SymfonyProcess(['bash', '-se'], env: [ + 'PATH' => $stubDirectory.':'.getenv('PATH'), + 'STUB_LOG' => $log, + 'STUB_IMAGE_MISSING' => $imageMissing ? '1' : '0', + ], input: $script."\n", timeout: 30); + $process->run(); + + return [ + 'successful' => $process->isSuccessful(), + 'calls' => File::exists($log) ? explode("\n", trim(File::get($log))) : [], + ]; +} + +it('keeps the containers of a service when an image cannot be fetched during a restart', function (string $sshUser) { + $service = serviceRestartImageCheckService($sshUser); + + $activity = StartService::run($service, stopBeforeStart: true); + + $result = runServiceRestartScript(RemoteProcessCommand::read($activity), $this->stubDirectory, imageMissing: true); + $calls = collect($result['calls']); + expect($result['successful'])->toBeFalse() + ->and($calls->filter(fn (string $call) => str_contains($call, 'up --no-start --no-recreate'))->count())->toBe(1) + ->and($calls->filter(fn (string $call) => str_starts_with($call, 'docker stop') || str_starts_with($call, 'docker rm'))->all())->toBe([]) + ->and($calls->filter(fn (string $call) => str_contains($call, 'up -d'))->all())->toBe([]); +})->with(['root' => 'root', 'non-root' => 'deploy']); + +it('stops the containers of a service only after the image check of a restart', function (string $sshUser) { + $service = serviceRestartImageCheckService($sshUser); + + $activity = StartService::run($service, stopBeforeStart: true); + + $result = runServiceRestartScript(RemoteProcessCommand::read($activity), $this->stubDirectory, imageMissing: false); + $calls = $result['calls']; + $position = fn (string $needle): int|false => collect($calls)->search(fn (string $call) => str_contains($call, $needle)); + expect($result['successful'])->toBeTrue() + ->and($position('up --no-start --no-recreate'))->toBeInt() + ->and($position('up --no-start --no-recreate'))->toBeLessThan($position('docker stop')) + ->and($position('docker stop'))->toBeLessThan($position('up -d')) + ->and($calls)->toContain("docker rm -f web-{$service->uuid} oneshot-{$service->uuid}"); +})->with(['root' => 'root', 'non-root' => 'deploy']);