diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index b8c0f1160d..404d3a9ad3 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -508,7 +508,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } $this->build_server = $buildServers->random(); - $this->application_deployment_queue->build_server_id = $this->build_server->id; + // Saved now: the cancel action looks for the helper container on this server. + $this->application_deployment_queue->update(['build_server_id' => $this->build_server->id]); $this->application_deployment_queue->addLogEntry("Found a suitable build server ({$this->build_server->name})."); $this->use_build_server = true; } @@ -5244,8 +5245,9 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); $timeout = $this->application->settings->deploymentStopGracePeriodSeconds(); if ($skipRemove) { + // The helper container (started with --rm) must also stop when the deployment was cancelled. $this->execute_remote_command( - [dockerStopCommand($timeout, escapeshellarg($containerName), $this->server), 'hidden' => true, 'ignore_errors' => true] + [dockerStopCommand($timeout, escapeshellarg($containerName), $this->server), 'hidden' => true, 'ignore_errors' => true, 'skip_cancellation_check' => true] ); } else { $this->execute_remote_command( diff --git a/app/Traits/ExecuteRemoteCommand.php b/app/Traits/ExecuteRemoteCommand.php index 21912fb82f..fe9d0a4c7b 100644 --- a/app/Traits/ExecuteRemoteCommand.php +++ b/app/Traits/ExecuteRemoteCommand.php @@ -87,6 +87,7 @@ trait ExecuteRemoteCommand $command_hidden = data_get($single_command, 'command_hidden', false); $skip_command_log = data_get($single_command, 'skip_command_log', false); $input = data_get($single_command, 'input'); + $skip_cancellation_check = data_get($single_command, 'skip_cancellation_check', false); $this->save = data_get($single_command, 'save'); if ($this->server->isNonRoot()) { if (str($command)->startsWith('docker exec')) { @@ -96,8 +97,8 @@ trait ExecuteRemoteCommand } } - // Check for cancellation before executing commands - if (isset($this->application_deployment_queue)) { + // Check for cancellation before executing commands. Cleanup commands must also run after a cancel. + if (! $skip_cancellation_check && isset($this->application_deployment_queue)) { $this->application_deployment_queue->refresh(); if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { throw new \RuntimeException('Deployment cancelled by user', 69420); @@ -126,9 +127,11 @@ trait ExecuteRemoteCommand $this->addRetryLogEntry($attempt, $maxRetries, $delay, $errorMessage); // Check for cancellation during retry wait - $this->application_deployment_queue->refresh(); - if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { - throw new \RuntimeException('Deployment cancelled by user during retry', 69420); + if (! $skip_cancellation_check) { + $this->application_deployment_queue->refresh(); + if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { + throw new \RuntimeException('Deployment cancelled by user during retry', 69420); + } } } diff --git a/tests/Feature/BuildServerFallbackPolicyTest.php b/tests/Feature/BuildServerFallbackPolicyTest.php index aee3c54384..f7a366e1c4 100644 --- a/tests/Feature/BuildServerFallbackPolicyTest.php +++ b/tests/Feature/BuildServerFallbackPolicyTest.php @@ -148,7 +148,7 @@ test('strict teams use an available dedicated build server', function () { ]); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer); - $deploymentQueue->shouldReceive('setAttribute')->with('build_server_id', $buildServer->id)->once()->andReturnSelf(); + $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $buildServer->id])->once()->andReturnTrue(); $deploymentQueue->shouldReceive('addLogEntry') ->once() ->with("Found a suitable build server ({$buildServer->name})."); @@ -179,7 +179,7 @@ test('build server selection only picks dedicated build servers', function () { $buildServer = makeRoleServer($team, 'build'); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer); - $deploymentQueue->shouldReceive('setAttribute')->with('build_server_id', $buildServer->id)->once()->andReturnSelf(); + $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $buildServer->id])->once()->andReturnTrue(); $deploymentQueue->shouldReceive('addLogEntry')->once()->with("Found a suitable build server ({$buildServer->name})."); invokeBuildServerSelection($job); @@ -194,7 +194,7 @@ test('fallback builds on the deployment server instead of another combined serve makeRoleServer($team, 'both'); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer); - $deploymentQueue->shouldNotReceive('setAttribute'); + $deploymentQueue->shouldNotReceive('update'); $deploymentQueue->shouldReceive('addLogEntry')->once()->with('No suitable build server found. Using the deployment server.'); invokeBuildServerSelection($job); @@ -210,7 +210,7 @@ test('deployments-only servers build on a dedicated build server without the app $buildServer = makeRoleServer($team, 'build'); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, ['docker_registry_image_name' => 'ghcr.io/coollabsio/app'], buildServerEnabled: false); - $deploymentQueue->shouldReceive('setAttribute')->with('build_server_id', $buildServer->id)->once()->andReturnSelf(); + $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $buildServer->id])->once()->andReturnTrue(); $deploymentQueue->shouldReceive('addLogEntry')->once()->with("Found a suitable build server ({$buildServer->name})."); invokeBuildServerSelection($job); @@ -252,7 +252,7 @@ test('deployments-only servers restart without a registry image', function () { $buildServer = makeRoleServer($team, 'build'); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, buildServerEnabled: false, restartOnly: true); - $deploymentQueue->shouldReceive('setAttribute')->with('build_server_id', $buildServer->id)->once()->andReturnSelf(); + $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $buildServer->id])->once()->andReturnTrue(); $deploymentQueue->shouldReceive('addLogEntry')->once()->with("Found a suitable build server ({$buildServer->name})."); invokeBuildServerSelection($job); @@ -312,7 +312,7 @@ test('additional deployments-only servers pull the registry image without a buil [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, ['docker_registry_image_name' => 'ghcr.io/coollabsio/app'], $buildServerEnabled); markAsAdditionalServer($job); - $deploymentQueue->shouldNotReceive('setAttribute'); + $deploymentQueue->shouldNotReceive('update'); $deploymentQueue->shouldReceive('addLogEntry')->once()->with('Additional server: pulls the image from the registry, no build server needed.'); invokeBuildServerSelection($job); @@ -331,7 +331,7 @@ test('additional servers do not use a build server when the application enables [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, ['docker_registry_image_name' => 'ghcr.io/coollabsio/app']); markAsAdditionalServer($job); - $deploymentQueue->shouldNotReceive('setAttribute'); + $deploymentQueue->shouldNotReceive('update'); $deploymentQueue->shouldReceive('addLogEntry')->once()->with('Additional server: pulls the image from the registry, no build server needed.'); invokeBuildServerSelection($job); @@ -436,7 +436,7 @@ test('strict teams restart on the deployment server when no build server is avai $deploymentServer = makeRoleServer($team, $role); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, buildServerEnabled: $buildServerEnabled, restartOnly: true); - $deploymentQueue->shouldNotReceive('setAttribute'); + $deploymentQueue->shouldNotReceive('update'); $deploymentQueue->shouldReceive('addLogEntry')->once()->with('No suitable build server found. Using the deployment server.'); invokeBuildServerSelection($job); @@ -454,7 +454,7 @@ test('strict teams restart on an available build server', function () { $buildServer = makeRoleServer($team, 'build'); [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, buildServerEnabled: false, restartOnly: true); - $deploymentQueue->shouldReceive('setAttribute')->with('build_server_id', $buildServer->id)->once()->andReturnSelf(); + $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $buildServer->id])->once()->andReturnTrue(); $deploymentQueue->shouldReceive('addLogEntry')->once()->with("Found a suitable build server ({$buildServer->name})."); invokeBuildServerSelection($job); @@ -481,3 +481,23 @@ test('a restart that has to rebuild applies the build server rules again', funct 'deployments-only server' => ['deployment', false, 'is set to deployments only, and no usable build server was found.'], 'application uses a build server' => ['both', true, 'No available dedicated build server was found.'], ]); + +test('the selected build server is saved on the deployment so a cancel finds the helper container', function () { + $team = Team::factory()->create(); + $deploymentServer = makeRoleServer($team, 'both'); + $buildServer = makeRoleServer($team, 'build'); + [$job] = makeBuildServerSelectionJob($team, $deploymentServer); + $deployment = ApplicationDeploymentQueue::query()->create([ + 'application_id' => '1', + 'deployment_uuid' => 'build-server-cancel-test', + 'server_id' => $deploymentServer->id, + ]); + (new ReflectionProperty(ApplicationDeploymentJob::class, 'application_deployment_queue'))->setValue($job, $deployment); + + invokeBuildServerSelection($job); + // execute_remote_command() refreshes the deployment before each command. + $deployment->refresh(); + + expect($deployment->build_server_id)->toBe($buildServer->id) + ->and(ApplicationDeploymentQueue::query()->find($deployment->id)->build_server_id)->toBe($buildServer->id); +}); diff --git a/tests/Feature/DeploymentCancelHelperShutdownTest.php b/tests/Feature/DeploymentCancelHelperShutdownTest.php new file mode 100644 index 0000000000..06bb2aae70 --- /dev/null +++ b/tests/Feature/DeploymentCancelHelperShutdownTest.php @@ -0,0 +1,70 @@ + 0]); + Process::fake(); +}); + +function makeCancelledDeploymentJob(): array +{ + $team = Team::factory()->create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $application = Application::factory()->create(['environment_id' => $environment->id]); + $deployment = ApplicationDeploymentQueue::query()->create([ + 'application_id' => $application->id, + 'deployment_uuid' => 'cancelled-helper', + 'server_id' => $server->id, + 'status' => ApplicationDeploymentStatus::CANCELLED_BY_USER->value, + ]); + + $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor(); + foreach ([ + 'application' => $application, + 'application_deployment_queue' => $deployment, + 'server' => $server, + 'saved_outputs' => collect(), + ] as $property => $value) { + (new ReflectionProperty(ApplicationDeploymentJob::class, $property))->setValue($job, $value); + } + + return [$job, $deployment]; +} + +test('a cancelled deployment still stops its helper container', function () { + [$job, $deployment] = makeCancelledDeploymentJob(); + + (new ReflectionMethod(ApplicationDeploymentJob::class, 'graceful_shutdown_container'))->invoke($job, 'cancelled-helper', true); + + Process::assertRan(fn (PendingProcess $process) => str_contains(remoteProcessScript($process), 'docker stop') && str_contains(remoteProcessScript($process), 'cancelled-helper')); + expect($deployment->refresh()->logs ?? '')->not->toContain('Error stopping container'); +}); + +test('a cancelled deployment does not run other remote commands', function () { + [$job] = makeCancelledDeploymentJob(); + + expect(fn () => $job->execute_remote_command(['echo build', 'hidden' => true])) + ->toThrow(RuntimeException::class, 'Deployment cancelled by user'); + + Process::assertNothingRan(); +});