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.
This commit is contained in:
Andras Bacsai
2026-08-20 11:10:14 +02:00
parent 8f90882875
commit 38502e8205
5 changed files with 168 additions and 107 deletions
+35 -60
View File
@@ -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();
}
}
+67 -42
View File
@@ -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
@@ -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" />
</svg>
@else
<svg class="size-4 shrink-0" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"
stroke-width="1.5" stroke="currentColor" aria-hidden="true">
<path stroke-linecap="round" stroke-linejoin="round"
d="M12 9v3.75m0-10.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.285Zm0 13.036h.008v.008H12v-.008Z" />
</svg>
<x-reicon name="x" class="size-4 shrink-0" />
@endif
<span class="sr-only">Two-factor authentication is {{ $enabled ? 'enabled' : 'disabled' }}</span>
</span>
@@ -0,0 +1,64 @@
<?php
use App\Jobs\DeleteResourceJob;
use App\Models\Application;
use App\Models\Environment;
use App\Models\InstanceSettings;
use App\Models\Project;
use App\Models\Server;
use App\Models\StandaloneDocker;
use App\Models\Team;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Process;
use Illuminate\Support\Facades\Queue;
uses(RefreshDatabase::class);
beforeEach(function () {
InstanceSettings::unguarded(fn () => 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();
});
@@ -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');
});