diff --git a/.ai/lessons.md b/.ai/lessons.md index a3027b4200..33185493cb 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -39,6 +39,8 @@ - SQLite returns `0`/`1` for boolean columns without a `boolean` cast, while Postgres returns real booleans. A strict `=== false` check on such a column never matches in tests, so tests can pass on behavior that Postgres blocks. Add the cast or compare loosely. - SQLite does not enforce `varchar(255)` lengths, while Postgres rejects a longer value (`SQLSTATE[22001]`). Tests cannot show a too-long name, so assert the length or the exact value of generated strings. +- `docker compose up` gives a recreated container the old mount at each path that the image declares with `VOLUME`, also a named volume or bind mount that the Compose file no longer has. Remove the old container first when a mount must go (`composeContainersWithRemovedVolumes()`); do not use `--renew-anon-volumes`, because it deletes the data of anonymous volumes. + - The Compose parsers (`parse()`, `applicationParser`, `serviceParser`) run on page mounts, refreshes, and deployments. Do not dispatch jobs or run server commands from them; let the caller that changes data start the work. Code that only needs the service names reads `docker_compose_raw` (`Application::composeServicesWithoutParse()`). - MCP tools that call REST routes remove response keys from `BuildsResponse::$sensitiveKeys` (for example `content`, `client_id`, `logs`) when the token has no `read:sensitive`, and `CallsRestApi::$blockedRestPaths` blocks some routes. Check both lists when you name the fields of a new API response or add an MCP tool for a route. diff --git a/app/Actions/Service/DeployServiceApplication.php b/app/Actions/Service/DeployServiceApplication.php index 3893f11d80..927d3be6cf 100644 --- a/app/Actions/Service/DeployServiceApplication.php +++ b/app/Actions/Service/DeployServiceApplication.php @@ -61,6 +61,7 @@ class DeployServiceApplication } // No stdin for Compose prompts: they would read the next script lines (see StartService). $upCommand .= " {$safeComposeServiceName} < /dev/null"; + $commands->push(...StartService::removeContainersWithRemovedVolumesCommands($service, $composeServiceName)); $commands->push('echo Starting service container.'); $commands->push($upCommand); diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index e0d80d759b..0e8dd32879 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -7,6 +7,7 @@ use App\Events\ServiceStartFinished; use App\Jobs\NotifyResourceStoppedJob; use App\Jobs\ServerFilesFromServerJob; use App\Models\LocalFileVolume; +use App\Models\LocalPersistentVolume; use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; @@ -63,6 +64,7 @@ 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)); $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 @@ -110,6 +112,41 @@ class StartService return "{$connectedNetworks} | grep -qxF {$network} || docker network connect --alias {$containerName} {$network} {$containerName} || echo {$warning}"; } + /** + * Removes the containers that `docker compose up` would give a volume that the user deleted + * in Coolify (see composeContainersWithRemovedVolumes()). + * + * @param string|null $composeServiceName only this Compose service, when `up` starts only it + * @return list + */ + public static function removeContainersWithRemovedVolumesCommands(Service $service, ?string $composeServiceName = null): array + { + $server = $service->server; + $projectMounts = (string) instant_remote_process([composeProjectMountsCommand($service->uuid)], $server, false, maxAttempts: 1); + if (trim($projectMounts) === '') { + return []; + } + $services = data_get(parseDockerComposeYaml((string) $service->docker_compose), 'services'); + $services = is_array($services) ? $services : []; + if ($composeServiceName !== null) { + $services = array_intersect_key($services, [$composeServiceName => true]); + } + $storageNames = LocalPersistentVolume::query() + ->where(fn ($query) => $query->where('resource_type', (new ServiceApplication)->getMorphClass())->whereIn('resource_id', $service->applications()->select('id'))) + ->orWhere(fn ($query) => $query->where('resource_type', (new ServiceDatabase)->getMorphClass())->whereIn('resource_id', $service->databases()->select('id'))) + ->pluck('name') + ->all(); + $commands = []; + foreach (composeContainersWithRemovedVolumes($projectMounts, $services, $service->uuid, $storageNames) as $containerName => $volumeName) { + $commands[] = 'echo '.escapeshellarg("Removing container {$containerName}, so that it does not keep the deleted volume {$volumeName}."); + // Without --time, Docker waits the stop_grace_period of the Compose file, as Compose does. + $commands[] = 'docker stop '.escapeshellarg($containerName).' || true'; + $commands[] = dockerRemoveCommand($containerName); + } + + return $commands; + } + /** * Queues one sync of the file storages with the server for each resource of the service that has * file storages. The sync marks each storage as a file or a directory, as it is on the server. diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 86d24c4bee..7c6b6fd144 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -1148,6 +1148,34 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue ->values(); } + /** + * Removes the containers that `docker compose up` would give a volume that the user deleted + * in Coolify (see composeContainersWithRemovedVolumes()). + */ + private function remove_containers_with_removed_volumes(): void + { + $this->execute_remote_command([ + composeProjectMountsCommand($this->application->uuid), + 'hidden' => true, + 'ignore_errors' => true, + 'save' => 'compose_project_mounts', + 'append' => false, + ]); + $services = data_get(parseDockerComposeYaml((string) $this->generated_docker_compose), 'services'); + $containers = composeContainersWithRemovedVolumes( + (string) $this->saved_outputs->get('compose_project_mounts', ''), + is_array($services) ? $services : [], + $this->application->uuid, + $this->application->persistentStorages()->pluck('name')->all(), + ); + foreach ($containers as $containerName => $volumeName) { + $this->application_deployment_queue->addLogEntry("Removing container {$containerName}, so that it does not keep the deleted volume {$volumeName}."); + // Without --time, Docker waits the stop_grace_period of the Compose file, as Compose does. + $this->execute_remote_command(['docker stop '.escapeshellarg($containerName), 'hidden' => true, 'ignore_errors' => true]); + $this->removeContainerWithTimeout($containerName); + } + } + /** * Starts the Compose containers. Content files are written first, because `docker compose up` * creates a missing bind source as an empty directory. @@ -1155,6 +1183,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue private function start_docker_compose_services(): void { $this->write_missing_content_files(); + $this->remove_containers_with_removed_volumes(); $server_workdir = $this->application->workdir(); if ($this->application->settings->is_raw_compose_deployment_enabled) { diff --git a/app/Livewire/Project/Shared/Storages/All.php b/app/Livewire/Project/Shared/Storages/All.php index 69f758bba3..8812973fb0 100644 --- a/app/Livewire/Project/Shared/Storages/All.php +++ b/app/Livewire/Project/Shared/Storages/All.php @@ -24,7 +24,7 @@ class All extends Component /** * Editable form state keyed by storage id. * - * @var array + * @var array}> */ public array $forms = []; @@ -89,6 +89,7 @@ class All extends Component $this->pendingRename = null; $this->rebuildForms(); + $this->rebuildStaleVolumeContainers(); $this->rebuildVolumeBackupMeta(); } @@ -247,11 +248,47 @@ class All extends Component 'canDeleteToApplyDriverOptions' => $this->canUpdate && $ignoresDriverOptions, 'replacedExternalVolume' => $this->isComposeOrService ? $storage->replacedExternalComposeVolume() : null, 'ignoresDriverOptions' => $ignoresDriverOptions, + 'usedByContainers' => [], ]; } $this->forms = $forms; } + /** + * Finds the containers that still use a stale volume. Docker Compose can keep such a volume + * in a container after the line was removed from the Compose file. After the entry is + * deleted, the next deployment starts that container without the data (#6875). + */ + private function rebuildStaleVolumeContainers(): void + { + $staleNames = collect($this->forms)->filter(fn (array $form) => $form['canDeleteStale'])->pluck('name'); + if ($staleNames->isEmpty()) { + return; + } + $server = $this->resource instanceof Application + ? $this->resource->destination->server + : $this->resource->service->server; + try { + $output = (string) instant_remote_process(["docker ps -a --no-trunc --format '{{.Names}} {{.Mounts}}'"], $server, false, maxAttempts: 1); + } catch (\Throwable) { + return; + } + $containers = collect(preg_split('/\R/', trim($output)))->filter()->mapWithKeys(function (string $line) { + [$name, $mounts] = array_pad(explode(' ', $line, 2), 2, ''); + + return [$name => explode(',', $mounts)]; + }); + foreach ($this->forms as $id => $form) { + if ($form['canDeleteStale']) { + $this->forms[$id]['usedByContainers'] = $containers + ->filter(fn (array $mounts) => in_array($form['name'], $mounts, true)) + ->keys() + ->values() + ->all(); + } + } + } + private function rebuildVolumeBackupMeta(): void { $this->volumeBackupMeta = []; diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 62be1532ad..a33717c386 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -507,6 +507,89 @@ function dockerRemoveCommand(string $container): string return dockerCommandIgnoringError($command, 'No such container:'); } +/** + * Lists the containers of a Compose project, one JSON object per line, for composeContainersWithRemovedVolumes(). + */ +function composeProjectMountsCommand(string $projectName): string +{ + $format = '{"name":{{json .Name}},"service":{{json (index .Config.Labels "com.docker.compose.service")}},"imageVolumes":{{json .Config.Volumes}},"mounts":{{json .Mounts}}}'; + + return 'docker ps -aq --filter '.escapeshellarg("label=com.docker.compose.project={$projectName}") + .' | xargs -r docker inspect --format '.escapeshellarg($format).' 2>/dev/null || true'; +} + +/** + * `docker compose up` gives a recreated container the mount of the old container at each path + * that the image declares with VOLUME. This also keeps a volume that the user deleted in Coolify, + * so its old data comes back (#6875). Returns the containers to remove before `up`. + * + * Only a volume that Coolify made (its name starts with "{uuid}_") and that has no storage row + * anymore counts. A volume that is only removed from the Compose file still has its row, and an + * external volume or a bind mount is not Coolify's, so Compose keeps using them as before. + * + * @param string $projectMounts output of composeProjectMountsCommand() + * @param array $composeServices the services of the Compose file that `up` starts + * @param list $storageNames names of the storage rows that the resource still has + * @return array container name => volume that the user deleted + */ +function composeContainersWithRemovedVolumes(string $projectMounts, array $composeServices, string $uuid, array $storageNames): array +{ + $normalize = fn (string $path): string => rtrim($path, '/') ?: '/'; + $containers = []; + foreach (preg_split('/\R/', trim($projectMounts)) as $line) { + $container = json_decode($line, true); + $service = data_get($container, 'service'); + if (! is_array($container) || ! is_string($service) || ! is_array(data_get($composeServices, $service))) { + continue; + } + $targets = composeServiceVolumeTargets(data_get($composeServices, "{$service}.volumes", [])); + if ($targets === null) { + continue; + } + $imageVolumes = array_map($normalize, array_keys(data_get($container, 'imageVolumes') ?? [])); + foreach (data_get($container, 'mounts') ?? [] as $mount) { + $name = (string) data_get($mount, 'Name'); + $path = $normalize((string) data_get($mount, 'Destination')); + if (data_get($mount, 'Type') !== 'volume' || ! str_starts_with($name, "{$uuid}_") || in_array($name, $storageNames, true) + || ! in_array($path, $imageVolumes, true) || in_array($path, $targets, true)) { + continue; + } + $containers[ltrim((string) data_get($container, 'name'), '/')] = $name; + } + } + + return $containers; +} + +/** + * The container paths of the volumes of a Compose service, or null when a path is not literal + * (for example `${DATA_PATH}`). + * + * @return list|null + */ +function composeServiceVolumeTargets(mixed $volumes): ?array +{ + $targets = []; + foreach (is_array($volumes) ? $volumes : [] as $volume) { + if (is_array($volume)) { + $target = data_get($volume, 'target'); + } else { + // source:target:mode, where the source can contain colons, for example ${DATA:-/data}. + $parts = explode(':', (string) $volume); + if (count($parts) > 1 && preg_match('/^[a-zA-Z,]+$/', end($parts))) { + array_pop($parts); + } + $target = end($parts); + } + if (! is_string($target) || ! str_starts_with($target, '/')) { + return null; + } + $targets[] = rtrim($target, '/') ?: '/'; + } + + return $targets; +} + function dockerNetworkRemoveCommand(string $network): string { $command = 'docker network rm '.escapeShellValue($network); diff --git a/resources/views/livewire/project/shared/storages/all.blade.php b/resources/views/livewire/project/shared/storages/all.blade.php index 98253ef137..7467be9dea 100644 --- a/resources/views/livewire/project/shared/storages/all.blade.php +++ b/resources/views/livewire/project/shared/storages/all.blade.php @@ -177,7 +177,10 @@ $form['replacedExternalVolume'] ? 'The next deployment uses the external volume \''.$form['replacedExternalVolume'].'\' instead of this volume.' : null, - ]))" confirmationText="{{ $form['name'] }}" + ]))" + :warningMessage="$form['usedByContainers'] !== [] + ? e('This volume is still in use by: '.implode(', ', $form['usedByContainers']).'. After you delete this entry, the next deployment starts the resource without the data in this volume. Back up the data first if you need it.') + : null" confirmationText="{{ $form['name'] }}" confirmationLabel="Please confirm by entering the Storage Name below" shortConfirmationLabel="Storage Name"> diff --git a/tests/Unit/ComposeContainersWithRemovedVolumesTest.php b/tests/Unit/ComposeContainersWithRemovedVolumesTest.php new file mode 100644 index 0000000000..db822d1242 --- /dev/null +++ b/tests/Unit/ComposeContainersWithRemovedVolumesTest.php @@ -0,0 +1,92 @@ + "/{$service}-abc123", + 'service' => $service, + 'imageVolumes' => $imageVolumes === [] ? null : array_fill_keys($imageVolumes, new stdClass), + 'mounts' => $mounts, + ]); +} + +function namedVolumeMount(string $name, string $destination): array +{ + return ['Type' => 'volume', 'Name' => $name, 'Source' => "/var/lib/docker/volumes/{$name}/_data", 'Destination' => $destination]; +} + +it('finds a container whose deleted volume Compose would keep', function () { + $projectMounts = inspectedComposeContainer('db', ['/var/lib/postgresql/data'], [ + namedVolumeMount('abc123_db-data', '/var/lib/postgresql/data'), + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['db' => ['image' => 'postgres:17']], 'abc123', ['abc123_other'])) + ->toBe(['db-abc123' => 'abc123_db-data']); +}); + +it('keeps a volume that is only removed from the Compose file, because its storage row still exists', function () { + $projectMounts = inspectedComposeContainer('db', ['/var/lib/postgresql/data'], [ + namedVolumeMount('abc123_db-data', '/var/lib/postgresql/data'), + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['db' => ['image' => 'postgres:17']], 'abc123', ['abc123_db-data']))->toBe([]); +}); + +it('keeps external volumes and bind mounts, because Coolify did not make them', function (array $mount) { + $projectMounts = inspectedComposeContainer('app', ['/data/'], [$mount]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['app' => ['image' => 'app', 'volumes' => ['/other:/other']]], 'abc123', []))->toBe([]); +})->with([ + 'external volume' => [namedVolumeMount('shared-data', '/data')], + 'bind mount' => [['Type' => 'bind', 'Source' => '/data/coolify/services/abc123/data', 'Destination' => '/data']], +]); + +it('keeps a container when the Compose file still mounts something at the path', function (mixed $volume) { + $projectMounts = inspectedComposeContainer('db', ['/var/lib/postgresql/data'], [ + namedVolumeMount('abc123_db-data', '/var/lib/postgresql/data'), + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['db' => ['volumes' => [$volume]]], 'abc123', []))->toBe([]); +})->with([ + 'short syntax' => 'abc123_db-data:/var/lib/postgresql/data', + 'short syntax with mode' => 'other-data:/var/lib/postgresql/data/:rw', + 'source with a default value' => '${DATA_PATH:-/srv/db}:/var/lib/postgresql/data', + 'anonymous volume' => '/var/lib/postgresql/data', + 'long syntax' => [['type' => 'volume', 'source' => 'other', 'target' => '/var/lib/postgresql/data']], +]); + +it('keeps a container whose anonymous volume Compose keeps on purpose', function () { + $projectMounts = inspectedComposeContainer('db', ['/var/lib/postgresql/data'], [ + namedVolumeMount(str_repeat('a1', 32), '/var/lib/postgresql/data'), + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['db' => ['image' => 'postgres:17']], 'abc123', []))->toBe([]); +}); + +it('keeps a container when its image declares no volume at the removed path', function () { + $projectMounts = inspectedComposeContainer('app', [], [ + namedVolumeMount('abc123_app-data', '/app/data'), + ['Type' => 'bind', 'Source' => '/data/coolify/secrets/s1', 'Destination' => '/run/secrets/s1'], + ['Type' => 'tmpfs', 'Destination' => '/tmp'], + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['app' => ['image' => 'app']], 'abc123', []))->toBe([]); +}); + +it('keeps containers of services that up does not start, and paths it cannot read', function () { + $projectMounts = implode("\n", [ + inspectedComposeContainer('old-service', ['/data'], [namedVolumeMount('abc123_old', '/data')]), + inspectedComposeContainer('web', ['/data'], [namedVolumeMount('abc123_web', '/data')]), + 'Error: No such object', + ]); + + expect(composeContainersWithRemovedVolumes($projectMounts, ['web' => ['volumes' => ['web-data:${DATA_TARGET}']]], 'abc123', []))->toBe([]); +});