mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-12 15:53:46 -04:00
fix(services): finish service cleanup when a volume cannot be removed
- Try to remove every volume and always delete the configuration directory (it holds the .env with secrets), then report the volumes that failed. - Delete volume backup schedules also when a service is removed from Coolify only. - The delete API no longer claims the server is unreachable when the caller asked to delete from Coolify only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b98e810e3f
commit
2e2e43fddc
@@ -20,29 +20,60 @@ class DeleteService
|
||||
|
||||
$this->removeContainers($service);
|
||||
|
||||
if ($deleteVolumes) {
|
||||
$commands = [];
|
||||
foreach ($service->applications()->get() as $application) {
|
||||
foreach ($application->persistentStorages()->get() as $storage) {
|
||||
$commands[] = 'docker volume rm -f '.escapeshellarg($storage->name);
|
||||
}
|
||||
}
|
||||
foreach ($service->databases()->get() as $database) {
|
||||
foreach ($database->persistentStorages()->get() as $storage) {
|
||||
$commands[] = 'docker volume rm -f '.escapeshellarg($storage->name);
|
||||
}
|
||||
}
|
||||
foreach ($commands as $command) {
|
||||
instant_remote_process([$command], $server);
|
||||
}
|
||||
}
|
||||
$failedVolumes = $deleteVolumes ? $this->removeVolumes($service) : [];
|
||||
|
||||
if ($deleteConnectedNetworks) {
|
||||
$service->deleteConnectedNetworks();
|
||||
}
|
||||
// The configuration directory holds the .env file with plaintext secrets, so it is removed
|
||||
// also when a volume could not be removed.
|
||||
if ($deleteConfigurations) {
|
||||
$service->deleteConfigurations();
|
||||
}
|
||||
|
||||
if ($failedVolumes !== []) {
|
||||
Log::warning('Could not remove all volumes of a deleted service.', [
|
||||
'service_uuid' => $service->uuid,
|
||||
'server_uuid' => $server->uuid,
|
||||
'volumes' => $failedVolumes,
|
||||
]);
|
||||
|
||||
throw new RuntimeException('Could not remove these volumes: '.collect($failedVolumes)
|
||||
->map(fn (string $error, string $volume): string => "{$volume} ({$error})")
|
||||
->implode(', '));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes every volume of the service. A volume that cannot be removed (for example because
|
||||
* another container still uses it) does not stop the removal of the others.
|
||||
*
|
||||
* @return array<string, string> The error message of each volume that could not be removed.
|
||||
*/
|
||||
private function removeVolumes(Service $service): array
|
||||
{
|
||||
$volumeNames = [];
|
||||
foreach ($service->applications()->get() as $application) {
|
||||
foreach ($application->persistentStorages()->get() as $storage) {
|
||||
$volumeNames[] = $storage->name;
|
||||
}
|
||||
}
|
||||
foreach ($service->databases()->get() as $database) {
|
||||
foreach ($database->persistentStorages()->get() as $storage) {
|
||||
$volumeNames[] = $storage->name;
|
||||
}
|
||||
}
|
||||
|
||||
$failedVolumes = [];
|
||||
foreach ($volumeNames as $volumeName) {
|
||||
try {
|
||||
instant_remote_process(['docker volume rm -f '.escapeshellarg($volumeName)], $service->server);
|
||||
} catch (Throwable $e) {
|
||||
$failedVolumes[$volumeName] = $e->getMessage();
|
||||
}
|
||||
}
|
||||
|
||||
return $failedVolumes;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1054,7 +1054,8 @@ class ServicesController extends Controller
|
||||
|
||||
$service->delete();
|
||||
|
||||
$deleteFromCoolifyOnly = $request->boolean('delete_from_coolify_only') || ! $service->server?->isFunctional();
|
||||
$deleteFromCoolifyOnlyRequested = $request->boolean('delete_from_coolify_only');
|
||||
$deleteFromCoolifyOnly = $deleteFromCoolifyOnlyRequested || ! $service->server?->isFunctional();
|
||||
|
||||
DeleteResourceJob::dispatch(
|
||||
resource: $service,
|
||||
@@ -1072,11 +1073,13 @@ class ServicesController extends Controller
|
||||
'delete_from_coolify_only' => $deleteFromCoolifyOnly,
|
||||
]);
|
||||
|
||||
return response()->json([
|
||||
'message' => $deleteFromCoolifyOnly
|
||||
? 'Server is not reachable. The service will be removed from Coolify only; Docker resources may remain.'
|
||||
: 'Service deletion request queued.',
|
||||
]);
|
||||
$message = match (true) {
|
||||
$deleteFromCoolifyOnlyRequested => 'The service will be removed from Coolify only; Docker resources will remain.',
|
||||
$deleteFromCoolifyOnly => 'Server is not reachable. The service will be removed from Coolify only; Docker resources may remain.',
|
||||
default => 'Service deletion request queued.',
|
||||
};
|
||||
|
||||
return response()->json(['message' => $message]);
|
||||
}
|
||||
|
||||
#[OA\Patch(
|
||||
|
||||
@@ -11,6 +11,7 @@ use App\Enums\ApplicationDeploymentStatus;
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationDeploymentQueue;
|
||||
use App\Models\ApplicationPreview;
|
||||
use App\Models\ScheduledVolumeBackup;
|
||||
use App\Models\Service;
|
||||
use App\Models\StandaloneClickhouse;
|
||||
use App\Models\StandaloneDragonfly;
|
||||
@@ -28,6 +29,7 @@ use Illuminate\Contracts\Queue\ShouldQueue;
|
||||
use Illuminate\Foundation\Bus\Dispatchable;
|
||||
use Illuminate\Queue\InteractsWithQueue;
|
||||
use Illuminate\Queue\SerializesModels;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
@@ -55,6 +57,9 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
|
||||
}
|
||||
|
||||
if ($this->deleteFromCoolifyOnly && $this->resource instanceof Service) {
|
||||
// No remote calls on this path: remove only the schedule records, so the scheduler does
|
||||
// not keep running backups of volumes that no longer exist in Coolify.
|
||||
$this->volumeBackupSchedules()->each->delete();
|
||||
$this->deleteLocalResource();
|
||||
|
||||
return;
|
||||
@@ -170,19 +175,24 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
|
||||
private function deleteScheduledVolumeBackups(): void
|
||||
{
|
||||
$server = data_get($this->resource, 'server') ?? data_get($this->resource, 'destination.server');
|
||||
|
||||
foreach ($this->volumeBackupSchedules() as $backup) {
|
||||
DeleteScheduledVolumeBackup::run($backup, $server);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return Collection<int, ScheduledVolumeBackup>
|
||||
*/
|
||||
private function volumeBackupSchedules(): Collection
|
||||
{
|
||||
$resources = $this->resource instanceof Service
|
||||
? $this->resource->applications()->get()->concat($this->resource->databases()->get())
|
||||
: collect([$this->resource]);
|
||||
|
||||
foreach ($resources as $resource) {
|
||||
$storages = $resource->persistentStorages()->get()->concat($resource->fileStorages()->get());
|
||||
|
||||
foreach ($storages as $storage) {
|
||||
foreach ($storage->scheduledBackups()->get() as $backup) {
|
||||
DeleteScheduledVolumeBackup::run($backup, $server);
|
||||
}
|
||||
}
|
||||
}
|
||||
return $resources->flatMap(fn ($resource) => $resource->persistentStorages()->get()
|
||||
->concat($resource->fileStorages()->get())
|
||||
->flatMap(fn ($storage) => $storage->scheduledBackups()->get()));
|
||||
}
|
||||
|
||||
private function deleteApplicationPreview(): void
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\DeleteResourceJob;
|
||||
use App\Models\Environment;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\Project;
|
||||
use App\Models\Server;
|
||||
use App\Models\Service;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
Server::flushIdentityMap();
|
||||
Storage::fake('ssh-mux');
|
||||
Queue::fake();
|
||||
InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]);
|
||||
|
||||
$this->team = Team::factory()->create();
|
||||
$this->user = User::factory()->create();
|
||||
$this->team->members()->attach($this->user->id, ['role' => 'owner']);
|
||||
session(['currentTeam' => $this->team]);
|
||||
$token = $this->user->createToken('service-delete-message', ['write']);
|
||||
$token->accessToken->forceFill(['team_id' => $this->team->id])->save();
|
||||
$this->bearerToken = $token->plainTextToken;
|
||||
|
||||
$this->server = Server::factory()->create(['team_id' => $this->team->id, 'ip' => '10.20.30.40']);
|
||||
$destination = $this->server->standaloneDockers()->firstOrFail();
|
||||
$project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
$this->service = Service::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => $destination->getMorphClass(),
|
||||
'server_id' => $this->server->id,
|
||||
]);
|
||||
});
|
||||
|
||||
function setServiceDeleteServerReachable(Server $server, bool $reachable): void
|
||||
{
|
||||
$server->settings->forceFill([
|
||||
'is_reachable' => $reachable,
|
||||
'is_usable' => $reachable,
|
||||
'force_disabled' => false,
|
||||
])->save();
|
||||
Server::flushIdentityMap();
|
||||
}
|
||||
|
||||
test('explicit delete from coolify only does not claim the server is unreachable', function (bool $serverReachable) {
|
||||
setServiceDeleteServerReachable($this->server, $serverReachable);
|
||||
|
||||
$this->withToken($this->bearerToken)
|
||||
->deleteJson("/api/v1/services/{$this->service->uuid}?delete_from_coolify_only=true")
|
||||
->assertOk()
|
||||
->assertJsonPath('message', 'The service will be removed from Coolify only; Docker resources will remain.');
|
||||
|
||||
Queue::assertPushed(DeleteResourceJob::class, fn (DeleteResourceJob $job): bool => $job->deleteFromCoolifyOnly);
|
||||
})->with([
|
||||
'reachable server' => [true],
|
||||
'unreachable server' => [false],
|
||||
]);
|
||||
|
||||
test('delete on a reachable server queues a full deletion', function () {
|
||||
setServiceDeleteServerReachable($this->server, true);
|
||||
|
||||
$this->withToken($this->bearerToken)
|
||||
->deleteJson("/api/v1/services/{$this->service->uuid}")
|
||||
->assertOk()
|
||||
->assertJsonPath('message', 'Service deletion request queued.');
|
||||
|
||||
Queue::assertPushed(DeleteResourceJob::class, fn (DeleteResourceJob $job): bool => ! $job->deleteFromCoolifyOnly);
|
||||
});
|
||||
|
||||
test('delete on an unreachable server falls back to removing from coolify only', function () {
|
||||
setServiceDeleteServerReachable($this->server, false);
|
||||
|
||||
$this->withToken($this->bearerToken)
|
||||
->deleteJson("/api/v1/services/{$this->service->uuid}")
|
||||
->assertOk()
|
||||
->assertJsonPath('message', 'Server is not reachable. The service will be removed from Coolify only; Docker resources may remain.');
|
||||
|
||||
Queue::assertPushed(DeleteResourceJob::class, fn (DeleteResourceJob $job): bool => $job->deleteFromCoolifyOnly);
|
||||
});
|
||||
@@ -0,0 +1,94 @@
|
||||
<?php
|
||||
|
||||
use App\Actions\Service\DeleteService;
|
||||
use App\Jobs\DeleteResourceJob;
|
||||
use App\Models\Environment;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Project;
|
||||
use App\Models\ScheduledVolumeBackup;
|
||||
use App\Models\Server;
|
||||
use App\Models\Service;
|
||||
use App\Models\ServiceApplication;
|
||||
use App\Models\ServiceDatabase;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
Server::flushIdentityMap();
|
||||
InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0]));
|
||||
Storage::fake('ssh-keys');
|
||||
Storage::fake('ssh-mux');
|
||||
Queue::fake();
|
||||
|
||||
$team = Team::factory()->create();
|
||||
$server = Server::factory()->create(['team_id' => $team->id]);
|
||||
$privateKey = PrivateKey::factory()->create(['team_id' => $team->id]);
|
||||
$server->update(['private_key_id' => $privateKey->id]);
|
||||
$server->settings()->update(['is_reachable' => true, 'is_usable' => true]);
|
||||
$destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = $project->environments()->first()
|
||||
?? Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$this->service = Service::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'server_id' => $server->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => $destination->getMorphClass(),
|
||||
]);
|
||||
$this->serviceApplication = ServiceApplication::create(['service_id' => $this->service->id, 'name' => 'web', 'image' => 'nginx:alpine']);
|
||||
$this->serviceDatabase = ServiceDatabase::create(['service_id' => $this->service->id, 'name' => 'db', 'image' => 'postgres:17-alpine']);
|
||||
$this->inUseVolume = $this->serviceApplication->persistentStorages()->create([
|
||||
'name' => "{$this->service->uuid}_web-data",
|
||||
'mount_path' => '/data',
|
||||
'host_path' => null,
|
||||
]);
|
||||
$this->serviceDatabase->persistentStorages()->create([
|
||||
'name' => "{$this->service->uuid}_db-data",
|
||||
'mount_path' => '/var/lib/postgresql/data',
|
||||
'host_path' => null,
|
||||
]);
|
||||
});
|
||||
|
||||
it('removes the other volumes, networks and the configuration directory when one volume is in use', function () {
|
||||
$commands = collect();
|
||||
Process::fake(function ($process) use ($commands) {
|
||||
$commands->push($process->command);
|
||||
if (str_contains($process->command, "docker volume rm -f '{$this->service->uuid}_web-data'")) {
|
||||
return Process::result(errorOutput: "Error response from daemon: remove {$this->service->uuid}_web-data: volume is in use", exitCode: 1);
|
||||
}
|
||||
|
||||
return Process::result(output: '');
|
||||
});
|
||||
|
||||
expect(fn () => app(DeleteService::class)->cleanupRemote($this->service, deleteVolumes: true, deleteConnectedNetworks: true, deleteConfigurations: true))
|
||||
->toThrow(RuntimeException::class, "{$this->service->uuid}_web-data");
|
||||
|
||||
$commandList = $commands->implode("\n");
|
||||
expect($commandList)
|
||||
->toContain("docker volume rm -f '{$this->service->uuid}_db-data'")
|
||||
->toContain("docker network rm {$this->service->uuid}")
|
||||
->toContain('rm -rf '.$this->service->workdir());
|
||||
});
|
||||
|
||||
it('removes volume backup schedules when a service is deleted from Coolify only', function () {
|
||||
$backup = $this->inUseVolume->scheduledBackups()->create([
|
||||
'team_id' => $this->service->environment->project->team_id,
|
||||
'frequency' => 'daily',
|
||||
'timeout' => 3600,
|
||||
]);
|
||||
Process::fake();
|
||||
|
||||
(new DeleteResourceJob($this->service, deleteFromCoolifyOnly: true))->handle();
|
||||
|
||||
Process::assertNothingRan();
|
||||
expect(ScheduledVolumeBackup::query()->find($backup->id))->toBeNull()
|
||||
->and(Service::withTrashed()->find($this->service->id))->toBeNull();
|
||||
});
|
||||
Reference in New Issue
Block a user