From 487e86c493171ed28cff16a9de4c0625561bd620 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:10:10 +0200 Subject: [PATCH] fix(service): delete a service part even when its container removal fails --- app/Actions/Service/DeleteService.php | 20 +++- .../Service/DeleteServiceSubresourceTest.php | 111 ++++++++++++++++++ 2 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/Service/DeleteServiceSubresourceTest.php diff --git a/app/Actions/Service/DeleteService.php b/app/Actions/Service/DeleteService.php index a48297e288..2e0010cdaa 100644 --- a/app/Actions/Service/DeleteService.php +++ b/app/Actions/Service/DeleteService.php @@ -5,7 +5,9 @@ namespace App\Actions\Service; use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; +use Illuminate\Support\Facades\Log; use RuntimeException; +use Throwable; class DeleteService { @@ -44,8 +46,9 @@ class DeleteService } /** - * 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). + * Removes the container of one service part. Returns false when the server does not respond, + * also when it is still marked reachable but the SSH call fails: the container then stays until + * the service starts again (compose up --remove-orphans), and the part can still be deleted from Coolify. */ public function removeSubresourceContainer(ServiceApplication|ServiceDatabase $resource): bool { @@ -54,7 +57,18 @@ class DeleteService return false; } - $this->removeContainers($service, $resource); + try { + $this->removeContainers($service, $resource); + } catch (Throwable $e) { + Log::warning('Could not remove the container of a service part; it is removed when the service starts again.', [ + 'service_uuid' => $service->uuid, + 'subresource_uuid' => $resource->uuid, + 'server_uuid' => $service->server->uuid, + 'error' => $e->getMessage(), + ]); + + return false; + } return true; } diff --git a/tests/Feature/Service/DeleteServiceSubresourceTest.php b/tests/Feature/Service/DeleteServiceSubresourceTest.php new file mode 100644 index 0000000000..97bdd0d0f6 --- /dev/null +++ b/tests/Feature/Service/DeleteServiceSubresourceTest.php @@ -0,0 +1,111 @@ +withoutVite(); + config()->set('app.maintenance.store', 'array'); + config()->set('constants.ssh.max_retries', 1); + InstanceSettings::forceCreate(['id' => 0]); + Server::flushIdentityMap(); + + $team = Team::factory()->create(); + $user = User::factory()->create(); + $team->members()->attach($user, ['role' => 'owner']); + $this->actingAs($user); + session(['currentTeam' => $team]); + + $this->server = Server::factory()->create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $this->service = Service::factory()->create([ + 'environment_id' => $environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + $this->application = ServiceApplication::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'web', + 'service_id' => $this->service->id, + 'image' => 'example/web:latest', + ]); + $this->database = ServiceDatabase::create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'postgres', + 'service_id' => $this->service->id, + 'image' => 'postgres:17', + ]); +}); + +function markServerReachable(Server $server): void +{ + $privateKey = PrivateKey::factory()->create(['team_id' => $server->team_id]); + $server->update(['private_key_id' => $privateKey->id]); + $server->settings()->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + Server::flushIdentityMap(); +} + +dataset('service parts', [ + 'application' => ['application', 'deleteApplication', ServiceApplication::class, 'Application'], + 'database' => ['database', 'deleteDatabase', ServiceDatabase::class, 'Database'], +]); + +it('deletes a service part from Coolify when the server stops responding while still marked reachable', function (string $part, string $method, string $model, string $label) { + markServerReachable($this->server); + Process::fake(['*' => Process::result(errorOutput: 'ssh: connect to host 1.2.3.4 port 22: Connection refused', exitCode: 255)]); + Log::spy(); + + Livewire::test(Index::class, ['serviceApplication' => $this->{$part}->fresh(), 'embedded' => true]) + ->call($method, 'password') + ->assertHasNoErrors() + ->assertDispatched('warning', "{$label} deleted from Coolify. The server does not respond, so its container is removed when the service starts again.") + ->assertNotDispatched('error'); + + expect($model::find($this->{$part}->id))->toBeNull(); + Process::assertRan(fn ($process) => str_contains($process->command, 'docker rm -f $container_ids')); + Log::shouldHaveReceived('warning')->withArgs(fn (string $message) => str_contains($message, 'service part'))->atLeast()->once(); +})->with('service parts'); + +it('deletes a service part from Coolify without SSH when the server is known to be offline', function (string $part, string $method, string $model, string $label) { + Process::fake(); + + Livewire::test(Index::class, ['serviceApplication' => $this->{$part}->fresh(), 'embedded' => true]) + ->call($method, 'password') + ->assertDispatched('warning', "{$label} deleted from Coolify. The server does not respond, so its container is removed when the service starts again."); + + expect($model::find($this->{$part}->id))->toBeNull(); + Process::assertNothingRan(); +})->with('service parts'); + +it('removes the container of a service part and reports success when the server responds', function (string $part, string $method, string $model, string $label) { + markServerReachable($this->server); + Process::fake(['*' => Process::result(output: '')]); + + Livewire::test(Index::class, ['serviceApplication' => $this->{$part}->fresh(), 'embedded' => true]) + ->call($method, 'password') + ->assertDispatched('success', "{$label} deleted.") + ->assertNotDispatched('warning'); + + expect($model::find($this->{$part}->id))->toBeNull(); + Process::assertRan(fn ($process) => str_contains($process->command, "coolify.service.subUuid={$this->{$part}->uuid}")); +})->with('service parts');