mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(docker): stop matching containers by numeric resource id labels
Numeric coolify.{type}Id labels can collide with resources on another
Coolify instance managing the same server, so they no longer select or
resolve container owners. Drop the legacyId fallback from container
lookups in DeleteService and ComplexStatusCheck.
Legacy application containers without a UUID label are now resolved by
their compose service name, which starts with the application UUID.
This commit is contained in:
Andras Bacsai
@@ -67,7 +67,7 @@ class DeleteService
|
||||
|
||||
// One sh -c line, so non-root servers run the whole script with sudo. A leading variable
|
||||
// assignment would become "sudo container_ids=...", which sudo rejects.
|
||||
$script = containerIdsByOwnerScript('service', $service->uuid, $filters, legacyExtraFilters: $legacyFilters, legacyId: (int) $service->id)
|
||||
$script = containerIdsByOwnerScript('service', $service->uuid, $filters, legacyExtraFilters: $legacyFilters)
|
||||
.'; [ -z "$container_ids" ] || docker rm -f $container_ids';
|
||||
instant_remote_process(['sh -c '.escapeshellarg($script)], $service->server);
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ class ComplexStatusCheck
|
||||
continue;
|
||||
}
|
||||
}
|
||||
$script = containerIdsByOwnerScript('application', $application->uuid, ['label=coolify.pullRequestId=0'], all: false, legacyId: (int) $application->id)
|
||||
$script = containerIdsByOwnerScript('application', $application->uuid, ['label=coolify.pullRequestId=0'], all: false)
|
||||
.'; [ -z "$container_ids" ] || docker container inspect $container_ids --format \'{{json .}}\'';
|
||||
$containers = instant_remote_process(['sh -c '.escapeshellarg($script)], $server, false);
|
||||
$containers = format_docker_command_output_to_json($containers);
|
||||
|
||||
@@ -65,8 +65,7 @@ function caddyTrafficAppKey(string $uuid, ?string $serviceName = null): string
|
||||
* New containers carry `coolify.{type}Uuid`. Containers created before that label carry a
|
||||
* numeric `coolify.{type}Id` instead. Ids change when a server moves to another instance, so
|
||||
* for those containers the owner is read from the compose project (Swarm: stack namespace),
|
||||
* which is the resource UUID for deployments since mid-2024. Use resolveContainerOwner() to
|
||||
* also fall back to the numeric id for older containers.
|
||||
* which is the resource UUID for deployments since mid-2024.
|
||||
*
|
||||
* @param 'application'|'service'|'database' $type
|
||||
*/
|
||||
@@ -89,11 +88,11 @@ function containerOwnerUuid(Collection|array|string $labels, string $type): ?str
|
||||
}
|
||||
|
||||
/**
|
||||
* Resource that owns a container, from the given candidates.
|
||||
*
|
||||
* Order: UUID label, compose project / stack namespace (see containerOwnerUuid()), then the
|
||||
* numeric id label of containers without a UUID label. The id is only correct on the instance
|
||||
* that created the container, so it is the last fallback.
|
||||
* Resource that owns a container, from the given candidates, by UUID label or compose project /
|
||||
* stack namespace (see containerOwnerUuid()). Applications deployed before mid-2024 used the
|
||||
* deployment directory as compose project; their compose service (the container name) starts
|
||||
* with the application UUID. The numeric id label is never used: another Coolify instance that
|
||||
* manages the same server can have a resource with the same id.
|
||||
*
|
||||
* @template TResource of \Illuminate\Database\Eloquent\Model
|
||||
*
|
||||
@@ -107,13 +106,13 @@ function resolveContainerOwner(Collection $resources, Collection|array|string $l
|
||||
|
||||
$uuid = containerOwnerUuid($labels, $type);
|
||||
$owner = $uuid ? $resources->firstWhere('uuid', $uuid) : null;
|
||||
if ($owner || filled($labels->get("coolify.{$type}Uuid"))) {
|
||||
if ($owner || $type !== 'application' || filled($labels->get('coolify.applicationUuid')) || ! $labels->has('coolify.applicationId')) {
|
||||
return $owner;
|
||||
}
|
||||
|
||||
$legacyId = $labels->get("coolify.{$type}Id");
|
||||
$composeService = (string) $labels->get('com.docker.compose.service');
|
||||
|
||||
return is_numeric($legacyId) ? $resources->first(fn ($resource) => (int) $resource->id === (int) $legacyId) : null;
|
||||
return $resources->first(fn ($resource) => $composeService === $resource->uuid || str_starts_with($composeService, $resource->uuid.'-'));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -131,13 +130,8 @@ function resolveContainerApplicationId(Collection $applications, Collection|arra
|
||||
|
||||
$uuid = containerOwnerUuid($labels, 'application');
|
||||
$id = $uuid ? Application::withTrashed()->where('uuid', $uuid)->value('id') : null;
|
||||
if ($id !== null || filled($labels->get('coolify.applicationUuid'))) {
|
||||
return $id !== null ? (int) $id : null;
|
||||
}
|
||||
|
||||
$legacyId = $labels->get('coolify.applicationId');
|
||||
|
||||
return is_numeric($legacyId) ? (int) $legacyId : null;
|
||||
return $id !== null ? (int) $id : null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -189,10 +183,9 @@ function resolveServiceContainerOwner(Collection $services, Collection|array $fl
|
||||
* @param 'application'|'service'|'database' $type
|
||||
* @param list<string> $extraFilters Additional `--filter` values, e.g. `label=coolify.pullRequestId=0`.
|
||||
* @param list<string>|null $legacyExtraFilters Filters for containers without the UUID label; defaults to $extraFilters.
|
||||
* @param int|null $legacyId Local numeric id: also match older containers by `coolify.{type}Id` (may repeat a container).
|
||||
* @return list<string>
|
||||
*/
|
||||
function dockerPsByOwnerCommands(string $type, string $uuid, array $extraFilters = [], string $format = '{{json .}}', bool $all = true, ?array $legacyExtraFilters = null, ?int $legacyId = null): array
|
||||
function dockerPsByOwnerCommands(string $type, string $uuid, array $extraFilters = [], string $format = '{{json .}}', bool $all = true, ?array $legacyExtraFilters = null): array
|
||||
{
|
||||
$filters = fn (array $values) => collect($values)->map(fn (string $filter) => '--filter '.escapeshellarg($filter))->implode(' ');
|
||||
$base = 'docker ps'.($all ? ' -a' : '');
|
||||
@@ -203,8 +196,9 @@ function dockerPsByOwnerCommands(string $type, string $uuid, array $extraFilters
|
||||
$base.' '.$filters(["label=coolify.{$type}Uuid={$uuid}", ...$extraFilters]).' '.$formatArgument,
|
||||
$base.' '.$filters(["label=coolify.{$type}Id", "label=com.docker.compose.project={$uuid}", ...$legacyFilters]).' '.$formatArgument,
|
||||
];
|
||||
if ($legacyId !== null) {
|
||||
$commands[] = $base.' '.$filters(["label=coolify.{$type}Id={$legacyId}", ...$legacyFilters]).' '.$formatArgument;
|
||||
if ($type === 'application') {
|
||||
// Deployed before mid-2024 (see resolveContainerOwner()): the container name starts with the UUID.
|
||||
$commands[] = $base.' '.$filters(['label=coolify.applicationId', "name=^{$uuid}", ...$legacyFilters]).' '.$formatArgument;
|
||||
}
|
||||
|
||||
return $commands;
|
||||
@@ -217,7 +211,7 @@ function dockerPsByOwnerCommands(string $type, string $uuid, array $extraFilters
|
||||
*/
|
||||
function containersOwnedBy(Server $server, string $type, Model $resource): Collection
|
||||
{
|
||||
$output = instant_remote_process(dockerPsByOwnerCommands($type, $resource->uuid, legacyId: (int) $resource->id), $server);
|
||||
$output = instant_remote_process(dockerPsByOwnerCommands($type, $resource->uuid), $server);
|
||||
|
||||
return format_docker_command_output_to_json($output)->filter()->unique('ID')->values();
|
||||
}
|
||||
@@ -229,9 +223,9 @@ function containersOwnedBy(Server $server, string $type, Model $resource): Colle
|
||||
* @param list<string> $extraFilters
|
||||
* @param list<string>|null $legacyExtraFilters
|
||||
*/
|
||||
function containerIdsByOwnerScript(string $type, string $uuid, array $extraFilters = [], bool $all = true, ?array $legacyExtraFilters = null, ?int $legacyId = null): string
|
||||
function containerIdsByOwnerScript(string $type, string $uuid, array $extraFilters = [], bool $all = true, ?array $legacyExtraFilters = null): string
|
||||
{
|
||||
$commands = dockerPsByOwnerCommands($type, $uuid, $extraFilters, '{{.ID}}', $all, $legacyExtraFilters, $legacyId);
|
||||
$commands = dockerPsByOwnerCommands($type, $uuid, $extraFilters, '{{.ID}}', $all, $legacyExtraFilters);
|
||||
|
||||
return 'container_ids=$({ '.implode('; ', $commands).'; } | sort -u)';
|
||||
}
|
||||
|
||||
@@ -67,7 +67,7 @@ describe('label producers', function () {
|
||||
});
|
||||
|
||||
describe('container lookups for services and databases', function () {
|
||||
test('database and service lookups match the UUID label, the compose project, and the older id label', function () {
|
||||
test('database and service lookups match the UUID label and the compose project, never a numeric id', function () {
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]);
|
||||
$this->server->update(['private_key_id' => $privateKey->id]);
|
||||
$this->server->settings()->update(['is_reachable' => true, 'is_usable' => true]);
|
||||
@@ -97,13 +97,13 @@ describe('container lookups for services and databases', function () {
|
||||
expect($commands->implode("\n"))
|
||||
->toContain("label=coolify.databaseUuid={$database->uuid}")
|
||||
->toContain("label=com.docker.compose.project={$database->uuid}")
|
||||
->toContain("label=coolify.databaseId={$database->id}")
|
||||
->not->toContain("label=coolify.databaseId={$database->id}")
|
||||
->toContain("label=coolify.serviceUuid={$service->uuid}")
|
||||
->toContain("label=com.docker.compose.project={$service->uuid}")
|
||||
->toContain("label=coolify.serviceId={$service->id}");
|
||||
->not->toContain("label=coolify.serviceId={$service->id}");
|
||||
});
|
||||
|
||||
test('service part containers are matched by UUID label, compose service name, then older id', function () {
|
||||
test('service part containers are matched by UUID label, then compose service name', function () {
|
||||
$service = Service::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'server_id' => $this->server->id,
|
||||
@@ -117,8 +117,8 @@ describe('container lookups for services and databases', function () {
|
||||
expect($partOf(['coolify.serviceUuid' => $service->uuid, 'coolify.service.subType' => 'application', 'coolify.service.subUuid' => $part->uuid]))->toBe($part->id)
|
||||
// Moved from another instance: foreign ids, but compose project and service name still match.
|
||||
->and($partOf(['coolify.serviceId' => '999', 'coolify.service.subType' => 'application', 'coolify.service.subId' => '999', 'com.docker.compose.project' => $service->uuid, 'com.docker.compose.service' => 'web']))->toBe($part->id)
|
||||
// Older container with another compose project: local ids.
|
||||
->and($partOf(['coolify.serviceId' => (string) $service->id, 'coolify.service.subType' => 'application', 'coolify.service.subId' => (string) $part->id, 'com.docker.compose.project' => 'other']))->toBe($part->id);
|
||||
// Another instance's container with the same numeric ids: not ours.
|
||||
->and($partOf(['coolify.serviceId' => (string) $service->id, 'coolify.service.subType' => 'application', 'coolify.service.subId' => (string) $part->id, 'com.docker.compose.project' => 'other']))->toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -138,26 +138,35 @@ describe('owner resolution', function () {
|
||||
->toBe('app-uuid');
|
||||
});
|
||||
|
||||
test('containers from before mid-2024 fall back to their local numeric id', function () {
|
||||
test('a local numeric id never makes another instance\'s container ours', function () {
|
||||
$applications = collect([$this->application]);
|
||||
$olderContainer = ['coolify.applicationId' => (string) $this->application->id, 'com.docker.compose.project' => 'deployment-uuid'];
|
||||
$localId = (string) $this->application->id;
|
||||
|
||||
expect(resolveContainerOwner($applications, $olderContainer, 'application')?->id)->toBe($this->application->id)
|
||||
// A UUID label is never overridden by the numeric id.
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationUuid' => 'unknown', 'coolify.applicationId' => (string) $this->application->id], 'application'))->toBeNull()
|
||||
->and(dockerPsByOwnerCommands('application', 'app-uuid', legacyId: 42))->toHaveCount(3)
|
||||
->and(dockerPsByOwnerCommands('application', 'app-uuid', legacyId: 42)[2])->toContain("--filter 'label=coolify.applicationId=42'");
|
||||
expect(resolveContainerOwner($applications, ['coolify.applicationId' => $localId, 'com.docker.compose.project' => 'other-instance-uuid'], 'application'))->toBeNull()
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationId' => $localId], 'application'))->toBeNull()
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationUuid' => 'unknown', 'coolify.applicationId' => $localId], 'application'))->toBeNull()
|
||||
->and(resolveContainerApplicationId($applications, ['coolify.applicationId' => $localId, 'com.docker.compose.project' => 'other-instance-uuid']))->toBeNull()
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationId' => '999999', 'com.docker.compose.project' => $this->application->uuid], 'application')?->id)->toBe($this->application->id)
|
||||
// Deployed before mid-2024: compose project was the deployment directory, the compose service starts with the UUID.
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationId' => '999999', 'com.docker.compose.project' => 'deployment-dir', 'com.docker.compose.service' => $this->application->uuid.'-104512123456'], 'application')?->id)->toBe($this->application->id)
|
||||
->and(resolveContainerOwner($applications, ['coolify.applicationId' => $localId, 'com.docker.compose.project' => 'deployment-dir', 'com.docker.compose.service' => 'other-instance-uuid-104512123456'], 'application'))->toBeNull()
|
||||
->and(implode("\n", dockerPsByOwnerCommands('application', 'app-uuid')))->not->toContain('label=coolify.applicationId=');
|
||||
});
|
||||
|
||||
test('docker ps commands match the UUID label and the compose project of older containers', function () {
|
||||
$commands = dockerPsByOwnerCommands('application', 'app-uuid', ['label=coolify.pullRequestId=0']);
|
||||
|
||||
expect($commands)->toHaveCount(2)
|
||||
expect($commands)->toHaveCount(3)
|
||||
->and($commands[0])->toContain("--filter 'label=coolify.applicationUuid=app-uuid'")
|
||||
->toContain("--filter 'label=coolify.pullRequestId=0'")
|
||||
->and($commands[1])->toContain("--filter 'label=coolify.applicationId'")
|
||||
->toContain("--filter 'label=com.docker.compose.project=app-uuid'")
|
||||
->toContain("--filter 'label=coolify.pullRequestId=0'");
|
||||
->toContain("--filter 'label=coolify.pullRequestId=0'")
|
||||
// Deployed before mid-2024: the container name starts with the application UUID.
|
||||
->and($commands[2])->toContain("--filter 'label=coolify.applicationId'")
|
||||
->toContain("--filter 'name=^app-uuid'")
|
||||
->toContain("--filter 'label=coolify.pullRequestId=0'")
|
||||
->and(dockerPsByOwnerCommands('service', 'service-uuid'))->toHaveCount(2);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -207,7 +216,7 @@ describe('status updates after a server transfer', function () {
|
||||
->and($this->application->fresh()->status)->toStartWith('running');
|
||||
});
|
||||
|
||||
test('Sentinel push and status check still match containers from before mid-2024 by their local id', function () {
|
||||
test('Sentinel push and status check still match containers from before mid-2024 by the UUID in their name', function () {
|
||||
$labels = [
|
||||
'coolify.managed' => 'true',
|
||||
'coolify.applicationId' => (string) $this->application->id,
|
||||
@@ -269,8 +278,8 @@ describe('orphaned preview cleanup', function () {
|
||||
expect($isOrphaned(['Labels' => "coolify.applicationId=999999,coolify.pullRequestId=7,com.docker.compose.project={$this->application->uuid}"]))->toBeFalse()
|
||||
->and($isOrphaned(['Labels' => "coolify.applicationUuid={$this->application->uuid},coolify.pullRequestId=7"]))->toBeFalse()
|
||||
->and($isOrphaned(['Labels' => "coolify.applicationUuid={$this->application->uuid},coolify.pullRequestId=8"]))->toBeTrue()
|
||||
// Containers from before mid-2024: another compose project, so the local id is used, as before.
|
||||
->and($isOrphaned(['Labels' => "coolify.applicationId={$this->application->id},coolify.pullRequestId=7,com.docker.compose.project=deployment-uuid"]))->toBeFalse()
|
||||
->and($isOrphaned(['Labels' => 'coolify.applicationId=999999,coolify.pullRequestId=7']))->toBeTrue();
|
||||
// Another instance's preview container with the same numeric id: not ours, so never removed.
|
||||
->and($isOrphaned(['Labels' => "coolify.applicationId={$this->application->id},coolify.pullRequestId=8,com.docker.compose.project=other-instance-uuid"]))->toBeFalse()
|
||||
->and($isOrphaned(['Labels' => 'coolify.applicationId=999999,coolify.pullRequestId=7']))->toBeFalse();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -147,8 +147,8 @@ it('removes service containers before its volumes and local metadata', function
|
||||
expect($commandList)
|
||||
->toContain("label=coolify.serviceUuid={$service->uuid}")
|
||||
->toContain("label=com.docker.compose.project={$service->uuid}")
|
||||
// Containers from before mid-2024 may have another compose project; the local id still finds them.
|
||||
->toContain("label=coolify.serviceId={$service->id}")
|
||||
// A numeric id can belong to another Coolify instance on the same server, so it never selects containers.
|
||||
->not->toContain("label=coolify.serviceId={$service->id}")
|
||||
->toContain('sort -u')
|
||||
->toContain('docker rm -f $container_ids')
|
||||
->toContain("docker volume rm -f '{$service->uuid}_web-data'")
|
||||
|
||||
Reference in New Issue
Block a user