fix(deploy): keep the healthy new container when a rolling update fails late

Laravel runs failed() on a fresh copy of the job, so the in-memory
newVersionIsHealthy flag was always false there and a failure after the
old container stopped removed the only running container. Keep the flag
in the cache, keyed by the deployment UUID.

Closes #12253

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

app/Jobs/ApplicationDeploymentJob.php

@@ -87,8 +87,6 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
     public static int $batch_counter = 0;
 
-    private bool $newVersionIsHealthy = false;
-
     private ApplicationDeploymentQueue $application_deployment_queue;
 
     private Application $application;
@@ -2725,7 +2723,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
                 // Implement healthcheck for swarm
             } else {
                 if ($this->application->isHealthcheckDisabled() && $this->application->custom_healthcheck_found === false) {
-                    $this->newVersionIsHealthy = true;
+                    $this->markNewVersionHealthy(true);
 
                     return;
                 }
@@ -2773,12 +2771,12 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
                         $healthCheckStatus = $this->trimmedSavedOutput('health_check')->replace('"', '')->value();
                         if ($healthCheckStatus === 'healthy') {
-                            $this->newVersionIsHealthy = true;
+                            $this->markNewVersionHealthy(true);
                             $this->application->update(['status' => 'running']);
                             $this->application_deployment_queue->addLogEntry('New container is healthy.');
                             break;
                         } elseif ($healthCheckStatus === 'unhealthy') {
-                            $this->newVersionIsHealthy = false;
+                            $this->markNewVersionHealthy(false);
                             $this->application_deployment_queue->addLogEntry('New container is unhealthy.', type: 'error');
                             $this->query_logs();
                             break;
@@ -2800,6 +2798,24 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         }
     }
 
+    /**
+     * Laravel runs failed() on a fresh copy of the job, so the health of the new version is kept outside the job instance.
+     */
+    private function markNewVersionHealthy(bool $healthy): void
+    {
+        $key = "deployment-new-version-healthy:{$this->deployment_uuid}";
+        if ($healthy) {
+            Cache::put($key, true, now()->addDay());
+        } else {
+            Cache::forget($key);
+        }
+    }
+
+    private function newVersionIsHealthy(): bool
+    {
+        return Cache::has("deployment-new-version-healthy:{$this->deployment_uuid}");
+    }
+
     private function query_logs()
     {
         $escapedContainerName = escapeshellarg($this->container_name);
@@ -2830,7 +2846,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         if ($this->use_build_server) {
             $this->server = $this->build_server;
         }
-        $this->newVersionIsHealthy = true;
+        $this->markNewVersionHealthy(true);
         $this->generate_image_names();
         $this->application_deployment_queue->addLogEntry("Starting pull request (#{$this->pull_request_id}) deployment of {$this->customRepository}:{$this->application->git_branch}.");
         $this->prepare_builder_image();
@@ -4944,7 +4960,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
     {
         try {
             $this->application_deployment_queue->addLogEntry('Removing old containers.');
-            if ($this->newVersionIsHealthy || $force) {
+            if ($this->newVersionIsHealthy() || $force) {
                 if ($this->application->settings->is_consistent_container_name_enabled) {
                     $containers = getCurrentApplicationContainerStatus($this->server, $this->application, $this->pull_request_id);
                     $this->containerNamesToRemove($containers)->each(function (string $containerName) {
@@ -4973,7 +4989,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
             }
         } catch (Exception $e) {
             // If new version is healthy, this is just cleanup - don't fail the deployment
-            if ($this->newVersionIsHealthy || $force) {
+            if ($this->newVersionIsHealthy() || $force) {
                 $this->application_deployment_queue->addLogEntry(
                     "Warning: Could not remove old container: {$e->getMessage()}",
                     'stderr',
@@ -6144,7 +6160,7 @@ 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) {
+                } 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');

tests/Feature/ApplicationDeploymentFailedKeepsHealthyContainerTest.php

@@ -0,0 +1,94 @@
+<?php
+
+use App\Jobs\ApplicationDeploymentJob;
+use App\Models\Application;
+use App\Models\ApplicationDeploymentQueue;
+use App\Models\Environment;
+use App\Models\InstanceSettings;
+use App\Models\PrivateKey;
+use App\Models\Project;
+use App\Models\Server;
+use App\Models\StandaloneDocker;
+use App\Models\Team;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Process\PendingProcess;
+use Illuminate\Support\Facades\Notification;
+use Illuminate\Support\Facades\Process;
+use Illuminate\Support\Facades\Queue;
+use Illuminate\Support\Sleep;
+
+uses(RefreshDatabase::class);
+
+beforeEach(function () {
+    Server::flushIdentityMap();
+    InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0]));
+
+    $team = Team::factory()->create();
+    $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]);
+    $server = Server::factory()->create(['team_id' => $team->id, 'private_key_id' => $privateKey->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(),
+        'build_pack' => 'dockerfile',
+    ]);
+
+    $this->deployment = ApplicationDeploymentQueue::create([
+        'application_id' => $application->id,
+        'deployment_uuid' => fake()->uuid(),
+        'status' => 'in_progress',
+        'server_id' => $server->id,
+        'destination_id' => $destination->id,
+        'commit' => 'HEAD',
+        'pull_request_id' => 0,
+    ]);
+
+    Process::fake([
+        '*State.Health.Status*' => Process::result(output: "\"healthy\"\n"),
+        '*' => Process::result(output: ''),
+    ]);
+    Queue::fake();
+    Notification::fake();
+    Sleep::fake();
+});
+
+/**
+ * Laravel calls failed() on a copy of the job that it unserializes from the payload taken at dispatch,
+ * so state that handle() keeps only in memory is lost there.
+ */
+function failQueuedCopy(string $queuedCopy): void
+{
+    unserialize($queuedCopy)->failed(new RuntimeException('Late failure'));
+}
+
+function removesContainer(string $containerName): Closure
+{
+    return fn (PendingProcess $process) => str_contains((string) $process->command, 'docker rm -f')
+        && str_contains((string) $process->command, $containerName);
+}
+
+it('keeps the new container when a deployment fails after the new container became healthy', function () {
+    $job = new ApplicationDeploymentJob($this->deployment->id);
+    $queuedCopy = serialize(clone $job);
+    $containerName = (fn () => $this->container_name)->call($job);
+    (fn () => $this->health_check())->call($job);
+    failQueuedCopy($queuedCopy);
+
+    Process::assertNotRan(removesContainer($containerName));
+    expect($this->deployment->fresh()->logs)->toContain('Keeping the new version of your application running.');
+});
+
+it('removes the new container when a deployment fails before the new container became healthy', function () {
+    $job = new ApplicationDeploymentJob($this->deployment->id);
+    $queuedCopy = serialize(clone $job);
+    $containerName = (fn () => $this->container_name)->call($job);
+
+    failQueuedCopy($queuedCopy);
+
+    Process::assertRan(removesContainer($containerName));
+    expect($this->deployment->fresh()->logs)->toContain('Removing the new version of your application.');
+});

tests/Unit/ApplicationDeploymentErrorLoggingTest.php

@@ -6,6 +6,9 @@ use App\Models\Application;
 use App\Models\ApplicationDeploymentQueue;
 use Illuminate\Contracts\Queue\Job;
 use Illuminate\Queue\TimeoutExceededException;
+use Tests\TestCase;
+
+uses(TestCase::class);
 
 /**
  * Test to verify that deployment errors are properly logged with comprehensive details.
@@ -91,6 +94,7 @@ it('logs comprehensive error details when failed() is called', function () {
     $containerNameProperty = $reflection->getProperty('container_name');
     $containerNameProperty->setAccessible(true);
     $containerNameProperty->setValue($job, 'test-container');
+    $reflection->getProperty('deployment_uuid')->setValue($job, 'error-logging-test');
 
     // Mock the failDeployment method to prevent errors
     $job->shouldReceive('failDeployment')->andReturn();
@@ -197,6 +201,7 @@ it('handles exceptions with no message gracefully', function () {
     $containerNameProperty = $reflection->getProperty('container_name');
     $containerNameProperty->setAccessible(true);
     $containerNameProperty->setValue($job, 'test-container');
+    $reflection->getProperty('deployment_uuid')->setValue($job, 'error-logging-test');
 
     $job->shouldReceive('failDeployment')->andReturn();
     $job->shouldReceive('execute_remote_command')->andReturn();
@@ -288,6 +293,7 @@ it('logs error code 0 correctly', function () {
     $containerNameProperty = $reflection->getProperty('container_name');
     $containerNameProperty->setAccessible(true);
     $containerNameProperty->setValue($job, 'test-container');
+    $reflection->getProperty('deployment_uuid')->setValue($job, 'error-logging-test');
 
     $job->shouldReceive('failDeployment')->andReturn();
     $job->shouldReceive('execute_remote_command')->andReturn();
@@ -393,6 +399,7 @@ it('logs the configured timeout when the deployment times out', function () {
 
     $containerNameProperty = $reflection->getProperty('container_name');
     $containerNameProperty->setValue($job, 'test-container');
+    $reflection->getProperty('deployment_uuid')->setValue($job, 'error-logging-test');
 
     $job->shouldReceive('failDeployment')->andReturn();
     $job->shouldReceive('execute_remote_command')->andReturn();

tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php

@@ -4,11 +4,14 @@ use App\Jobs\ApplicationDeploymentJob;
 use App\Models\Application;
 use App\Models\ApplicationDeploymentQueue;
 use App\Models\ApplicationSetting;
+use Tests\TestCase;
+
+uses(TestCase::class);
 
 /**
  * @return array<int, array<int|string, mixed>>
  */
-function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $customInternalName, int $pullRequestId, bool $newVersionIsHealthy = false): array
+function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $customInternalName, int $pullRequestId): array
 {
     $settings = new ApplicationSetting;
     $settings->setRawAttributes([
@@ -36,8 +39,8 @@ function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $cust
     $reflection->getProperty('application_deployment_queue')->setValue($job, $queue);
     $reflection->getProperty('application')->setValue($job, $application);
     $reflection->getProperty('pull_request_id')->setValue($job, $pullRequestId);
+    $reflection->getProperty('deployment_uuid')->setValue($job, 'cleanup-test-deployment');
     $reflection->getProperty('container_name')->setValue($job, 'new-container-20261001T120000');
-    $reflection->getProperty('newVersionIsHealthy')->setValue($job, $newVersionIsHealthy);
 
     $job->failed(new RuntimeException('Health check failed'));
 
@@ -62,14 +65,3 @@ 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');
-});