diff --git a/app/Console/Commands/CleanupStuckedResources.php b/app/Console/Commands/CleanupStuckedResources.php index 165a3ae219..0874970fb6 100644 --- a/app/Console/Commands/CleanupStuckedResources.php +++ b/app/Console/Commands/CleanupStuckedResources.php @@ -13,7 +13,6 @@ use App\Models\Server; use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; -use App\Models\SslCertificate; use App\Models\StandaloneClickhouse; use App\Models\StandaloneDragonfly; use App\Models\StandaloneKeydb; @@ -39,13 +38,14 @@ class CleanupStuckedResources extends Command private function cleanup_stucked_resources() { try { - $teams = Team::all()->filter(function ($team) { - return $team->members()->count() === 0 && $team->servers()->count() === 0; - }); + $teams = Team::query() + ->whereDoesntHave('members') + ->whereDoesntHave('servers') + ->lazyById(); foreach ($teams as $team) { $team->delete(); } - $servers = Server::all()->filter(function ($server) { + $servers = Server::query()->with('team.subscription')->lazyById()->filter(function ($server) { return $server->isFunctional(); }); if (isCloud()) { @@ -60,7 +60,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stucked resources: {$e->getMessage()}\n"; } try { - $servers = Server::onlyTrashed()->get(); + $servers = Server::onlyTrashed()->lazyById(); foreach ($servers as $server) { echo "Force deleting stuck server: {$server->name}\n"; $server->forceDelete(); @@ -69,7 +69,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck servers: {$e->getMessage()}\n"; } try { - $applicationsDeploymentQueue = ApplicationDeploymentQueue::get(); + $applicationsDeploymentQueue = ApplicationDeploymentQueue::query()->lazyById(); foreach ($applicationsDeploymentQueue as $applicationDeploymentQueue) { if (is_null($applicationDeploymentQueue->application)) { echo "Deleting stuck application deployment queue: {$applicationDeploymentQueue->id}\n"; @@ -80,7 +80,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck application deployment queue: {$e->getMessage()}\n"; } try { - $applications = Application::withTrashed()->whereNotNull('deleted_at')->get(); + $applications = Application::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($applications as $application) { echo "Deleting stuck application: {$application->name}\n"; DeleteResourceJob::dispatch($application); @@ -89,18 +89,18 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck application: {$e->getMessage()}\n"; } try { - $applicationsPreviews = ApplicationPreview::get(); + $applicationsPreviews = ApplicationPreview::query() + ->whereDoesntHave('application') + ->lazyById(); foreach ($applicationsPreviews as $applicationPreview) { - if (! data_get($applicationPreview, 'application')) { - echo "Deleting stuck application preview: {$applicationPreview->uuid}\n"; - DeleteResourceJob::dispatch($applicationPreview); - } + echo "Deleting stuck application preview: {$applicationPreview->uuid}\n"; + DeleteResourceJob::dispatch($applicationPreview); } } catch (\Throwable $e) { echo "Error in cleaning stuck application: {$e->getMessage()}\n"; } try { - $applicationsPreviews = ApplicationPreview::withTrashed()->whereNotNull('deleted_at')->get(); + $applicationsPreviews = ApplicationPreview::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($applicationsPreviews as $applicationPreview) { echo "Deleting stuck application preview: {$applicationPreview->fqdn}\n"; DeleteResourceJob::dispatch($applicationPreview); @@ -109,7 +109,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck application: {$e->getMessage()}\n"; } try { - $postgresqls = StandalonePostgresql::withTrashed()->whereNotNull('deleted_at')->get(); + $postgresqls = StandalonePostgresql::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($postgresqls as $postgresql) { echo "Deleting stuck postgresql: {$postgresql->name}\n"; DeleteResourceJob::dispatch($postgresql); @@ -118,7 +118,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck postgresql: {$e->getMessage()}\n"; } try { - $rediss = StandaloneRedis::withTrashed()->whereNotNull('deleted_at')->get(); + $rediss = StandaloneRedis::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($rediss as $redis) { echo "Deleting stuck redis: {$redis->name}\n"; DeleteResourceJob::dispatch($redis); @@ -127,7 +127,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck redis: {$e->getMessage()}\n"; } try { - $keydbs = StandaloneKeydb::withTrashed()->whereNotNull('deleted_at')->get(); + $keydbs = StandaloneKeydb::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($keydbs as $keydb) { echo "Deleting stuck keydb: {$keydb->name}\n"; DeleteResourceJob::dispatch($keydb); @@ -136,7 +136,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck keydb: {$e->getMessage()}\n"; } try { - $dragonflies = StandaloneDragonfly::withTrashed()->whereNotNull('deleted_at')->get(); + $dragonflies = StandaloneDragonfly::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($dragonflies as $dragonfly) { echo "Deleting stuck dragonfly: {$dragonfly->name}\n"; DeleteResourceJob::dispatch($dragonfly); @@ -145,7 +145,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck dragonfly: {$e->getMessage()}\n"; } try { - $clickhouses = StandaloneClickhouse::withTrashed()->whereNotNull('deleted_at')->get(); + $clickhouses = StandaloneClickhouse::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($clickhouses as $clickhouse) { echo "Deleting stuck clickhouse: {$clickhouse->name}\n"; DeleteResourceJob::dispatch($clickhouse); @@ -154,7 +154,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck clickhouse: {$e->getMessage()}\n"; } try { - $mongodbs = StandaloneMongodb::withTrashed()->whereNotNull('deleted_at')->get(); + $mongodbs = StandaloneMongodb::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($mongodbs as $mongodb) { echo "Deleting stuck mongodb: {$mongodb->name}\n"; DeleteResourceJob::dispatch($mongodb); @@ -163,7 +163,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck mongodb: {$e->getMessage()}\n"; } try { - $mysqls = StandaloneMysql::withTrashed()->whereNotNull('deleted_at')->get(); + $mysqls = StandaloneMysql::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($mysqls as $mysql) { echo "Deleting stuck mysql: {$mysql->name}\n"; DeleteResourceJob::dispatch($mysql); @@ -172,7 +172,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck mysql: {$e->getMessage()}\n"; } try { - $mariadbs = StandaloneMariadb::withTrashed()->whereNotNull('deleted_at')->get(); + $mariadbs = StandaloneMariadb::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($mariadbs as $mariadb) { echo "Deleting stuck mariadb: {$mariadb->name}\n"; DeleteResourceJob::dispatch($mariadb); @@ -181,7 +181,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck mariadb: {$e->getMessage()}\n"; } try { - $services = Service::withTrashed()->whereNotNull('deleted_at')->get(); + $services = Service::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($services as $service) { echo "Deleting stuck service: {$service->name}\n"; DeleteResourceJob::dispatch($service); @@ -190,7 +190,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck service: {$e->getMessage()}\n"; } try { - $serviceApps = ServiceApplication::withTrashed()->whereNotNull('deleted_at')->get(); + $serviceApps = ServiceApplication::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($serviceApps as $serviceApp) { echo "Deleting stuck serviceapp: {$serviceApp->name}\n"; $serviceApp->forceDelete(); @@ -199,7 +199,7 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck serviceapp: {$e->getMessage()}\n"; } try { - $serviceDbs = ServiceDatabase::withTrashed()->whereNotNull('deleted_at')->get(); + $serviceDbs = ServiceDatabase::withTrashed()->whereNotNull('deleted_at')->lazyById(); foreach ($serviceDbs as $serviceDb) { echo "Deleting stuck serviceapp: {$serviceDb->name}\n"; $serviceDb->forceDelete(); @@ -208,19 +208,27 @@ class CleanupStuckedResources extends Command echo "Error in cleaning stuck serviceapp: {$e->getMessage()}\n"; } try { - $scheduled_tasks = ScheduledTask::all(); + $scheduled_tasks = ScheduledTask::query() + ->where(function ($query): void { + $query->where(function ($query): void { + $query->whereNull('application_id')->whereNull('service_id'); + })->orWhere(function ($query): void { + $query->whereNotNull('application_id')->whereDoesntHave('application'); + })->orWhere(function ($query): void { + $query->whereNotNull('service_id')->whereDoesntHave('service'); + }); + }) + ->lazyById(); foreach ($scheduled_tasks as $scheduled_task) { - if (! $scheduled_task->service && ! $scheduled_task->application) { - echo "Deleting stuck scheduledtask: {$scheduled_task->name}\n"; - $scheduled_task->delete(); - } + echo "Deleting stuck scheduledtask: {$scheduled_task->name}\n"; + $scheduled_task->delete(); } } catch (\Throwable $e) { echo "Error in cleaning stuck scheduledtasks: {$e->getMessage()}\n"; } try { - $scheduled_backups = ScheduledDatabaseBackup::all(); + $scheduled_backups = ScheduledDatabaseBackup::query()->lazyById(); foreach ($scheduled_backups as $scheduled_backup) { try { $server = $scheduled_backup->server(); @@ -238,7 +246,7 @@ class CleanupStuckedResources extends Command // Cleanup any resources that are not attached to any environment or destination or server try { - $applications = Application::all(); + $applications = Application::query()->lazyById(); foreach ($applications as $application) { if (! data_get($application, 'environment')) { echo 'Application without environment: '.$application->name.'\n'; @@ -263,7 +271,7 @@ class CleanupStuckedResources extends Command echo "Error in application: {$e->getMessage()}\n"; } try { - $postgresqls = StandalonePostgresql::all()->where('id', '!=', 0); + $postgresqls = StandalonePostgresql::query()->where('id', '!=', 0)->lazyById(); foreach ($postgresqls as $postgresql) { if (! data_get($postgresql, 'environment')) { echo 'Postgresql without environment: '.$postgresql->name.'\n'; @@ -288,7 +296,7 @@ class CleanupStuckedResources extends Command echo "Error in postgresql: {$e->getMessage()}\n"; } try { - $redis = StandaloneRedis::all(); + $redis = StandaloneRedis::query()->lazyById(); foreach ($redis as $redis) { if (! data_get($redis, 'environment')) { echo 'Redis without environment: '.$redis->name.'\n'; @@ -314,7 +322,7 @@ class CleanupStuckedResources extends Command } try { - $mongodbs = StandaloneMongodb::all(); + $mongodbs = StandaloneMongodb::query()->lazyById(); foreach ($mongodbs as $mongodb) { if (! data_get($mongodb, 'environment')) { echo 'Mongodb without environment: '.$mongodb->name.'\n'; @@ -340,7 +348,7 @@ class CleanupStuckedResources extends Command } try { - $mysqls = StandaloneMysql::all(); + $mysqls = StandaloneMysql::query()->lazyById(); foreach ($mysqls as $mysql) { if (! data_get($mysql, 'environment')) { echo 'Mysql without environment: '.$mysql->name.'\n'; @@ -366,7 +374,7 @@ class CleanupStuckedResources extends Command } try { - $mariadbs = StandaloneMariadb::all(); + $mariadbs = StandaloneMariadb::query()->lazyById(); foreach ($mariadbs as $mariadb) { if (! data_get($mariadb, 'environment')) { echo 'Mariadb without environment: '.$mariadb->name.'\n'; @@ -392,7 +400,7 @@ class CleanupStuckedResources extends Command } try { - $services = Service::all(); + $services = Service::query()->lazyById(); foreach ($services as $service) { if (! data_get($service, 'environment')) { echo 'Service without environment: '.$service->name.'\n'; @@ -417,43 +425,23 @@ class CleanupStuckedResources extends Command echo "Error in service: {$e->getMessage()}\n"; } try { - $serviceApplications = ServiceApplication::all(); + $serviceApplications = ServiceApplication::query()->whereDoesntHave('service')->lazyById(); foreach ($serviceApplications as $service) { - if (! data_get($service, 'service')) { - echo 'ServiceApplication without service: '.$service->name.'\n'; - $service->forceDelete(); - - continue; - } + echo 'ServiceApplication without service: '.$service->name.'\n'; + $service->forceDelete(); } } catch (\Throwable $e) { echo "Error in serviceApplications: {$e->getMessage()}\n"; } try { - $serviceDatabases = ServiceDatabase::all(); + $serviceDatabases = ServiceDatabase::query()->whereDoesntHave('service')->lazyById(); foreach ($serviceDatabases as $service) { - if (! data_get($service, 'service')) { - echo 'ServiceDatabase without service: '.$service->name.'\n'; - $service->forceDelete(); - - continue; - } + echo 'ServiceDatabase without service: '.$service->name.'\n'; + $service->forceDelete(); } } catch (\Throwable $e) { echo "Error in ServiceDatabases: {$e->getMessage()}\n"; } - try { - $orphanedCerts = SslCertificate::whereNotIn('server_id', function ($query) { - $query->select('id')->from('servers'); - })->get(); - - foreach ($orphanedCerts as $cert) { - echo "Deleting orphaned SSL certificate: {$cert->id} (server_id: {$cert->server_id})\n"; - $cert->delete(); - } - } catch (\Throwable $e) { - echo "Error in cleaning orphaned SSL certificates: {$e->getMessage()}\n"; - } } } diff --git a/app/Jobs/DeleteResourceJob.php b/app/Jobs/DeleteResourceJob.php index dff7d88de1..e6f63ec669 100644 --- a/app/Jobs/DeleteResourceJob.php +++ b/app/Jobs/DeleteResourceJob.php @@ -47,9 +47,7 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue public function handle(): void { if ($this->resource instanceof ApplicationPreview) { - DB::transaction(function (): void { - $this->deleteApplicationPreview(); - }); + $this->deleteApplicationPreview(); return; } @@ -99,17 +97,17 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue ]); } - 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(), - ]); - } + 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(), + ]); + } + DB::transaction(function (): void { if ($this->resource instanceof Service) { app(DeleteService::class)->deleteLocal($this->resource); @@ -163,10 +161,22 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue } } - private function deleteApplicationPreview() + private function deleteApplicationPreview(): void { $application = $this->resource->application; - $server = $application->destination->server; + + if (! $application) { + $this->deleteApplicationPreviewLocally(); + + return; + } + + $server = $application->destination?->server; + if (! $server) { + $this->deleteApplicationPreviewLocally(); + + return; + } $pull_request_id = $this->resource->pull_request_id; // Ensure the preview is soft deleted (may already be done in Livewire component) @@ -239,6 +249,14 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue $this->resource->forceDelete(); } + private function deleteApplicationPreviewLocally(): void + { + DB::transaction(function (): void { + $this->resource->persistentStorages()->delete(); + ApplicationPreview::withoutEvents(fn () => $this->resource->forceDelete()); + }); + } + private function stopPreviewContainers(array $containers, $server, int $timeout = 30) { if (empty($containers)) { diff --git a/tests/Feature/CleanupStuckedResourcesTest.php b/tests/Feature/CleanupStuckedResourcesTest.php new file mode 100644 index 0000000000..24229cf3d1 --- /dev/null +++ b/tests/Feature/CleanupStuckedResourcesTest.php @@ -0,0 +1,44 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + Queue::fake(); + + $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]); + $application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + $validTask = ScheduledTask::factory()->create([ + 'team_id' => $team->id, + 'application_id' => $application->id, + ]); + $orphanedTask = ScheduledTask::factory()->create([ + 'team_id' => $team->id, + 'application_id' => PHP_INT_MAX, + ]); + + $this->artisan('cleanup:stucked-resources')->assertSuccessful(); + + expect($validTask->fresh())->not->toBeNull() + ->and($orphanedTask->fresh())->toBeNull(); +}); diff --git a/tests/Feature/DeleteResourceJobAtomicityTest.php b/tests/Feature/DeleteResourceJobAtomicityTest.php index 11955a82cd..758227004d 100644 --- a/tests/Feature/DeleteResourceJobAtomicityTest.php +++ b/tests/Feature/DeleteResourceJobAtomicityTest.php @@ -2,13 +2,16 @@ use App\Jobs\DeleteResourceJob; use App\Models\Application; +use App\Models\ApplicationPreview; use App\Models\Environment; use App\Models\InstanceSettings; use App\Models\Project; +use App\Models\ScheduledVolumeBackup; use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\Team; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Process; use Illuminate\Support\Facades\Queue; @@ -62,3 +65,47 @@ it('rolls back local metadata deletion when deleting the resource fails', functi expect($this->storage->fresh())->not->toBeNull() ->and(Application::withTrashed()->find($this->application->id))->not->toBeNull(); }); + +it('deletes scheduled volume backups outside the local deletion transaction', function () { + $backup = $this->storage->scheduledBackups()->create([ + 'team_id' => $this->application->environment->project->team_id, + 'frequency' => 'daily', + 'timeout' => 3600, + ]); + $transactionLevel = DB::transactionLevel(); + $deletingTransactionLevel = null; + + ScheduledVolumeBackup::deleting(function (ScheduledVolumeBackup $deletingBackup) use ($backup, &$deletingTransactionLevel): void { + if ($deletingBackup->is($backup)) { + $deletingTransactionLevel = DB::transactionLevel(); + } + }); + Process::fake(['*' => Process::result(output: '')]); + + (new DeleteResourceJob($this->application))->handle(); + + expect($deletingTransactionLevel)->toBe($transactionLevel); +}); + +it('deletes preview metadata locally when its application destination is missing', function () { + $this->application->restore(); + $this->application->update(['destination_id' => PHP_INT_MAX]); + $preview = ApplicationPreview::create([ + 'uuid' => 'preview-without-destination', + 'application_id' => $this->application->id, + 'pull_request_id' => 45, + 'pull_request_html_url' => 'https://github.com/coollabsio/coolify/pull/45', + ]); + $previewStorage = $preview->persistentStorages()->create([ + 'name' => 'preview-without-destination-data', + 'mount_path' => '/preview-data', + 'host_path' => null, + ]); + Process::fake(); + + (new DeleteResourceJob($preview))->handle(); + + Process::assertNothingRan(); + expect(ApplicationPreview::withTrashed()->find($preview->id))->toBeNull() + ->and($previewStorage->fresh())->toBeNull(); +});