From 7183e5277947c24cda959c7b3e6d9c4ea771cb1e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:23:35 +0200 Subject: [PATCH] fix(deploy): never force start the same deployment twice Force start now uses the locked start path. It still skips the concurrency limits, but a deployment that is no longer queued is not started again, and the deployment page says so. --- .../Project/Application/DeploymentNavbar.php | 7 +- bootstrap/helpers/applications.php | 22 ++--- .../DeploymentNavbarForceStartTest.php | 81 +++++++++++++++++++ .../QueueApplicationDeploymentCommitTest.php | 56 +++++++++++++ 4 files changed, 154 insertions(+), 12 deletions(-) create mode 100644 tests/Feature/DeploymentNavbarForceStartTest.php diff --git a/app/Livewire/Project/Application/DeploymentNavbar.php b/app/Livewire/Project/Application/DeploymentNavbar.php index 3abc2da73c..fd97ac35eb 100644 --- a/app/Livewire/Project/Application/DeploymentNavbar.php +++ b/app/Livewire/Project/Application/DeploymentNavbar.php @@ -55,7 +55,12 @@ class DeploymentNavbar extends Component { try { $this->authorize('deploy', $this->application); - force_start_deployment($this->application_deployment_queue); + $started = force_start_deployment($this->application_deployment_queue); + $this->application_deployment_queue->refresh(); + + if (! $started) { + $this->dispatch('info', 'This deployment is no longer queued, so it was not started again.'); + } } catch (\Throwable $e) { return handleError($e, $this); } diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index 8fe4c0d393..e334bcd682 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -123,15 +123,14 @@ function queue_application_deployment(Application $application, string $deployme 'deployment_uuid' => $deployment_uuid, ]; } -function force_start_deployment(ApplicationDeploymentQueue $deployment) +/** + * Start a queued deployment right away, ignoring the per-application and per-server + * concurrency limits. It returns false without dispatching when the deployment is no + * longer queued, so a repeated force start never runs the same deployment twice. + */ +function force_start_deployment(ApplicationDeploymentQueue $deployment): bool { - $deployment->update([ - 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, - ]); - - ApplicationDeploymentJob::dispatch( - application_deployment_queue_id: $deployment->id, - ); + return start_queued_deployment($deployment, force: true); } function queue_next_deployment(Application $application) { @@ -150,10 +149,11 @@ function queue_next_deployment(Application $application) * Start a queued deployment if it can run now. The check and the status change run under * the same application and server row locks as queue admission, so two workers cannot both * start the same deployment or exceed the per-application and per-server limits. + * With $force, the limits are skipped but the deployment still has to be queued. */ -function start_queued_deployment(ApplicationDeploymentQueue $deployment): bool +function start_queued_deployment(ApplicationDeploymentQueue $deployment, bool $force = false): bool { - $started = DB::transaction(function () use ($deployment): bool { + $started = DB::transaction(function () use ($deployment, $force): bool { Application::query()->whereKey($deployment->application_id)->lockForUpdate()->first(); Server::query()->whereKey($deployment->server_id)->lockForUpdate()->first(); $current = ApplicationDeploymentQueue::query()->whereKey($deployment->id)->lockForUpdate()->first(); @@ -162,7 +162,7 @@ function start_queued_deployment(ApplicationDeploymentQueue $deployment): bool return false; } - if (! next_queuable($current->server_id, $current->application_id, $current->commit, $current->pull_request_id)) { + if (! $force && ! next_queuable($current->server_id, $current->application_id, $current->commit, $current->pull_request_id)) { return false; } diff --git a/tests/Feature/DeploymentNavbarForceStartTest.php b/tests/Feature/DeploymentNavbarForceStartTest.php new file mode 100644 index 0000000000..3d4ebe4cd8 --- /dev/null +++ b/tests/Feature/DeploymentNavbarForceStartTest.php @@ -0,0 +1,81 @@ +create(['id' => 0]); + }); + + $this->user = User::factory()->create(); + $this->team = Team::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'admin']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->server->settings->update(['concurrent_builds' => 0]); + $destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + $this->deployment = ApplicationDeploymentQueue::create([ + 'application_id' => $this->application->id, + 'deployment_uuid' => 'navbar-force-start', + 'server_id' => $this->server->id, + 'destination_id' => $destination->id, + 'status' => ApplicationDeploymentStatus::QUEUED->value, + ]); +}); + +test('force start starts a queued deployment once even when clicked twice', function () { + $component = Livewire::test(DeploymentNavbar::class, ['application_deployment_queue' => $this->deployment]); + + $component->call('force_start')->assertNotDispatched('info'); + $component->call('force_start')->assertDispatched('info', 'This deployment is no longer queued, so it was not started again.'); + + expect($this->deployment->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1); +}); + +test('force start from two open pages starts the deployment once', function () { + $firstAdmin = Livewire::test(DeploymentNavbar::class, ['application_deployment_queue' => $this->deployment]); + $secondAdmin = Livewire::test(DeploymentNavbar::class, ['application_deployment_queue' => $this->deployment]); + + $firstAdmin->call('force_start'); + $secondAdmin->call('force_start')->assertDispatched('info'); + + Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1); +}); + +test('members cannot force start a deployment', function () { + $this->team->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + + Livewire::test(DeploymentNavbar::class, ['application_deployment_queue' => $this->deployment]) + ->call('force_start'); + + expect($this->deployment->fresh()->status)->toBe(ApplicationDeploymentStatus::QUEUED->value); + Bus::assertNotDispatched(ApplicationDeploymentJob::class); +}); diff --git a/tests/Feature/QueueApplicationDeploymentCommitTest.php b/tests/Feature/QueueApplicationDeploymentCommitTest.php index 422047979b..1241d8d506 100644 --- a/tests/Feature/QueueApplicationDeploymentCommitTest.php +++ b/tests/Feature/QueueApplicationDeploymentCommitTest.php @@ -117,6 +117,62 @@ describe('queue_application_deployment commit resolution', function () { Bus::assertNotDispatched(ApplicationDeploymentJob::class); }); + test('force starts a queued deployment only once when two requests hold the same queued row', function () { + $application = makeApplication($this->environment->id, $this->destination->id, 'HEAD'); + $this->server->settings->update(['concurrent_builds' => 0]); + queue_application_deployment($application, 'double-force-start'); + $firstClick = ApplicationDeploymentQueue::query()->sole(); + $secondClick = ApplicationDeploymentQueue::query()->sole(); + + $firstResult = force_start_deployment($firstClick); + $secondResult = force_start_deployment($secondClick); + + Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1); + expect($firstResult)->toBeTrue() + ->and($secondResult)->toBeFalse() + ->and(ApplicationDeploymentQueue::query()->sole()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + }); + + test('force start bypasses the concurrency limits inside a transaction', function () { + $application = makeApplication($this->environment->id, $this->destination->id, 'HEAD'); + queue_application_deployment($application, 'running-deployment'); + queue_application_deployment($application, 'forced-deployment', commit: 'another-commit'); + $forced = ApplicationDeploymentQueue::query()->where('deployment_uuid', 'forced-deployment')->sole(); + $initialLevel = DB::transactionLevel(); + $startLevel = null; + + ApplicationDeploymentQueue::updated(function (ApplicationDeploymentQueue $deployment) use (&$startLevel): void { + if ($deployment->status === ApplicationDeploymentStatus::IN_PROGRESS->value) { + $startLevel = DB::transactionLevel(); + } + }); + + expect($forced->status)->toBe(ApplicationDeploymentStatus::QUEUED->value) + ->and(force_start_deployment($forced))->toBeTrue() + ->and($startLevel)->toBeGreaterThan($initialLevel) + ->and($forced->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 2); + }); + + test('force start does not restart a deployment that is no longer queued', function (ApplicationDeploymentStatus $status) { + $application = makeApplication($this->environment->id, $this->destination->id, 'HEAD'); + $this->server->settings->update(['concurrent_builds' => 0]); + queue_application_deployment($application, 'not-queued-deployment'); + $stale = ApplicationDeploymentQueue::query()->sole(); + ApplicationDeploymentQueue::query()->whereKey($stale->id)->update(['status' => $status->value]); + + $result = force_start_deployment($stale); + + Bus::assertNotDispatched(ApplicationDeploymentJob::class); + expect($stale->fresh()->status)->toBe($status->value) + ->and($result)->toBeFalse(); + })->with([ + ApplicationDeploymentStatus::IN_PROGRESS, + ApplicationDeploymentStatus::FINISHED, + ApplicationDeploymentStatus::FAILED, + ApplicationDeploymentStatus::CANCELLED_BY_USER, + ]); + test('skips a matching commit without creating or dispatching a second deployment', function () { $application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');