mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
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.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
<?php
|
||||
|
||||
use App\Enums\ApplicationDeploymentStatus;
|
||||
use App\Jobs\ApplicationDeploymentJob;
|
||||
use App\Livewire\Project\Application\DeploymentNavbar;
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationDeploymentQueue;
|
||||
use App\Models\Environment;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\Project;
|
||||
use App\Models\Server;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Bus;
|
||||
use Livewire\Livewire;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
Bus::fake([ApplicationDeploymentJob::class]);
|
||||
|
||||
InstanceSettings::unguarded(function () {
|
||||
InstanceSettings::query()->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);
|
||||
});
|
||||
@@ -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');
|
||||
|
||||
|
||||
Reference in New Issue
Block a user