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.
This commit is contained in:
Andras Bacsai
2026-10-01 07:16:13 +02:00
parent 014e3c29b1
commit 766fe56448
2 changed files with 64 additions and 1 deletions
+1 -1
View File
@@ -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');
@@ -0,0 +1,63 @@
<?php
use App\Jobs\ApplicationDeploymentJob;
use App\Models\Application;
use App\Models\ApplicationDeploymentQueue;
use App\Models\ApplicationSetting;
/**
* @return array<int, array<int|string, mixed>>
*/
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();
});