From 766fe56448dd03b9ebe10212e51129cef16e6658 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 1 Oct 2026 07:16:13 +0200 Subject: [PATCH] fix(deploy): remove the new container when a rolling update fails Since rolling updates also run for applications with a custom internal name (when consistent container names are off), the new container gets a generated name. failed() still kept the new container whenever a custom internal name was set, so an exception after the new container started (for example an SSH or docker inspect error during the health check) left it running next to the old version with the same proxy labels. failed() now keeps the container only for consistent container names and pull request deployments, the same cases that skip rolling updates. --- app/Jobs/ApplicationDeploymentJob.php | 2 +- ...onDeploymentFailedContainerCleanupTest.php | 63 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 6a3101e1cc..10364885b6 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -5838,7 +5838,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); $code = $exception->getCode(); if ($code !== 69420) { // 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 || str($this->application->settings->custom_internal_name)->isNotEmpty() || $this->pull_request_id !== 0) { + if ($this->application->settings->is_consistent_container_name_enabled || $this->pull_request_id !== 0) { // do not remove already running container for PR deployments } else { $this->application_deployment_queue->addLogEntry('Deployment failed. Removing the new version of your application.', 'stderr'); diff --git a/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php b/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php new file mode 100644 index 0000000000..514d21fe14 --- /dev/null +++ b/tests/Unit/ApplicationDeploymentFailedContainerCleanupTest.php @@ -0,0 +1,63 @@ +> + */ +function runFailedDeploymentCleanup(bool $consistentContainerName, ?string $customInternalName, int $pullRequestId): array +{ + $settings = new ApplicationSetting; + $settings->setRawAttributes([ + 'is_consistent_container_name_enabled' => $consistentContainerName, + 'custom_internal_name' => $customInternalName, + ]); + + $application = new Application; + $application->setRawAttributes(['build_pack' => 'dockerfile']); + $application->setRelation('settings', $settings); + + $queue = Mockery::mock(ApplicationDeploymentQueue::class); + $queue->shouldReceive('addLogEntry')->andReturnNull(); + + $job = Mockery::mock(ApplicationDeploymentJob::class)->makePartial(); + $job->shouldAllowMockingProtectedMethods(); + $job->shouldReceive('failDeployment')->andReturnNull(); + + $remoteCommands = []; + $job->shouldReceive('execute_remote_command')->andReturnUsing(function (...$commands) use (&$remoteCommands) { + array_push($remoteCommands, ...$commands); + }); + + $reflection = new ReflectionClass(ApplicationDeploymentJob::class); + $reflection->getProperty('application_deployment_queue')->setValue($job, $queue); + $reflection->getProperty('application')->setValue($job, $application); + $reflection->getProperty('pull_request_id')->setValue($job, $pullRequestId); + $reflection->getProperty('container_name')->setValue($job, 'new-container-20261001T120000'); + + $job->failed(new RuntimeException('Health check failed')); + + return $remoteCommands; +} + +it('removes the new rolling-update container when only a custom internal name is set', function () { + $remoteCommands = runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: 'my-api', pullRequestId: 0); + + expect($remoteCommands)->toHaveCount(1) + ->and($remoteCommands[0][0])->toContain('new-container-20261001T120000'); +}); + +it('removes the new rolling-update container when no custom name is set', function () { + expect(runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: null, pullRequestId: 0))->toHaveCount(1); +}); + +it('keeps the running container when consistent container names are enabled', function () { + expect(runFailedDeploymentCleanup(consistentContainerName: true, customInternalName: 'my-api', pullRequestId: 0))->toBeEmpty(); +}); + +it('keeps the running container for pull request deployments', function () { + expect(runFailedDeploymentCleanup(consistentContainerName: false, customInternalName: 'my-api', pullRequestId: 12))->toBeEmpty(); +});