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,