mirror of
https://github.com/coollabsio/coolify.git
synced 2026-09-26 01:10:30 -04:00
fix(deployments): prevent deployment queue admission races (#11987)
This commit is contained in:
Andras Bacsai
@@ -9,6 +9,7 @@ use App\Models\ApplicationDeploymentQueue;
|
||||
use App\Models\EnvironmentVariable;
|
||||
use App\Models\Server;
|
||||
use App\Models\StandaloneDocker;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Spatie\Url\Url;
|
||||
|
||||
function queue_application_deployment(Application $application, string $deployment_uuid, ?int $pull_request_id = 0, ?string $commit = null, bool $force_rebuild = false, bool $is_webhook = false, bool $is_api = false, bool $restart_only = false, ?string $git_type = null, bool $no_questions_asked = false, ?Server $server = null, ?StandaloneDocker $destination = null, bool $only_this_server = false, bool $rollback = false, ?string $docker_registry_image_tag = null)
|
||||
@@ -30,32 +31,30 @@ function queue_application_deployment(Application $application, string $deployme
|
||||
$destination_id = $destination->id;
|
||||
}
|
||||
|
||||
// Check if the deployment queue is full for this server
|
||||
$serverForQueueCheck = $server ?? Server::find($server_id);
|
||||
$queue_limit = $serverForQueueCheck->settings->deployment_queue_limit ?? 25;
|
||||
$queued_count = ApplicationDeploymentQueue::where('server_id', $server_id)
|
||||
->where('status', ApplicationDeploymentStatus::QUEUED->value)
|
||||
->count();
|
||||
$admission = DB::transaction(function () use ($application, $application_id, $commit, $deployment_uuid, $deployment_url, $destination_id, $docker_registry_image_tag, $force_rebuild, $git_type, $is_api, $is_webhook, $no_questions_asked, $only_this_server, $pull_request_id, $restart_only, $rollback, $server_id, $server_name) {
|
||||
// Lock stable rows because an empty deployment queue has no row to lock.
|
||||
Application::query()->whereKey($application_id)->lockForUpdate()->firstOrFail();
|
||||
$serverForQueueCheck = Server::query()->whereKey($server_id)->lockForUpdate()->firstOrFail();
|
||||
$queue_limit = $serverForQueueCheck->settings->deployment_queue_limit ?? 25;
|
||||
$queued_count = ApplicationDeploymentQueue::where('server_id', $server_id)
|
||||
->where('status', ApplicationDeploymentStatus::QUEUED->value)
|
||||
->count();
|
||||
|
||||
if ($queued_count >= $queue_limit) {
|
||||
return [
|
||||
'status' => 'queue_full',
|
||||
'message' => 'Deployment queue is full. Please wait for existing deployments to complete.',
|
||||
];
|
||||
}
|
||||
if ($queued_count >= $queue_limit) {
|
||||
return [
|
||||
'status' => 'queue_full',
|
||||
'message' => 'Deployment queue is full. Please wait for existing deployments to complete.',
|
||||
];
|
||||
}
|
||||
|
||||
// Check if there's already a deployment in progress or queued for this application and commit
|
||||
$existing_deployment = ApplicationDeploymentQueue::where('application_id', $application_id)
|
||||
->where('commit', $commit)
|
||||
->where('pull_request_id', $pull_request_id)
|
||||
->where('docker_registry_image_tag', $docker_registry_image_tag)
|
||||
->whereIn('status', [ApplicationDeploymentStatus::IN_PROGRESS->value, ApplicationDeploymentStatus::QUEUED->value])
|
||||
->first();
|
||||
$existing_deployment = ApplicationDeploymentQueue::where('application_id', $application_id)
|
||||
->where('commit', $commit)
|
||||
->where('pull_request_id', $pull_request_id)
|
||||
->where('docker_registry_image_tag', $docker_registry_image_tag)
|
||||
->whereIn('status', [ApplicationDeploymentStatus::IN_PROGRESS->value, ApplicationDeploymentStatus::QUEUED->value])
|
||||
->first();
|
||||
|
||||
if ($existing_deployment) {
|
||||
// If force_rebuild is true or rollback is true or no_questions_asked is true, we'll still create a new deployment
|
||||
if (! $force_rebuild && ! $rollback && ! $no_questions_asked) {
|
||||
// Return the existing deployment's details
|
||||
if ($existing_deployment && ! $force_rebuild && ! $rollback && ! $no_questions_asked) {
|
||||
return [
|
||||
'status' => 'skipped',
|
||||
'message' => 'Deployment already queued for this commit.',
|
||||
@@ -63,27 +62,33 @@ function queue_application_deployment(Application $application, string $deployme
|
||||
'existing_deployment' => $existing_deployment,
|
||||
];
|
||||
}
|
||||
|
||||
return ApplicationDeploymentQueue::create([
|
||||
'application_id' => $application_id,
|
||||
'application_name' => $application->name,
|
||||
'server_id' => $server_id,
|
||||
'server_name' => $server_name,
|
||||
'destination_id' => $destination_id,
|
||||
'deployment_uuid' => $deployment_uuid,
|
||||
'deployment_url' => $deployment_url,
|
||||
'pull_request_id' => $pull_request_id,
|
||||
'docker_registry_image_tag' => $docker_registry_image_tag,
|
||||
'force_rebuild' => $force_rebuild,
|
||||
'is_webhook' => $is_webhook,
|
||||
'is_api' => $is_api,
|
||||
'restart_only' => $restart_only,
|
||||
'commit' => $commit,
|
||||
'rollback' => $rollback,
|
||||
'git_type' => $git_type,
|
||||
'only_this_server' => $only_this_server,
|
||||
]);
|
||||
});
|
||||
|
||||
if (is_array($admission)) {
|
||||
return $admission;
|
||||
}
|
||||
|
||||
$deployment = ApplicationDeploymentQueue::create([
|
||||
'application_id' => $application_id,
|
||||
'application_name' => $application->name,
|
||||
'server_id' => $server_id,
|
||||
'server_name' => $server_name,
|
||||
'destination_id' => $destination_id,
|
||||
'deployment_uuid' => $deployment_uuid,
|
||||
'deployment_url' => $deployment_url,
|
||||
'pull_request_id' => $pull_request_id,
|
||||
'docker_registry_image_tag' => $docker_registry_image_tag,
|
||||
'force_rebuild' => $force_rebuild,
|
||||
'is_webhook' => $is_webhook,
|
||||
'is_api' => $is_api,
|
||||
'restart_only' => $restart_only,
|
||||
'commit' => $commit,
|
||||
'rollback' => $rollback,
|
||||
'git_type' => $git_type,
|
||||
'only_this_server' => $only_this_server,
|
||||
]);
|
||||
$deployment = $admission;
|
||||
|
||||
if (auth()->check() && ! $is_webhook && ! $is_api && ! $rollback) {
|
||||
auditLog($restart_only ? 'ui.application.restarted' : 'ui.application.deployed', [
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
<?php
|
||||
|
||||
use App\Enums\ApplicationDeploymentStatus;
|
||||
use App\Jobs\ApplicationDeploymentJob;
|
||||
use App\Models\Application;
|
||||
use App\Models\ApplicationDeploymentQueue;
|
||||
@@ -12,6 +13,7 @@ use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Bus;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
@@ -45,6 +47,88 @@ function makeApplication(int $environmentId, int $destinationId, ?string $gitCom
|
||||
}
|
||||
|
||||
describe('queue_application_deployment commit resolution', function () {
|
||||
test('inserts only while an admission transaction is open', function () {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
$initialLevel = DB::transactionLevel();
|
||||
$creationLevel = null;
|
||||
|
||||
ApplicationDeploymentQueue::created(function () use (&$creationLevel): void {
|
||||
$creationLevel = DB::transactionLevel();
|
||||
});
|
||||
|
||||
queue_application_deployment($application, 'atomic-admission');
|
||||
|
||||
expect($creationLevel)->toBeGreaterThan($initialLevel);
|
||||
});
|
||||
|
||||
test('skips a matching commit without creating or dispatching a second deployment', function () {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
$first = queue_application_deployment($application, 'first-admission');
|
||||
$second = queue_application_deployment($application, 'second-admission');
|
||||
|
||||
expect($first['status'])->toBe('queued')
|
||||
->and($second['status'])->toBe('skipped')
|
||||
->and($second['deployment_uuid'])->toBe('first-admission')
|
||||
->and($second['existing_deployment']->deployment_uuid)->toBe('first-admission')
|
||||
->and(ApplicationDeploymentQueue::query()->count())->toBe(1);
|
||||
Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1);
|
||||
});
|
||||
|
||||
test('allows explicit duplicate bypasses while queue capacity remains available', function (string $flag) {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
queue_application_deployment($application, 'first-admission');
|
||||
$result = queue_application_deployment($application, 'bypass-admission', ...[$flag => true]);
|
||||
|
||||
expect($result['status'])->toBe('queued')
|
||||
->and(ApplicationDeploymentQueue::query()->count())->toBe(2);
|
||||
})->with(['force_rebuild', 'rollback', 'no_questions_asked']);
|
||||
|
||||
test('checks duplicate commits across servers but uses the target server queue', function () {
|
||||
$otherServer = Server::factory()->create(['team_id' => $this->team->id]);
|
||||
$otherDestination = StandaloneDocker::factory()->create([
|
||||
'server_id' => $otherServer->id,
|
||||
'network' => 'test-network-'.fake()->unique()->word(),
|
||||
]);
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
queue_application_deployment($application, 'first-server-admission');
|
||||
$skipped = queue_application_deployment($application, 'second-server-admission', server: $otherServer, destination: $otherDestination);
|
||||
$forced = queue_application_deployment($application, 'forced-second-server-admission', force_rebuild: true, server: $otherServer, destination: $otherDestination);
|
||||
|
||||
expect($skipped['status'])->toBe('skipped')
|
||||
->and($skipped['deployment_uuid'])->toBe('first-server-admission')
|
||||
->and($forced['status'])->toBe('queued')
|
||||
->and(ApplicationDeploymentQueue::query()->where('server_id', $otherServer->id)->sole()->deployment_uuid)->toBe('forced-second-server-admission');
|
||||
});
|
||||
|
||||
test('rejects a full server queue before a different commit is inserted', function () {
|
||||
$this->server->settings->update(['concurrent_builds' => 0, 'deployment_queue_limit' => 1]);
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
queue_application_deployment($application, 'first-admission');
|
||||
$result = queue_application_deployment($application, 'full-admission', commit: 'another-commit');
|
||||
|
||||
expect($result)->toBe([
|
||||
'status' => 'queue_full',
|
||||
'message' => 'Deployment queue is full. Please wait for existing deployments to complete.',
|
||||
])->and(ApplicationDeploymentQueue::query()->where('status', ApplicationDeploymentStatus::QUEUED->value)->count())->toBe(1);
|
||||
Bus::assertNotDispatched(ApplicationDeploymentJob::class);
|
||||
});
|
||||
|
||||
test('does not let a forced deployment exceed the queued limit', function () {
|
||||
$this->server->settings->update(['concurrent_builds' => 0, 'deployment_queue_limit' => 1]);
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
queue_application_deployment($application, 'first-admission');
|
||||
$result = queue_application_deployment($application, 'forced-full-admission', force_rebuild: true);
|
||||
|
||||
expect($result['status'])->toBe('queue_full')
|
||||
->and(ApplicationDeploymentQueue::query()->count())->toBe(1);
|
||||
Bus::assertNotDispatched(ApplicationDeploymentJob::class);
|
||||
});
|
||||
|
||||
test('rejects a commit with disallowed characters before creating a deployment', function () {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
|
||||
@@ -55,7 +139,7 @@ describe('queue_application_deployment commit resolution', function () {
|
||||
is_webhook: true,
|
||||
))->toThrow(Exception::class, 'Invalid deployment commit');
|
||||
|
||||
$this->assertDatabaseMissing('application_deployment_queue', [
|
||||
$this->assertDatabaseMissing((new ApplicationDeploymentQueue)->getTable(), [
|
||||
'deployment_uuid' => 'invalid-queued-commit',
|
||||
]);
|
||||
Bus::assertNotDispatched(ApplicationDeploymentJob::class);
|
||||
@@ -73,7 +157,7 @@ describe('queue_application_deployment commit resolution', function () {
|
||||
deployment_uuid: 'invalid-fallback-commit',
|
||||
))->toThrow(Exception::class, 'Invalid deployment commit');
|
||||
|
||||
$this->assertDatabaseMissing('application_deployment_queue', [
|
||||
$this->assertDatabaseMissing((new ApplicationDeploymentQueue)->getTable(), [
|
||||
'deployment_uuid' => 'invalid-fallback-commit',
|
||||
]);
|
||||
Bus::assertNotDispatched(ApplicationDeploymentJob::class);
|
||||
|
||||
Reference in New Issue
Block a user