diff --git a/app/Actions/Service/DeleteService.php b/app/Actions/Service/DeleteService.php index 2e0010cdaa..0b511c6dca 100644 --- a/app/Actions/Service/DeleteService.php +++ b/app/Actions/Service/DeleteService.php @@ -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 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; } /** diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 63a1a8daf7..dba19f6751 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -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( diff --git a/app/Jobs/DeleteResourceJob.php b/app/Jobs/DeleteResourceJob.php index deba20d99a..3d0e62e766 100644 --- a/app/Jobs/DeleteResourceJob.php +++ b/app/Jobs/DeleteResourceJob.php @@ -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 + */ + 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 diff --git a/tests/Feature/Api/ServiceDeleteMessageApiTest.php b/tests/Feature/Api/ServiceDeleteMessageApiTest.php new file mode 100644 index 0000000000..6cc34f4ffd --- /dev/null +++ b/tests/Feature/Api/ServiceDeleteMessageApiTest.php @@ -0,0 +1,87 @@ + 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); +}); diff --git a/tests/Feature/DeleteServiceRemoteCleanupTest.php b/tests/Feature/DeleteServiceRemoteCleanupTest.php new file mode 100644 index 0000000000..52affebdfe --- /dev/null +++ b/tests/Feature/DeleteServiceRemoteCleanupTest.php @@ -0,0 +1,94 @@ + 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(); +});