mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-04 10:42:23 +00:00
fix(storage): stop a deleted volume from coming back after a redeploy
Docker Compose gives a recreated container the old mount at each path that the image declares with VOLUME, also a volume deleted in Coolify. Before `up`, remove only containers that still mount a Coolify volume whose storage entry was deleted. The stale volume delete dialog now warns when a container still uses the volume. Closes #6875 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5c71067481
commit
780d67ccbb
8 files changed
+286
-2
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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<string>
|
||||
*/
|
||||
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.
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -24,7 +24,7 @@ class All extends Component
|
||||
/**
|
||||
* Editable form state keyed by storage id.
|
||||
*
|
||||
* @var array<int|string, array{name: string, mountPath: string, isPreviewSuffixEnabled: bool, isReadOnly: bool, isShared: bool, canDeleteStale: bool, replacedExternalVolume: ?string, ignoresDriverOptions: bool, canDeleteToApplyDriverOptions: bool}>
|
||||
* @var array<int|string, array{name: string, mountPath: string, isPreviewSuffixEnabled: bool, isReadOnly: bool, isShared: bool, canDeleteStale: bool, replacedExternalVolume: ?string, ignoresDriverOptions: bool, canDeleteToApplyDriverOptions: bool, usedByContainers: list<string>}>
|
||||
*/
|
||||
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 = [];
|
||||
|
||||
@@ -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<string, mixed> $composeServices the services of the Compose file that `up` starts
|
||||
* @param list<string> $storageNames names of the storage rows that the resource still has
|
||||
* @return array<string, string> 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<string>|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);
|
||||
|
||||
@@ -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">
|
||||
<x-slot:trigger>
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
<?php
|
||||
|
||||
use Tests\TestCase;
|
||||
|
||||
uses(TestCase::class);
|
||||
|
||||
/**
|
||||
* `docker compose up` gives a recreated container the old mount at each path that the image
|
||||
* declares with VOLUME, also a volume that the user deleted in Coolify (#6875).
|
||||
*/
|
||||
function inspectedComposeContainer(string $service, array $imageVolumes, array $mounts): string
|
||||
{
|
||||
return json_encode([
|
||||
'name' => "/{$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([]);
|
||||
});
|
||||
Reference in new issue
Block a user