mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(deploy): start queued deployments atomically and report cancelled multi-server runs
- Decide whether to start a queued deployment inside the application and server lock, so two requests cannot start the same app twice. - Send the multi-server summary when the last child is cancelled. - Show the registry message when an inline Dockerfile image is missing on a deployments-only additional server.
This commit is contained in:
@@ -310,6 +310,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
|
||||
$this->application_deployment_queue->refresh();
|
||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) {
|
||||
$this->application_deployment_queue->addLogEntry('Deployment was cancelled before starting.');
|
||||
$this->handleCancelledDeployment();
|
||||
|
||||
return;
|
||||
}
|
||||
@@ -659,7 +660,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
|
||||
/**
|
||||
* An inline Dockerfile image has no commit tag. The main server builds it and pushes it with the
|
||||
* `latest` tag just before it queues the additional servers, so an additional server pulls that
|
||||
* image instead of building its own. A failed pull fails the deployment.
|
||||
* image instead of building its own. A missing image fails the deployment with a clear message.
|
||||
*/
|
||||
private function pull_image_for_additional_server(): bool
|
||||
{
|
||||
@@ -667,11 +668,28 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
|
||||
return false;
|
||||
}
|
||||
|
||||
$image = escapeshellarg($this->production_image_name);
|
||||
$this->application_deployment_queue->addLogEntry("Pulling image ({$this->production_image_name}) that the main server pushed. Build step skipped.");
|
||||
$this->execute_remote_command([
|
||||
'docker pull '.escapeshellarg($this->production_image_name),
|
||||
'hidden' => true,
|
||||
]);
|
||||
$this->execute_remote_command(
|
||||
[
|
||||
"docker pull {$image}",
|
||||
'hidden' => true,
|
||||
'ignore_errors' => true,
|
||||
],
|
||||
[
|
||||
"docker images -q {$image} 2>/dev/null",
|
||||
'hidden' => true,
|
||||
'save' => 'local_image_found',
|
||||
],
|
||||
);
|
||||
|
||||
if (str($this->saved_outputs->get('local_image_found'))->isEmpty()) {
|
||||
$message = "Image ({$this->production_image_name}) not found in the registry; the main server must push it first.";
|
||||
if (! $this->server->canBuildApplications()) {
|
||||
$message .= " The server ({$this->server->name}) is set to deployments only and cannot build it.";
|
||||
}
|
||||
throw new DeploymentException($message);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
@@ -5614,12 +5632,24 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf");
|
||||
|
||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) {
|
||||
$this->application_deployment_queue->addLogEntry('Deployment cancelled by user, stopping execution.');
|
||||
$this->handleCancelledDeployment();
|
||||
throw new DeploymentException('Deployment cancelled by user', 69420);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* A cancelled deployment on an additional server can be the last one of a multi-server
|
||||
* deployment, so it must also trigger the combined notification.
|
||||
*/
|
||||
private function handleCancelledDeployment(): void
|
||||
{
|
||||
if (filled($this->application_deployment_queue->parent_deployment_uuid)) {
|
||||
$this->sendMultiServerDeploymentNotification($this->application_deployment_queue->parent_deployment_uuid);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Update the deployment status in the database.
|
||||
*/
|
||||
|
||||
@@ -63,7 +63,7 @@ function queue_application_deployment(Application $application, string $deployme
|
||||
];
|
||||
}
|
||||
|
||||
return ApplicationDeploymentQueue::create([
|
||||
$deployment = ApplicationDeploymentQueue::create([
|
||||
'application_id' => $application_id,
|
||||
'application_name' => $application->name,
|
||||
'server_id' => $server_id,
|
||||
@@ -83,13 +83,24 @@ function queue_application_deployment(Application $application, string $deployme
|
||||
'only_this_server' => $only_this_server,
|
||||
'parent_deployment_uuid' => $parent_deployment_uuid,
|
||||
]);
|
||||
|
||||
// Decide whether to start while the application and server rows are still locked,
|
||||
// so concurrent requests cannot both see an idle queue and start two deployments.
|
||||
$started = $no_questions_asked || next_queuable($server_id, $application_id, $commit, $pull_request_id);
|
||||
if ($started) {
|
||||
$deployment->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
}
|
||||
|
||||
return ['deployment' => $deployment, 'started' => $started];
|
||||
});
|
||||
|
||||
if (is_array($admission)) {
|
||||
if (! isset($admission['deployment'])) {
|
||||
return $admission;
|
||||
}
|
||||
|
||||
$deployment = $admission;
|
||||
$deployment = $admission['deployment'];
|
||||
|
||||
if (auth()->check() && ! $is_webhook && ! $is_api && ! $rollback) {
|
||||
auditLog($restart_only ? 'ui.application.restarted' : 'ui.application.deployed', [
|
||||
@@ -100,17 +111,7 @@ function queue_application_deployment(Application $application, string $deployme
|
||||
]);
|
||||
}
|
||||
|
||||
if ($no_questions_asked) {
|
||||
$deployment->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
ApplicationDeploymentJob::dispatch(
|
||||
application_deployment_queue_id: $deployment->id,
|
||||
);
|
||||
} elseif (next_queuable($server_id, $application_id, $commit, $pull_request_id)) {
|
||||
$deployment->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
if ($admission['started']) {
|
||||
ApplicationDeploymentJob::dispatch(
|
||||
application_deployment_queue_id: $deployment->id,
|
||||
);
|
||||
@@ -141,19 +142,46 @@ function queue_next_deployment(Application $application)
|
||||
->sortBy('created_at');
|
||||
|
||||
foreach ($queued_deployments as $next_deployment) {
|
||||
// Check if this queued deployment can actually run
|
||||
if (next_queuable($next_deployment->server_id, $next_deployment->application_id, $next_deployment->commit, $next_deployment->pull_request_id)) {
|
||||
$next_deployment->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
|
||||
ApplicationDeploymentJob::dispatch(
|
||||
application_deployment_queue_id: $next_deployment->id,
|
||||
);
|
||||
}
|
||||
start_queued_deployment($next_deployment);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
function start_queued_deployment(ApplicationDeploymentQueue $deployment): bool
|
||||
{
|
||||
$started = DB::transaction(function () use ($deployment): 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();
|
||||
|
||||
if (! $current || $current->status !== ApplicationDeploymentStatus::QUEUED->value) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (! next_queuable($current->server_id, $current->application_id, $current->commit, $current->pull_request_id)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$current->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
if ($started) {
|
||||
ApplicationDeploymentJob::dispatch(
|
||||
application_deployment_queue_id: $deployment->id,
|
||||
);
|
||||
}
|
||||
|
||||
return $started;
|
||||
}
|
||||
|
||||
function next_queuable(string $server_id, string $application_id, string $commit = 'HEAD', int $pull_request_id = 0): bool
|
||||
{
|
||||
// Check if there's already a deployment in progress for this application with the same pull_request_id
|
||||
@@ -188,19 +216,8 @@ function next_after_cancel(?Server $server = null)
|
||||
->get()
|
||||
->sortBy('created_at');
|
||||
|
||||
if ($next_found->count() > 0) {
|
||||
foreach ($next_found as $next) {
|
||||
// Use next_queuable to properly check if this deployment can run
|
||||
if (next_queuable($next->server_id, $next->application_id, $next->commit, $next->pull_request_id)) {
|
||||
$next->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
|
||||
ApplicationDeploymentJob::dispatch(
|
||||
application_deployment_queue_id: $next->id,
|
||||
);
|
||||
}
|
||||
}
|
||||
foreach ($next_found as $next) {
|
||||
start_queued_deployment($next);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -273,6 +273,46 @@ describe('combined deployment notification', function () {
|
||||
Notification::assertNotSentTo($this->team, DeploymentFailed::class);
|
||||
});
|
||||
|
||||
test('sends the summary when the last additional server deployment is cancelled while running', function () {
|
||||
$parentUuid = new_public_id();
|
||||
Cache::put("multi-server-deployment-queued:{$parentUuid}", true);
|
||||
createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::FINISHED->value, $parentUuid);
|
||||
$cancelled = createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::CANCELLED_BY_USER->value, $parentUuid);
|
||||
$job = makeMultiServerDeploymentJob(['application' => $this->application, 'application_deployment_queue' => $cancelled]);
|
||||
|
||||
expect(fn () => callMultiServerDeploymentJob($job, 'failDeployment'))
|
||||
->toThrow(DeploymentException::class, 'Deployment cancelled by user');
|
||||
|
||||
Notification::assertSentToTimes($this->team, DeploymentFailed::class, 1);
|
||||
Notification::assertSentTo($this->team, DeploymentFailed::class, fn (DeploymentFailed $notification) => $notification->deployment_uuid === $cancelled->deployment_uuid);
|
||||
Notification::assertNotSentTo($this->team, DeploymentSuccess::class);
|
||||
});
|
||||
|
||||
test('sends the summary when the last additional server deployment is cancelled before it starts', function () {
|
||||
$parentUuid = new_public_id();
|
||||
Cache::put("multi-server-deployment-queued:{$parentUuid}", true);
|
||||
createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::FINISHED->value, $parentUuid);
|
||||
$cancelled = createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::CANCELLED_BY_USER->value, $parentUuid);
|
||||
[$job] = makeMultiServerDeploymentJob(['application' => $this->application, 'application_deployment_queue' => $cancelled]);
|
||||
|
||||
$job->handle();
|
||||
|
||||
Notification::assertSentToTimes($this->team, DeploymentFailed::class, 1);
|
||||
Notification::assertSentTo($this->team, DeploymentFailed::class, fn (DeploymentFailed $notification) => $notification->deployment_uuid === $cancelled->deployment_uuid);
|
||||
});
|
||||
|
||||
test('does not notify when a cancelled additional server deployment is not the last one', function () {
|
||||
$parentUuid = new_public_id();
|
||||
Cache::put("multi-server-deployment-queued:{$parentUuid}", true);
|
||||
createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::IN_PROGRESS->value, $parentUuid);
|
||||
$cancelled = createMultiServerDeployment($this->application, $this->extraServer, ApplicationDeploymentStatus::CANCELLED_BY_USER->value, $parentUuid);
|
||||
[$job] = makeMultiServerDeploymentJob(['application' => $this->application, 'application_deployment_queue' => $cancelled]);
|
||||
|
||||
$job->handle();
|
||||
|
||||
Notification::assertNothingSent();
|
||||
});
|
||||
|
||||
test('sends a failed notification that links to the failed server deployment', function () {
|
||||
$parentUuid = new_public_id();
|
||||
Cache::put("multi-server-deployment-queued:{$parentUuid}", true);
|
||||
@@ -297,15 +337,21 @@ class MultiServerRecordingDeploymentJob extends ApplicationDeploymentJob
|
||||
|
||||
public function __construct() {}
|
||||
|
||||
/** Output that `docker images -q` returns; empty means the image is missing. */
|
||||
public string $localImageOutput = 'sha256:0123456789ab';
|
||||
|
||||
public function execute_remote_command(...$commands): void
|
||||
{
|
||||
foreach ($commands as $command) {
|
||||
$this->recordedCommands[] = $command['command'] ?? $command[0];
|
||||
if (isset($command['save'])) {
|
||||
(new ReflectionProperty(ApplicationDeploymentJob::class, 'saved_outputs'))->getValue($this)->put($command['save'], $this->localImageOutput);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function makeInlineDockerfileJob(Application $application, bool $isAdditionalServer): array
|
||||
function makeInlineDockerfileJob(Application $application, bool $isAdditionalServer, ?Server $server = null): array
|
||||
{
|
||||
$reflection = new ReflectionClass(ApplicationDeploymentJob::class);
|
||||
$job = new MultiServerRecordingDeploymentJob;
|
||||
@@ -316,6 +362,8 @@ function makeInlineDockerfileJob(Application $application, bool $isAdditionalSer
|
||||
'application_deployment_queue' => $queue,
|
||||
'is_this_additional_server' => $isAdditionalServer,
|
||||
'pull_request_id' => 0,
|
||||
'saved_outputs' => collect(),
|
||||
'server' => $server ?? $application->destination->server,
|
||||
] as $name => $value) {
|
||||
$reflection->getProperty($name)->setValue($job, $value);
|
||||
}
|
||||
@@ -336,7 +384,19 @@ describe('inline Dockerfile image on additional servers', function () {
|
||||
[$job, $reflection] = makeInlineDockerfileJob($this->application->fresh(), true);
|
||||
|
||||
expect($reflection->getMethod('pull_image_for_additional_server')->invoke($job))->toBeTrue()
|
||||
->and($job->recordedCommands)->toBe(["docker pull 'registry.example.com/nginx-multi:latest'"]);
|
||||
->and($job->recordedCommands)->toBe([
|
||||
"docker pull 'registry.example.com/nginx-multi:latest'",
|
||||
"docker images -q 'registry.example.com/nginx-multi:latest' 2>/dev/null",
|
||||
]);
|
||||
});
|
||||
|
||||
test('a deployments only server explains that the main server must push a missing image', function () {
|
||||
$this->extraServer->settings->update(['server_role' => 'deployment']);
|
||||
[$job, $reflection] = makeInlineDockerfileJob($this->application->fresh(), true, $this->extraServer->fresh());
|
||||
$job->localImageOutput = '';
|
||||
|
||||
expect(fn () => $reflection->getMethod('pull_image_for_additional_server')->invoke($job))
|
||||
->toThrow(DeploymentException::class, 'Image (registry.example.com/nginx-multi:latest) not found in the registry; the main server must push it first. The server (extra-server) is set to deployments only and cannot build it.');
|
||||
});
|
||||
|
||||
test('the main server builds the image', function () {
|
||||
|
||||
@@ -61,6 +61,62 @@ describe('queue_application_deployment commit resolution', function () {
|
||||
expect($creationLevel)->toBeGreaterThan($initialLevel);
|
||||
});
|
||||
|
||||
test('starts a deployment only while the admission transaction is open', function () {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
$initialLevel = DB::transactionLevel();
|
||||
$startLevel = null;
|
||||
|
||||
ApplicationDeploymentQueue::updated(function (ApplicationDeploymentQueue $deployment) use (&$startLevel): void {
|
||||
if ($deployment->status === ApplicationDeploymentStatus::IN_PROGRESS->value) {
|
||||
$startLevel = DB::transactionLevel();
|
||||
}
|
||||
});
|
||||
|
||||
$result = queue_application_deployment($application, 'atomic-start');
|
||||
|
||||
expect($result['status'])->toBe('queued')
|
||||
->and($startLevel)->toBeGreaterThan($initialLevel);
|
||||
Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1);
|
||||
});
|
||||
|
||||
test('starts the next queued deployment only while a transaction is open', function (string $trigger) {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
$this->server->settings->update(['concurrent_builds' => 0]);
|
||||
queue_application_deployment($application, 'waiting-deployment');
|
||||
$this->server->settings->update(['concurrent_builds' => 1]);
|
||||
$initialLevel = DB::transactionLevel();
|
||||
$startLevel = null;
|
||||
|
||||
ApplicationDeploymentQueue::updated(function (ApplicationDeploymentQueue $deployment) use (&$startLevel): void {
|
||||
if ($deployment->status === ApplicationDeploymentStatus::IN_PROGRESS->value) {
|
||||
$startLevel = DB::transactionLevel();
|
||||
}
|
||||
});
|
||||
|
||||
$trigger === 'queue_next_deployment'
|
||||
? queue_next_deployment($application)
|
||||
: next_after_cancel($this->server);
|
||||
|
||||
expect($startLevel)->toBeGreaterThan($initialLevel)
|
||||
->and(ApplicationDeploymentQueue::query()->sole()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value);
|
||||
Bus::assertDispatchedTimes(ApplicationDeploymentJob::class, 1);
|
||||
})->with(['queue_next_deployment', 'next_after_cancel']);
|
||||
|
||||
test('does not start a queued deployment that another worker already started', function () {
|
||||
$application = makeApplication($this->environment->id, $this->destination->id, 'HEAD');
|
||||
$this->server->settings->update(['concurrent_builds' => 0]);
|
||||
queue_application_deployment($application, 'stale-deployment');
|
||||
$this->server->settings->update(['concurrent_builds' => 5]);
|
||||
$stale = ApplicationDeploymentQueue::query()->sole();
|
||||
|
||||
ApplicationDeploymentQueue::query()->whereKey($stale->id)->update([
|
||||
'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
|
||||
]);
|
||||
|
||||
expect(start_queued_deployment($stale))->toBeFalse();
|
||||
Bus::assertNotDispatched(ApplicationDeploymentJob::class);
|
||||
});
|
||||
|
||||
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