From 2b5bffe37d4e550cc485b2f2aafe23c0c52466e2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 08:02:02 +0200 Subject: [PATCH] fix(service): delete resources from Coolify when server is unreachable DeleteResourceJob no longer restores a service and rethrows when remote cleanup fails. The resource is always removed from Coolify, and the team gets a notification that Docker resources may remain on the server. No notification is sent when the server itself was already deleted. removeSubresourceContainer now returns false instead of throwing when the server is not functional. Deleting a service application or database then shows a warning that the container is removed on the next service start. StopService handles a missing destination server, environment, project, or team without throwing. The danger zone warning now shows for any resource whose server is unreachable, not only services. --- app/Actions/Service/DeleteService.php | 13 ++-- app/Actions/Service/StopService.php | 10 ++- app/Jobs/DeleteResourceJob.php | 23 +++---- app/Livewire/Project/Service/Index.php | 12 ++-- .../livewire/project/shared/danger.blade.php | 8 +-- .../DeleteResourceJobAtomicityTest.php | 69 ++++++++++++++++--- 6 files changed, 99 insertions(+), 36 deletions(-) diff --git a/app/Actions/Service/DeleteService.php b/app/Actions/Service/DeleteService.php index 1ecc029c43..a48297e288 100644 --- a/app/Actions/Service/DeleteService.php +++ b/app/Actions/Service/DeleteService.php @@ -43,15 +43,20 @@ class DeleteService } } - public function removeSubresourceContainer(ServiceApplication|ServiceDatabase $resource): void + /** + * Removes the container of one service part. Returns false when the server does not respond: + * the container then stays until the service starts again (compose up --remove-orphans). + */ + public function removeSubresourceContainer(ServiceApplication|ServiceDatabase $resource): bool { $service = $resource->service; - $server = $service?->server; - if (! $server?->isFunctional()) { - throw new RuntimeException('Server is not functional.'); + if (! $service?->server?->isFunctional()) { + return false; } $this->removeContainers($service, $resource); + + return true; } private function removeContainers(Service $service, ServiceApplication|ServiceDatabase|null $subresource = null): void diff --git a/app/Actions/Service/StopService.php b/app/Actions/Service/StopService.php index ce101c5b80..f9d65d1ed1 100644 --- a/app/Actions/Service/StopService.php +++ b/app/Actions/Service/StopService.php @@ -30,8 +30,9 @@ class StopService $activity->save(); }); - $server = $service->destination->server; - if (! $server->isFunctional()) { + // The server is null once it is deleted (for example "Delete server" with all resources). + $server = $service->destination?->server; + if (! $server?->isFunctional()) { return 'Server is not functional'; } @@ -66,7 +67,10 @@ class StopService } catch (\Exception $e) { return $e->getMessage(); } finally { - ServiceStatusChanged::dispatch($service->environment->project->team->id); + $teamId = $service->environment?->project?->team?->id; + if ($teamId !== null) { + ServiceStatusChanged::dispatch($teamId); + } } } diff --git a/app/Jobs/DeleteResourceJob.php b/app/Jobs/DeleteResourceJob.php index 98df673335..deba20d99a 100644 --- a/app/Jobs/DeleteResourceJob.php +++ b/app/Jobs/DeleteResourceJob.php @@ -99,24 +99,21 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue } } } catch (\Throwable $e) { - if ($this->resource instanceof Service) { - if ($this->resource->trashed()) { - $this->resource->restore(); - } - - $this->resource->server?->team?->notify(new GeneralNotification( - "Service deletion failed for '{$this->resource->name}'. Docker resources may still exist on server '{$this->resource->server?->name}'. You can retry the cleanup or select 'Remove from Coolify only' in the deletion dialog. Error: {$e->getMessage()}", - success: false, - )); - - throw $e; - } - + // A resource can always be deleted from Coolify, also when its server does not respond. Log::warning('Remote cleanup failed while deleting resource; continuing with local deletion.', [ 'resource_id' => $this->resource->id, 'resource_type' => $this->resource->type(), 'error' => $e->getMessage(), ]); + + // A deleted server has nothing left to clean up. Otherwise tell the team what may remain. + $server = data_get($this->resource, 'destination.server'); + if ($server) { + $this->resource->team()?->notify(new GeneralNotification( + "'{$this->resource->name}' was removed from Coolify, but its Docker resources could not be removed from server '{$server->name}' and may still exist there. Error: {$e->getMessage()}", + success: false, + )); + } } try { diff --git a/app/Livewire/Project/Service/Index.php b/app/Livewire/Project/Service/Index.php index dbba532261..abdcada989 100644 --- a/app/Livewire/Project/Service/Index.php +++ b/app/Livewire/Project/Service/Index.php @@ -256,9 +256,11 @@ class Index extends Component return 'The provided password is incorrect.'; } - app(DeleteService::class)->removeSubresourceContainer($this->serviceDatabase); + $containerRemoved = app(DeleteService::class)->removeSubresourceContainer($this->serviceDatabase); $this->serviceDatabase->delete(); - $this->dispatch('success', 'Database deleted.'); + $containerRemoved + ? $this->dispatch('success', 'Database deleted.') + : $this->dispatch('warning', 'Database deleted from Coolify. The server does not respond, so its container is removed when the service starts again.'); return redirectRoute($this, 'project.service.configuration', $this->parameters); } catch (\Throwable $e) { @@ -504,9 +506,11 @@ class Index extends Component return 'The provided password is incorrect.'; } - app(DeleteService::class)->removeSubresourceContainer($this->serviceApplication); + $containerRemoved = app(DeleteService::class)->removeSubresourceContainer($this->serviceApplication); $this->serviceApplication->delete(); - $this->dispatch('success', 'Application deleted.'); + $containerRemoved + ? $this->dispatch('success', 'Application deleted.') + : $this->dispatch('warning', 'Application deleted from Coolify. The server does not respond, so its container is removed when the service starts again.'); return redirectRoute($this, 'project.service.configuration', $this->parameters); } catch (\Throwable $e) { diff --git a/resources/views/livewire/project/shared/danger.blade.php b/resources/views/livewire/project/shared/danger.blade.php index 1edd4123d4..faeff1aed2 100644 --- a/resources/views/livewire/project/shared/danger.blade.php +++ b/resources/views/livewire/project/shared/danger.blade.php @@ -8,11 +8,11 @@ default => 'resource', }; @endphp - @if ($resource instanceof \App\Models\Service && !$resource->server?->isFunctional()) + @if ($resource && !data_get($resource, 'destination.server')?->isFunctional()) - Coolify cannot remove or verify Docker resources on this server. The deletion dialog will default to - removing this service from Coolify only. Its containers, volumes, networks, and configuration files may - remain on the server. + Coolify cannot stop or remove Docker resources on this server. Deleting removes this {{ $resourceLabel }} + from Coolify only. Its containers, volumes, networks, and configuration files remain on the server and can + start again when the server is back. @endif InstanceSettings::firstOrCreate(['id' => 0])); $team = Team::factory()->create(); @@ -59,7 +61,7 @@ it('deletes a non-service Coolify resource when remote cleanup fails', function Queue::assertNotPushed(QueuedCommand::class); }); -it('keeps a service when remote cleanup fails', function () { +it('removes a service from Coolify and warns the team when its server does not respond', function () { Notification::fake(); $service = Service::factory()->create([ 'environment_id' => $this->application->environment_id, @@ -76,23 +78,74 @@ it('keeps a service when remote cleanup fails', function () { 'webhook_enabled' => true, 'webhook_url' => 'https://example.com/webhook', ]); - $service->delete(); Process::fake(); - expect(fn () => (new DeleteResourceJob($service))->handle()) - ->toThrow(RuntimeException::class, 'Server is not functional.'); + (new DeleteResourceJob($service))->handle(); - expect(Service::find($service->id))->not->toBeNull() - ->and(ServiceApplication::where('service_id', $service->id)->exists())->toBeTrue(); - Notification::assertCount(1); + Process::assertNothingRan(); + expect(Service::withTrashed()->find($service->id))->toBeNull() + ->and(ServiceApplication::withTrashed()->where('service_id', $service->id)->exists())->toBeFalse(); Notification::assertSentTo( $service->team(), GeneralNotification::class, fn (GeneralNotification $notification): bool => ! $notification->success - && str_contains($notification->message, 'Remove from Coolify only') + && str_contains($notification->message, "was removed from Coolify, but its Docker resources could not be removed from server '{$service->server->name}'") ); }); +it('removes a service from Coolify when its server was deleted', function () { + Notification::fake(); + $service = Service::factory()->create([ + 'environment_id' => $this->application->environment_id, + 'server_id' => $this->application->destination->server_id, + 'destination_id' => $this->application->destination_id, + 'destination_type' => $this->application->destination_type, + ]); + ServiceApplication::create([ + 'service_id' => $service->id, + 'name' => 'web', + 'image' => 'nginx:alpine', + ]); + // "Delete server" soft-deletes the server before the queued resource jobs run. + $service->server->delete(); + Server::flushIdentityMap(); + Process::fake(); + + (new DeleteResourceJob($service->fresh()))->handle(); + + Process::assertNothingRan(); + expect(Service::withTrashed()->find($service->id))->toBeNull() + ->and(ServiceApplication::withTrashed()->where('service_id', $service->id)->exists())->toBeFalse(); + Notification::assertNothingSent(); +}); + +it('stops a service without an error when its server was deleted', function () { + $service = Service::factory()->create([ + 'environment_id' => $this->application->environment_id, + 'server_id' => $this->application->destination->server_id, + 'destination_id' => $this->application->destination_id, + 'destination_type' => $this->application->destination_type, + ]); + $service->server->delete(); + Server::flushIdentityMap(); + + expect(StopService::run($service->fresh()))->toBe('Server is not functional'); +}); + +it('skips the container of a service part when the server does not respond', function () { + $service = Service::factory()->create([ + 'environment_id' => $this->application->environment_id, + 'server_id' => $this->application->destination->server_id, + 'destination_id' => $this->application->destination_id, + 'destination_type' => $this->application->destination_type, + ]); + $part = ServiceApplication::create(['service_id' => $service->id, 'name' => 'web', 'image' => 'nginx:alpine']); + Process::fake(); + + expect(app(DeleteService::class)->removeSubresourceContainer($part))->toBeFalse(); + Process::assertNothingRan(); +}); + it('deletes only local service metadata when explicitly requested', function () { $service = Service::factory()->create([ 'environment_id' => $this->application->environment_id,