diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 145f08093c..af4cd2c264 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -5615,8 +5615,31 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); } $this->updateDeploymentStatus($status); - $this->handleStatusTransition($status); - queue_next_deployment($this->application); + + // The status is stored. Side effect failures are logged so they cannot flip a finished + // deployment to failed or stop the next queued deployment from starting. + try { + $this->handleStatusTransition($status); + } catch (Throwable $e) { + $this->logStatusTransitionSideEffectFailure("Post-{$status->value} actions failed", $e); + } + + try { + queue_next_deployment($this->application); + } catch (Throwable $e) { + $this->logStatusTransitionSideEffectFailure('Starting the next queued deployment failed', $e); + } + } + + private function logStatusTransitionSideEffectFailure(string $context, Throwable $e): void + { + \Log::warning("{$context} for deployment {$this->deployment_uuid}: {$e->getMessage()}"); + + try { + $this->application_deployment_queue->addLogEntry("Warning: {$context}: {$e->getMessage()}", 'stderr'); + } catch (Throwable) { + // The warning is already in the application log. + } } /** @@ -5845,6 +5868,10 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); // 69420 means failed to push the image to the registry, so we don't need to remove the new version as it is the currently running one if ($this->application->settings->is_consistent_container_name_enabled || $this->pull_request_id !== 0) { // do not remove already running container for PR deployments + } elseif ($this->newVersionIsHealthy) { + // The new container passed its health check, so the old container may already be removed. + // A later failure (additional destinations, queue advancement, ...) must not remove the only running version. + $this->application_deployment_queue->addLogEntry('Deployment failed after the new version became healthy. Keeping the new version of your application running.', 'stderr'); } else { $this->application_deployment_queue->addLogEntry('Deployment failed. Removing the new version of your application.', 'stderr'); $this->removeContainerWithTimeout($this->container_name); diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index a3a5417b4e..890a3dac48 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -6,10 +6,13 @@ use App\Jobs\ApplicationDeploymentJob; use App\Jobs\VolumeCloneJob; use App\Models\Application; use App\Models\ApplicationDeploymentQueue; +use App\Models\ApplicationPreview; use App\Models\EnvironmentVariable; use App\Models\Server; use App\Models\StandaloneDocker; +use App\Notifications\Application\DeploymentFailed; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use Spatie\Url\Url; function queue_application_deployment(Application $application, string $deployment_uuid, ?int $pull_request_id = 0, ?string $commit = null, bool $force_rebuild = false, bool $is_webhook = false, bool $is_api = false, bool $restart_only = false, ?string $git_type = null, bool $no_questions_asked = false, ?Server $server = null, ?StandaloneDocker $destination = null, bool $only_this_server = false, bool $rollback = false, ?string $docker_registry_image_tag = null, ?string $parent_deployment_uuid = null) @@ -173,13 +176,58 @@ function start_queued_deployment(ApplicationDeploymentQueue $deployment, bool $f return true; }); - if ($started) { + if (! $started) { + return false; + } + + try { ApplicationDeploymentJob::dispatch( application_deployment_queue_id: $deployment->id, ); + } catch (Throwable $exception) { + fail_undispatchable_deployment($deployment, $exception); + + return false; } - return $started; + return true; +} + +/** + * Fail a deployment whose job could not be created or queued, for example a deployment queued + * before an upgrade with a commit that no longer validates. Leaving it in progress would block + * every later deployment of the application and stop the queue advancement loop. + */ +function fail_undispatchable_deployment(ApplicationDeploymentQueue $deployment, Throwable $exception): void +{ + $failed = ApplicationDeploymentQueue::query() + ->whereKey($deployment->id) + ->where('status', ApplicationDeploymentStatus::IN_PROGRESS->value) + ->update(['status' => ApplicationDeploymentStatus::FAILED->value]); + + if ($failed === 0) { + return; + } + + Log::warning("Deployment {$deployment->deployment_uuid} could not be started: {$exception->getMessage()}"); + + $deployment->refresh(); + $deployment->addLogEntry('========================================', 'stderr'); + $deployment->addLogEntry("Deployment could not be started: {$exception->getMessage()}", 'stderr'); + $deployment->addLogEntry('========================================', 'stderr'); + + try { + $application = Application::query()->find($deployment->application_id); + if (! $application || filled($deployment->parent_deployment_uuid)) { + return; + } + $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()}"); + } } function next_queuable(string $server_id, string $application_id, string $commit = 'HEAD', int $pull_request_id = 0): bool diff --git a/tests/Feature/ApplicationDeploymentStatusSideEffectsTest.php b/tests/Feature/ApplicationDeploymentStatusSideEffectsTest.php new file mode 100644 index 0000000000..c0355e1ee3 --- /dev/null +++ b/tests/Feature/ApplicationDeploymentStatusSideEffectsTest.php @@ -0,0 +1,71 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + + $team = Team::factory()->create(); + $this->server = Server::factory()->create(['team_id' => $team->id]); + $this->destination = StandaloneDocker::where('server_id', $this->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' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + Process::fake(['*' => Process::result(output: '')]); + Queue::fake(); + Notification::fake(); +}); + +function createDeploymentForSideEffectsTest(string $status, string $createdAt): ApplicationDeploymentQueue +{ + $deployment = ApplicationDeploymentQueue::create([ + 'application_id' => test()->application->id, + 'deployment_uuid' => fake()->uuid(), + 'status' => $status, + 'server_id' => test()->server->id, + 'destination_id' => test()->destination->id, + 'commit' => 'HEAD', + 'pull_request_id' => 0, + ]); + $deployment->forceFill(['created_at' => $createdAt])->save(); + + return $deployment; +} + +test('a failing post-success side effect keeps the deployment finished and starts the next queued deployment', function () { + $current = createDeploymentForSideEffectsTest('in_progress', '2026-10-01 10:00:00'); + $next = createDeploymentForSideEffectsTest('queued', '2026-10-01 10:01:00'); + Event::listen(ApplicationConfigurationChanged::class, fn () => throw new RuntimeException('broadcast failed')); + + $job = new ApplicationDeploymentJob($current->id); + (fn () => $this->completeDeployment())->call($job); + + $current->refresh(); + expect($current->status)->toBe('finished') + ->and($current->logs)->toContain('broadcast failed') + ->and($next->fresh()->status)->toBe('in_progress'); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $next->id); +}); diff --git a/tests/Feature/QueuedDeploymentInvalidCommitTest.php b/tests/Feature/QueuedDeploymentInvalidCommitTest.php new file mode 100644 index 0000000000..c31d822931 --- /dev/null +++ b/tests/Feature/QueuedDeploymentInvalidCommitTest.php @@ -0,0 +1,89 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->team->emailNotificationSettings->update([ + 'smtp_enabled' => true, + 'deployment_failure_email_notifications' => true, + ]); + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->server->settings->update([ + 'is_reachable' => true, + 'is_usable' => true, + 'force_disabled' => false, + ]); + $this->destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $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->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + Process::fake(['*' => Process::result(output: '')]); + Queue::fake(); + Notification::fake(); +}); + +function createQueuedDeploymentWithCommit(string $commit, string $createdAt): ApplicationDeploymentQueue +{ + $deployment = ApplicationDeploymentQueue::create([ + 'application_id' => test()->application->id, + 'deployment_uuid' => fake()->uuid(), + 'status' => 'queued', + 'server_id' => test()->server->id, + 'destination_id' => test()->destination->id, + 'commit' => $commit, + 'pull_request_id' => 0, + ]); + $deployment->forceFill(['created_at' => $createdAt])->save(); + + return $deployment; +} + +test('a queued deployment with an invalid commit fails and the next queued deployment starts', function () { + $invalidDeployment = createQueuedDeploymentWithCommit('abc;touch /tmp/pwned', '2026-10-01 10:00:00'); + $validDeployment = createQueuedDeploymentWithCommit('HEAD', '2026-10-01 10:01:00'); + + queue_next_deployment($this->application); + + $invalidDeployment->refresh(); + expect($invalidDeployment->status)->toBe('failed') + ->and($invalidDeployment->logs)->toContain('Deployment could not be started') + ->and($invalidDeployment->logs)->toContain('Invalid deployment commit') + ->and($validDeployment->fresh()->status)->toBe('in_progress'); + + Queue::assertPushed(ApplicationDeploymentJob::class, 1); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $validDeployment->id); + Notification::assertSentTo($this->team, DeploymentFailed::class); +}); + +test('force starting a queued deployment with an invalid commit fails it instead of leaving it in progress', function () { + $invalidDeployment = createQueuedDeploymentWithCommit('$(id)', '2026-10-01 10:00:00'); + + expect(force_start_deployment($invalidDeployment))->toBeFalse() + ->and($invalidDeployment->fresh()->status)->toBe('failed'); + Queue::assertNothingPushed(); +}); diff --git a/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php b/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php index 514d21fe14..39d5b5c278 100644 --- a/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php +++ b/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php @@ -8,7 +8,7 @@ use App\Models\ApplicationSetting; /** * @return array> */ -function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $customInternalName, int $pullRequestId): array +function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $customInternalName, int $pullRequestId, bool $newVersionIsHealthy = false): array { $settings = new ApplicationSetting; $settings->setRawAttributes([ @@ -37,6 +37,7 @@ function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $cust $reflection->getProperty('application')->setValue($job, $application); $reflection->getProperty('pull_request_id')->setValue($job, $pullRequestId); $reflection->getProperty('container_name')->setValue($job, 'new-container-20261001T120000'); + $reflection->getProperty('newVersionIsHealthy')->setValue($job, $newVersionIsHealthy); $job->failed(new RuntimeException('Health check failed')); @@ -61,3 +62,14 @@ it('keeps the running container when consistent container names are enabled', fu it('keeps the running container for pull request deployments', function () { expect(runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: 'my-api', pullRequestId: 12))->toBeEmpty(); }); + +it('keeps the new container when the deployment fails after the new container became healthy', function () { + expect(runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: null, pullRequestId: 0, newVersionIsHealthy: true))->toBeEmpty(); +}); + +it('removes the new container when the deployment fails before the new container became healthy', function () { + $remoteCommands = runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: null, pullRequestId: 0, newVersionIsHealthy: false); + + expect($remoteCommands)->toHaveCount(1) + ->and($remoteCommands[0][0])->toContain('new-container-20261001T120000'); +});