From 203b8804f08c376ab8e11d69628243ec35f1eb7e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:29:04 +0200 Subject: [PATCH] fix(deployments): continue the queue when a deployment cannot be dispatched - A dispatch failure for one additional server no longer skips the other servers or the combined notification - After a deployment fails to dispatch, the queue starts the next queued deployment; nested queue advances run in a loop, not as recursion, so repeated failures end --- app/Jobs/ApplicationDeploymentJob.php | 37 ++-- bootstrap/helpers/applications.php | 49 ++++- ...loymentDispatchFailureQueueAdvanceTest.php | 174 ++++++++++++++++++ 3 files changed, 240 insertions(+), 20 deletions(-) create mode 100644 tests/Feature/DeploymentDispatchFailureQueueAdvanceTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 36fb321fa6..c8641240cc 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2807,7 +2807,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } /** - * @return int The number of deployments that Coolify queued on additional servers + * @return int The number of deployments that Coolify created on additional servers, including ones that failed to dispatch */ private function deploy_to_additional_destinations(): int { @@ -2839,18 +2839,29 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue continue; } $deployment_uuid = new_public_id(); - // Deploy the exact commit and image tag of the main server, also for a rollback. - $result = queue_application_deployment( - deployment_uuid: $deployment_uuid, - application: $this->application, - commit: $this->commit, - no_questions_asked: true, - server: $server, - destination: $destination, - rollback: $this->rollback, - docker_registry_image_tag: $this->application_deployment_queue->docker_registry_image_tag, - parent_deployment_uuid: $this->deployment_uuid, - ); + try { + // Deploy the exact commit and image tag of the main server, also for a rollback. + $result = queue_application_deployment( + deployment_uuid: $deployment_uuid, + application: $this->application, + commit: $this->commit, + no_questions_asked: true, + server: $server, + destination: $destination, + rollback: $this->rollback, + docker_registry_image_tag: $this->application_deployment_queue->docker_registry_image_tag, + parent_deployment_uuid: $this->deployment_uuid, + ); + } catch (Throwable $e) { + \Log::warning("Deployment {$this->deployment_uuid} could not queue the deployment to {$server->name}: {$e->getMessage()}"); + $this->application_deployment_queue->addLogEntry("Deployment to {$server->name} could not be started: {$e->getMessage()}", 'stderr'); + // A deployment that failed to dispatch is already failed; it counts for the combined notification. + if (ApplicationDeploymentQueue::query()->where('deployment_uuid', $deployment_uuid)->exists()) { + $queued++; + } + + continue; + } if ($result['status'] !== 'queued') { $this->application_deployment_queue->addLogEntry("Deployment to {$server->name} was not queued: {$result['message']}", 'stderr'); diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index c2a012692f..2f57eb8a9b 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -146,8 +146,33 @@ function force_start_deployment(ApplicationDeploymentQueue $deployment): bool * Start the queued deployments that can run now on the application's primary server, on the * server of the deployment that just ended (an additional server), and on every other server * with a queued deployment of this application, because those waited for this one to end. + * + * A deployment that fails to dispatch advances the queue again. A call made while the queue + * is already advancing is run after the current pass instead of recursing. Each such call + * needs a deployment that moved from queued to failed, so the loop ends. */ function queue_next_deployment(Application $application, ?int $finished_deployment_server_id = null) +{ + static $advancing = false; + static $pending = []; + + $pending[] = [$application, $finished_deployment_server_id]; + if ($advancing) { + return; + } + + $advancing = true; + try { + while ($next = array_shift($pending)) { + start_next_queued_deployments(...$next); + } + } finally { + $advancing = false; + $pending = []; + } +} + +function start_next_queued_deployments(Application $application, ?int $finished_deployment_server_id = null): void { $application_queued_server_ids = ApplicationDeploymentQueue::where('application_id', $application->id) ->where('status', ApplicationDeploymentStatus::QUEUED) @@ -240,18 +265,28 @@ function fail_undispatchable_deployment(ApplicationDeploymentQueue $deployment, $deployment->addLogEntry("Deployment could not be started: {$exception->getMessage()}", 'stderr'); $deployment->addLogEntry('========================================', 'stderr'); + $application = Application::query()->find($deployment->application_id); + if (! $application) { + return; + } + try { - $application = Application::query()->find($deployment->application_id); - if (! $application || filled($deployment->parent_deployment_uuid)) { - return; + if (blank($deployment->parent_deployment_uuid)) { + $preview = $deployment->pull_request_id !== 0 + ? ApplicationPreview::findPreviewByApplicationAndPullId($application->id, $deployment->pull_request_id) + : null; + $application->environment?->project?->team?->notify(new DeploymentFailed($application, $deployment->deployment_uuid, $preview)); } - $preview = $deployment->pull_request_id !== 0 - ? ApplicationPreview::findPreviewByApplicationAndPullId($application->id, $deployment->pull_request_id) - : null; - $application->environment?->project?->team?->notify(new DeploymentFailed($application, $deployment->deployment_uuid, $preview)); } catch (Throwable $notificationException) { Log::warning("Failed to send the failure notification for deployment {$deployment->deployment_uuid}: {$notificationException->getMessage()}"); } + + // The failed deployment no longer holds its build slot, so the deployments it blocked can start. + try { + queue_next_deployment($application, $deployment->server_id); + } catch (Throwable $queueException) { + Log::warning("Starting the next queued deployment after {$deployment->deployment_uuid} failed: {$queueException->getMessage()}"); + } } function next_queuable(string $server_id, string $application_id, string $commit = 'HEAD', int $pull_request_id = 0): bool diff --git a/tests/Feature/DeploymentDispatchFailureQueueAdvanceTest.php b/tests/Feature/DeploymentDispatchFailureQueueAdvanceTest.php new file mode 100644 index 0000000000..f1b2ffa985 --- /dev/null +++ b/tests/Feature/DeploymentDispatchFailureQueueAdvanceTest.php @@ -0,0 +1,174 @@ +shouldFail)(ApplicationDeploymentQueue::query()->findOrFail($job->application_deployment_queue_id))) { + throw new RuntimeException('queue connection refused'); + } + + return parent::push($job, $data, $queue); + } + }; + $fake->shouldFail = $shouldFail; + Queue::swap($fake); + + return $fake; +} + +beforeEach(function () { + Server::flushIdentityMap(); + Notification::fake(); + config()->set('cache.default', 'array'); + InstanceSettings::unguarded(fn () => InstanceSettings::query()->firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->team->emailNotificationSettings->update([ + 'smtp_enabled' => true, + 'deployment_failure_email_notifications' => true, + ]); + $makeServer = function (string $name): Server { + $server = Server::factory()->create(['team_id' => $this->team->id, 'name' => $name]); + $server->settings->update(['concurrent_builds' => 1]); + + return $server; + }; + $this->mainServer = $makeServer('main-server'); + $this->mainDestination = StandaloneDocker::where('server_id', $this->mainServer->id)->firstOrFail(); + $this->firstExtraServer = $makeServer('first-extra-server'); + $this->secondExtraServer = $makeServer('second-extra-server'); + + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = $project->environments()->first() ?? Environment::factory()->create(['project_id' => $project->id]); + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $this->mainDestination->id, + 'destination_type' => $this->mainDestination->getMorphClass(), + 'build_pack' => 'dockerfile', + ]); + + $this->queueDeployment = fn (string $uuid, string $status = 'queued'): ApplicationDeploymentQueue => ApplicationDeploymentQueue::create([ + 'application_id' => $this->application->id, + 'deployment_uuid' => $uuid, + 'status' => $status, + 'server_id' => $this->mainServer->id, + 'destination_id' => $this->mainDestination->id, + 'commit' => 'HEAD', + 'pull_request_id' => 0, + 'only_this_server' => true, + ]); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +test('an additional server whose deployment cannot be dispatched does not stop the other additional servers', function () { + foreach ([$this->firstExtraServer, $this->secondExtraServer] as $server) { + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $this->application->additional_networks()->attach($destination->id, ['server_id' => $server->id]); + } + $this->application->refresh(); + $parent = ($this->queueDeployment)('dispatch-fail-parent', ApplicationDeploymentStatus::FINISHED->value); + fakeQueueFailingDeploymentDispatch(fn (ApplicationDeploymentQueue $deployment) => $deployment->server_id === $this->firstExtraServer->id); + + $reflection = new ReflectionClass(ApplicationDeploymentJob::class); + $job = $reflection->newInstanceWithoutConstructor(); + foreach ([ + 'application' => $this->application, + 'application_deployment_queue' => $parent, + 'deployment_uuid' => $parent->deployment_uuid, + 'server' => $this->mainServer, + 'mainServer' => $this->mainServer, + 'destination' => $this->mainDestination, + 'pull_request_id' => 0, + 'preview' => null, + 'only_this_server' => false, + 'commit' => 'HEAD', + 'rollback' => false, + ] as $name => $value) { + $reflection->getProperty($name)->setValue($job, $value); + } + + $reflection->getMethod('handleSuccessfulDeployment')->invoke($job); + + $failed = ApplicationDeploymentQueue::where('server_id', $this->firstExtraServer->id)->sole(); + $started = ApplicationDeploymentQueue::where('server_id', $this->secondExtraServer->id)->sole(); + expect($failed->status)->toBe(ApplicationDeploymentStatus::FAILED->value) + ->and($failed->logs)->toContain('queue connection refused') + ->and($started->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value) + ->and($started->parent_deployment_uuid)->toBe($parent->deployment_uuid) + ->and($parent->fresh()->logs)->toContain('queue connection refused') + ->and(Cache::has("multi-server-deployment-queued:{$parent->deployment_uuid}"))->toBeTrue(); + Queue::assertPushed(ApplicationDeploymentJob::class, 1); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $started->id); + // The summary waits for the second server, then reports the failed first server. + Notification::assertNothingSent(); +}); + +test('a deployment that cannot be dispatched starts the next queued deployment on its server', function () { + $waiting = ($this->queueDeployment)('dispatch-fail-waiting'); + $this->travel(1)->seconds(); + fakeQueueFailingDeploymentDispatch(fn (ApplicationDeploymentQueue $deployment) => $deployment->deployment_uuid === 'dispatch-fail-broken'); + + expect(fn () => queue_application_deployment($this->application, 'dispatch-fail-broken', no_questions_asked: true)) + ->toThrow(RuntimeException::class, 'queue connection refused'); + + expect(ApplicationDeploymentQueue::where('deployment_uuid', 'dispatch-fail-broken')->value('status'))->toBe(ApplicationDeploymentStatus::FAILED->value) + ->and($waiting->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + Queue::assertPushed(ApplicationDeploymentJob::class, 1); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $waiting->id); + Notification::assertSentTo($this->team, DeploymentFailed::class, fn (DeploymentFailed $notification) => $notification->deployment_uuid === 'dispatch-fail-broken'); +}); + +test('when no deployment can be dispatched, each queued deployment fails once and the queue stops', function () { + $waiting = collect(range(1, 3))->map(function (int $number) { + $this->travel(1)->seconds(); + + return ($this->queueDeployment)("dispatch-fail-waiting-{$number}"); + }); + $this->travel(1)->seconds(); + fakeQueueFailingDeploymentDispatch(fn () => true); + + expect(fn () => queue_application_deployment($this->application, 'dispatch-fail-broken', no_questions_asked: true)) + ->toThrow(RuntimeException::class, 'queue connection refused'); + + expect(ApplicationDeploymentQueue::where('application_id', $this->application->id)->pluck('status')->unique()->all()) + ->toBe([ApplicationDeploymentStatus::FAILED->value]); + foreach ($waiting as $deployment) { + expect(substr_count($deployment->fresh()->logs, 'Deployment could not be started'))->toBe(1); + } + Queue::assertNothingPushed(); + Notification::assertSentToTimes($this->team, DeploymentFailed::class, 4); +});