From 38502e82054f03bf4abf86faa0c0921f78447b32 Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Thu, 20 Aug 2026 11:10:14 +0200
Subject: [PATCH] fix(resources): preserve deletion when remote cleanup fails
Separate remote service cleanup from resource deletion and ensure database removal
completes despite cleanup errors. Update the disabled two-factor badge icon.
---
app/Actions/Service/DeleteService.php | 95 ++++++---------
app/Jobs/DeleteResourceJob.php | 109 +++++++++++-------
.../components/two-factor-badge.blade.php | 6 +-
.../DeleteResourceJobAtomicityTest.php | 64 ++++++++++
.../Feature/TeamMemberTwoFactorStatusTest.php | 1 +
5 files changed, 168 insertions(+), 107 deletions(-)
create mode 100644 tests/Feature/DeleteResourceJobAtomicityTest.php
diff --git a/app/Actions/Service/DeleteService.php b/app/Actions/Service/DeleteService.php
index 460600d699..2def132804 100644
--- a/app/Actions/Service/DeleteService.php
+++ b/app/Actions/Service/DeleteService.php
@@ -2,77 +2,52 @@
namespace App\Actions\Service;
-use App\Actions\Server\CleanupDocker;
use App\Models\Service;
-use Illuminate\Support\Facades\Log;
-use Lorisleiva\Actions\Concerns\AsAction;
class DeleteService
{
- use AsAction;
-
- public function handle(Service $service, bool $deleteVolumes, bool $deleteConnectedNetworks, bool $deleteConfigurations, bool $dockerCleanup)
+ public function cleanupRemote(Service $service, bool $deleteVolumes, bool $deleteConnectedNetworks, bool $deleteConfigurations): void
{
- try {
- $server = data_get($service, 'server');
- if ($deleteVolumes && $server->isFunctional()) {
- $storagesToDelete = collect([]);
-
- $service->environment_variables()->delete();
- $commands = [];
- foreach ($service->applications()->get() as $application) {
- $storages = $application->persistentStorages()->get();
- foreach ($storages as $storage) {
- $storagesToDelete->push($storage);
- }
- }
- foreach ($service->databases()->get() as $database) {
- $storages = $database->persistentStorages()->get();
- foreach ($storages as $storage) {
- $storagesToDelete->push($storage);
- }
- }
- foreach ($storagesToDelete as $storage) {
+ $server = data_get($service, 'server');
+ if ($deleteVolumes && $server->isFunctional()) {
+ $commands = [];
+ foreach ($service->applications()->get() as $application) {
+ foreach ($application->persistentStorages()->get() as $storage) {
$commands[] = 'docker volume rm -f '.escapeshellarg($storage->name);
}
-
- // Execute volume deletion first, this must be done first otherwise volumes will not be deleted.
- if (! empty($commands)) {
- foreach ($commands as $command) {
- $result = instant_remote_process([$command], $server, false);
- if ($result !== null && $result !== 0) {
- Log::error('Error deleting volumes: '.$result);
- }
- }
- }
- }
-
- if ($deleteConnectedNetworks) {
- $service->deleteConnectedNetworks();
- }
-
- instant_remote_process(["docker rm -f $service->uuid"], $server, throwError: false);
- } catch (\Exception $e) {
- throw new \RuntimeException($e->getMessage());
- } finally {
- if ($deleteConfigurations) {
- $service->deleteConfigurations();
- }
- foreach ($service->applications()->get() as $application) {
- $application->forceDelete();
}
foreach ($service->databases()->get() as $database) {
- $database->forceDelete();
+ foreach ($database->persistentStorages()->get() as $storage) {
+ $commands[] = 'docker volume rm -f '.escapeshellarg($storage->name);
+ }
}
- foreach ($service->scheduled_tasks as $task) {
- $task->delete();
- }
- $service->tags()->detach();
- $service->forceDelete();
-
- if ($dockerCleanup) {
- CleanupDocker::dispatch($server, false, false);
+ foreach ($commands as $command) {
+ instant_remote_process([$command], $server, false);
}
}
+
+ if ($deleteConnectedNetworks) {
+ $service->deleteConnectedNetworks();
+ }
+ if ($deleteConfigurations) {
+ $service->deleteConfigurations();
+ }
+ instant_remote_process(["docker rm -f $service->uuid"], $server, throwError: false);
+ }
+
+ public function deleteLocal(Service $service): void
+ {
+ foreach ($service->applications()->get() as $application) {
+ $application->forceDelete();
+ }
+ foreach ($service->databases()->get() as $database) {
+ $database->forceDelete();
+ }
+ foreach ($service->scheduled_tasks as $task) {
+ $task->delete();
+ }
+ $service->environment_variables()->delete();
+ $service->tags()->detach();
+ $service->forceDelete();
}
}
diff --git a/app/Jobs/DeleteResourceJob.php b/app/Jobs/DeleteResourceJob.php
index 124cc16cca..dff7d88de1 100644
--- a/app/Jobs/DeleteResourceJob.php
+++ b/app/Jobs/DeleteResourceJob.php
@@ -4,7 +4,6 @@ namespace App\Jobs;
use App\Actions\Application\StopApplication;
use App\Actions\Database\StopDatabase;
-use App\Actions\Server\CleanupDocker;
use App\Actions\Service\DeleteService;
use App\Actions\Service\StopService;
use App\Actions\Shared\DeleteScheduledVolumeBackup;
@@ -28,6 +27,8 @@ use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Log;
class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
{
@@ -43,20 +44,17 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
$this->onQueue('high');
}
- public function handle()
+ public function handle(): void
{
- if (! $this->resource instanceof ApplicationPreview) {
- $this->deleteScheduledVolumeBackups();
+ if ($this->resource instanceof ApplicationPreview) {
+ DB::transaction(function (): void {
+ $this->deleteApplicationPreview();
+ });
+
+ return;
}
try {
- // Handle ApplicationPreview instances separately
- if ($this->resource instanceof ApplicationPreview) {
- $this->deleteApplicationPreview();
-
- return;
- }
-
switch ($this->resource->type()) {
case 'application':
StopApplication::run($this->resource, previewDeployments: true, dockerCleanup: $this->dockerCleanup);
@@ -73,21 +71,71 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
break;
case 'service':
StopService::run($this->resource, $this->deleteConnectedNetworks, $this->dockerCleanup);
- DeleteService::run($this->resource, $this->deleteVolumes, $this->deleteConnectedNetworks, $this->deleteConfigurations, $this->dockerCleanup);
-
- return;
+ app(DeleteService::class)->cleanupRemote(
+ $this->resource,
+ $this->deleteVolumes,
+ $this->deleteConnectedNetworks,
+ $this->deleteConfigurations,
+ );
+ break;
}
- if ($this->deleteConfigurations) {
- $this->resource->deleteConfigurations();
+ if (! $this->resource instanceof Service) {
+ if ($this->deleteConfigurations) {
+ $this->resource->deleteConfigurations();
+ }
+ if ($this->deleteVolumes) {
+ $this->resource->deleteVolumes();
+ }
+ if ($this->deleteConnectedNetworks && $this->resource->type() === 'application') {
+ $this->resource->deleteConnectedNetworks();
+ }
}
+ } catch (\Throwable $e) {
+ 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(),
+ ]);
+ }
+
+ DB::transaction(function (): void {
+ try {
+ $this->deleteScheduledVolumeBackups();
+ } catch (\Throwable $e) {
+ Log::warning('Remote backup cleanup failed while deleting resource; continuing with local deletion.', [
+ 'resource_id' => $this->resource->id,
+ 'resource_type' => $this->resource->type(),
+ 'error' => $e->getMessage(),
+ ]);
+ }
+
+ if ($this->resource instanceof Service) {
+ app(DeleteService::class)->deleteLocal($this->resource);
+
+ return;
+ }
+
if ($this->deleteVolumes) {
- $this->resource->deleteVolumes();
$this->resource->persistentStorages()->delete();
}
- $this->resource->fileStorages()->delete(); // these are file mounts which should probably have their own flag
+ $this->resource->fileStorages()->delete();
- $isDatabase = $this->resource instanceof StandalonePostgresql
+ if ($this->isDatabase()) {
+ $this->resource->sslCertificates()->delete();
+ $this->resource->scheduledBackups()->delete();
+ $this->resource->tags()->detach();
+ }
+ $this->resource->environment_variables()->delete();
+ $this->resource->forceDelete();
+ });
+
+ Artisan::queue('cleanup:stucked-resources');
+ }
+
+ private function isDatabase(): bool
+ {
+ return $this->resource instanceof StandalonePostgresql
|| $this->resource instanceof StandaloneRedis
|| $this->resource instanceof StandaloneMongodb
|| $this->resource instanceof StandaloneMysql
@@ -95,29 +143,6 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue
|| $this->resource instanceof StandaloneKeydb
|| $this->resource instanceof StandaloneDragonfly
|| $this->resource instanceof StandaloneClickhouse;
-
- if ($isDatabase) {
- $this->resource->sslCertificates()->delete();
- $this->resource->scheduledBackups()->delete();
- $this->resource->tags()->detach();
- }
- $this->resource->environment_variables()->delete();
-
- if ($this->deleteConnectedNetworks && $this->resource->type() === 'application') {
- $this->resource->deleteConnectedNetworks();
- }
- } catch (\Throwable $e) {
- throw $e;
- } finally {
- $this->resource->forceDelete();
- if ($this->dockerCleanup) {
- $server = data_get($this->resource, 'server') ?? data_get($this->resource, 'destination.server');
- if ($server) {
- CleanupDocker::dispatch($server, false, false);
- }
- }
- Artisan::queue('cleanup:stucked-resources');
- }
}
private function deleteScheduledVolumeBackups(): void
diff --git a/resources/views/components/two-factor-badge.blade.php b/resources/views/components/two-factor-badge.blade.php
index e43201c140..0ed0a4bef8 100644
--- a/resources/views/components/two-factor-badge.blade.php
+++ b/resources/views/components/two-factor-badge.blade.php
@@ -15,11 +15,7 @@
d="M9 12.75 11.25 15 15 9.75m-3-7.036A11.959 11.959 0 0 1 3.598 6 11.99 11.99 0 0 0 3 9.749c0 5.592 3.824 10.29 9 11.622 5.176-1.332 9-6.03 9-11.622 0-1.31-.21-2.571-.598-3.751h-.152c-3.196 0-6.1-1.248-8.25-3.285Z" />
@else
-
+
@endif
Two-factor authentication is {{ $enabled ? 'enabled' : 'disabled' }}
diff --git a/tests/Feature/DeleteResourceJobAtomicityTest.php b/tests/Feature/DeleteResourceJobAtomicityTest.php
new file mode 100644
index 0000000000..11955a82cd
--- /dev/null
+++ b/tests/Feature/DeleteResourceJobAtomicityTest.php
@@ -0,0 +1,64 @@
+ InstanceSettings::firstOrCreate(['id' => 0]));
+
+ $team = Team::factory()->create();
+ $server = Server::factory()->create(['team_id' => $team->id]);
+ $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->application = Application::factory()->create([
+ 'environment_id' => $environment->id,
+ 'destination_id' => $destination->id,
+ 'destination_type' => $destination->getMorphClass(),
+ ]);
+ $this->storage = $this->application->persistentStorages()->create([
+ 'name' => 'delete-resource-job-test',
+ 'mount_path' => '/data',
+ 'host_path' => null,
+ ]);
+
+ $this->application->delete();
+ Queue::fake();
+});
+
+it('deletes the Coolify resource when remote cleanup fails', function () {
+ Process::fake(['*' => Process::result(errorOutput: 'SSH connection timed out', exitCode: 255)]);
+
+ (new DeleteResourceJob($this->application))->handle();
+
+ expect(Application::withTrashed()->find($this->application->id))->toBeNull();
+});
+
+it('rolls back local metadata deletion when deleting the resource fails', function () {
+ Process::fake(['*' => Process::result(output: '')]);
+ $applicationUuid = $this->application->uuid;
+ Application::deleting(function (Application $application) use ($applicationUuid): void {
+ if ($application->uuid === $applicationUuid && $application->isForceDeleting()) {
+ throw new RuntimeException('Local deletion failed.');
+ }
+ });
+
+ expect(fn () => (new DeleteResourceJob($this->application))->handle())
+ ->toThrow(RuntimeException::class, 'Local deletion failed.');
+
+ expect($this->storage->fresh())->not->toBeNull()
+ ->and(Application::withTrashed()->find($this->application->id))->not->toBeNull();
+});
diff --git a/tests/Feature/TeamMemberTwoFactorStatusTest.php b/tests/Feature/TeamMemberTwoFactorStatusTest.php
index 0caaf4207f..d9ef52295d 100644
--- a/tests/Feature/TeamMemberTwoFactorStatusTest.php
+++ b/tests/Feature/TeamMemberTwoFactorStatusTest.php
@@ -91,6 +91,7 @@ test('the member row renders a disabled badge when two factor is not configured'
Livewire::test(Member::class, ['member' => $memberWithoutTwoFactor])
->assertSee('Two-factor authentication is disabled')
+ ->assertSeeHtml('M18.4697 19.5303')
->assertDontSee('Two-factor authentication is enabled');
});