From bb6dfd147ec6fdaa4e7c1e75fea7f4c05aededc4 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:12:58 +0100 Subject: [PATCH] fix(mcp): queue MCP deployments behind a deployment in progress The MCP deploy tool and control start skipped the deployment queue, so two rolling updates of one application could run at once and remove the new container of each other. A rolling update now also removes only the containers that ran before it started. Closes #12442 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + app/Jobs/ApplicationDeploymentJob.php | 37 ++++-- app/Mcp/Tools/Control.php | 3 +- app/Mcp/Tools/Deploy.php | 3 +- tests/Feature/Mcp/McpDeployQueueTest.php | 105 ++++++++++++++++++ ...plicationDeploymentContainerNamingTest.php | 32 ++++++ 6 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 tests/Feature/Mcp/McpDeployQueueTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d57334dcf..56b650cf55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- Queue a deployment from the MCP `deploy` tool and the MCP `control` start action while a deployment of the same application is in progress, as the API and the **Deploy** button do. Before, both started at once, so two rolling updates could remove the new container of each other and stop the application. A deployment of the same commit is now skipped, and the response gives the UUID of the deployment in progress. A rolling update now also removes only the containers that ran before it started ([#12442](https://github.com/coollabsio/coolify/issues/12442)) - Do not copy the preview deployments of an application when you clone it. A clone of an application with a fixed preview URL template (for example `demo-mr-{{pr_id}}.example.com`) failed with a duplicate preview domain error and left a stopped, partial clone. The clone gets its own previews from new pull requests - Handle an existing Cloudflare DNS record with a different IP when Coolify adds the DNS record of a new domain automatically. Coolify now replaces the record when it created the record and no other resource uses it. Otherwise it opens the **Configure DNS** dialog, where you can replace the record, and the domain shows the conflict instead of "Not checked" - Add the Cloudflare DNS record automatically, or open the **Configure DNS** dialog, also when you edit a domain to a hostname in a connected Cloudflare zone, for example when you change a generated domain to your own domain. Before, Coolify did this only when you added a new domain. This applies to the **Domains** pages, the application and service application settings, and the service application API diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 1a4949e9b2..25dca2348b 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -141,6 +141,14 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue private ?string $currently_running_container_name = null; + /** + * Containers that ran before the rolling update started its new container. Only these are + * removed after it, so the new container of another deployment of the application stays. + * + * @var list|null + */ + private ?array $containerNamesBeforeRollingUpdate = null; + private string $basedir; private string $workdir; @@ -2944,6 +2952,9 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } else { $this->application_deployment_queue->addLogEntry('----------------------------------------'); $this->application_deployment_queue->addLogEntry('Rolling update started.'); + $this->containerNamesBeforeRollingUpdate = getCurrentApplicationContainerStatus($this->server, $this->application, $this->pull_request_id) + ->pluck('Names') + ->all(); $this->start_by_compose_file(); $this->health_check(); $this->wait_for_proxy_to_route_new_container(); @@ -5310,13 +5321,8 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); }); } else { $containers = getCurrentApplicationContainerStatus($this->server, $this->application, $this->pull_request_id); - if ($this->pull_request_id === 0) { - $containers = $containers->filter(function ($container) { - return data_get($container, 'Names') !== $this->container_name && data_get($container, 'Names') !== addPreviewDeploymentSuffix($this->container_name, $this->pull_request_id); - }); - } - $containers->each(function ($container) { - $this->graceful_shutdown_container(data_get($container, 'Names')); + $this->oldGeneratedContainerNames($containers)->each(function (string $containerName) { + $this->graceful_shutdown_container($containerName); }); } } else { @@ -5346,6 +5352,23 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); } } + /** + * The containers to remove when container names are generated per deployment: all except the + * new one, and after a rolling update only the ones that ran before its new container started. + */ + private function oldGeneratedContainerNames(Collection $containers): Collection + { + $names = $containers->pluck('Names')->filter(); + if ($this->pull_request_id === 0) { + $names = $names->reject(fn (string $name) => $name === $this->container_name || $name === addPreviewDeploymentSuffix($this->container_name, $this->pull_request_id)); + } + if ($this->containerNamesBeforeRollingUpdate !== null) { + $names = $names->filter(fn (string $name) => in_array($name, $this->containerNamesBeforeRollingUpdate, true)); + } + + return $names->values(); + } + private function containerNamesToRemove(Collection $containers): Collection { return $containers diff --git a/app/Mcp/Tools/Control.php b/app/Mcp/Tools/Control.php index e65c64ebe9..6622e381c7 100644 --- a/app/Mcp/Tools/Control.php +++ b/app/Mcp/Tools/Control.php @@ -126,13 +126,12 @@ class Control extends Tool force_rebuild: false, restart_only: $action === 'restart', is_mcp: true, - no_questions_asked: $action === 'start', ); if (($result['status'] ?? null) === 'skipped' || ($result['status'] ?? null) === 'queue_full') { return [ 'message' => $result['message'] ?? 'Deployment skipped.', - 'deployment_uuid' => null, + 'deployment_uuid' => $result['deployment_uuid'] ?? null, ]; } diff --git a/app/Mcp/Tools/Deploy.php b/app/Mcp/Tools/Deploy.php index fd15fffa1c..43e143a5f5 100644 --- a/app/Mcp/Tools/Deploy.php +++ b/app/Mcp/Tools/Deploy.php @@ -80,7 +80,6 @@ class Deploy extends Tool pull_request_id: $pullRequestId, force_rebuild: $force, is_mcp: true, - no_questions_asked: true, cancel_previous: $cancelPrevious, ); @@ -88,7 +87,7 @@ class Deploy extends Tool return $this->mcpSuccess($request, $this->respond([ 'ok' => false, 'message' => $result['message'] ?? 'Deployment not queued.', - 'deployment_uuid' => null, + 'deployment_uuid' => $result['deployment_uuid'] ?? null, ]), ['resource_uuid' => $uuid]); } diff --git a/tests/Feature/Mcp/McpDeployQueueTest.php b/tests/Feature/Mcp/McpDeployQueueTest.php new file mode 100644 index 0000000000..fdc6dd7f16 --- /dev/null +++ b/tests/Feature/Mcp/McpDeployQueueTest.php @@ -0,0 +1,105 @@ + 'array', + 'session.driver' => 'array', + 'app.maintenance.driver' => 'file', + ]); + Queue::fake(); + + InstanceSettings::query()->delete(); + $settings = new InstanceSettings(['is_mcp_server_enabled' => true]); + $settings->id = 0; + $settings->save(); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $server = Server::factory()->create(['team_id' => $this->team->id]); + $destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = $project->environments()->first() + ?? 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->running = ApplicationDeploymentQueue::create([ + 'application_id' => $this->application->id, + 'deployment_uuid' => 'running-deployment', + 'server_id' => $server->id, + 'destination_id' => $destination->id, + 'pull_request_id' => 0, + 'commit' => 'abc1234', + 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value, + ]); +}); + +function mcpDeployQueueCall(string $tool, array $arguments) +{ + auth()->forgetGuards(); + $token = test()->user->createToken('mcp-deploy-queue', ['read', 'deploy'])->plainTextToken; + + return test()->withHeaders([ + 'Content-Type' => 'application/json', + 'Accept' => 'application/json, text/event-stream', + 'Authorization' => 'Bearer '.$token, + ])->postJson('/mcp', [ + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'tools/call', + 'params' => ['name' => $tool, 'arguments' => (object) $arguments], + ]); +} + +function mcpDeployQueueNewDeployment(): ?ApplicationDeploymentQueue +{ + return ApplicationDeploymentQueue::query()->where('deployment_uuid', '!=', 'running-deployment')->first(); +} + +test('deploy tool queues behind a deployment of the same application that is in progress', function () { + $response = mcpDeployQueueCall('deploy', ['uuid' => $this->application->uuid]); + + $response->assertOk(); + expect($response->json('result.isError'))->toBeFalse() + ->and(mcpDeployQueueNewDeployment()?->status)->toBe(ApplicationDeploymentStatus::QUEUED->value) + ->and($this->running->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); +}); + +test('deploy tool skips a deployment of a commit that is already in progress', function () { + $this->application->update(['git_commit_sha' => 'abc1234']); + + $response = mcpDeployQueueCall('deploy', ['uuid' => $this->application->uuid]); + + $response->assertOk(); + expect($response->json('result.content.0.text'))->toContain('Deployment already queued for this commit.') + ->toContain('running-deployment') + ->and(mcpDeployQueueNewDeployment())->toBeNull(); +}); + +test('control start queues behind a deployment of the same application that is in progress', function () { + $response = mcpDeployQueueCall('control', ['resource' => 'application', 'action' => 'start', 'uuid' => $this->application->uuid]); + + $response->assertOk(); + expect($response->json('result.isError'))->toBeFalse($response->json('result.content.0.text')) + ->and(mcpDeployQueueNewDeployment()?->status)->toBe(ApplicationDeploymentStatus::QUEUED->value); +}); diff --git a/tests/Unit/ApplicationDeploymentContainerNamingTest.php b/tests/Unit/ApplicationDeploymentContainerNamingTest.php index 385e547eb6..462c4614ed 100644 --- a/tests/Unit/ApplicationDeploymentContainerNamingTest.php +++ b/tests/Unit/ApplicationDeploymentContainerNamingTest.php @@ -83,3 +83,35 @@ it('uses the container name prefix for generated container names only', function ->and(generateApplicationContainerName(applicationWithContainerNamePrefix(consistent: true)))->toBe('application-uuid') ->and(generateApplicationContainerName(applicationWithContainerNamePrefix(), 42))->toBe('application-uuid-pr-42'); }); + +it('removes all old generated containers but not the new one', function () { + $application = applicationWithContainerNaming(); + $application->settings->is_consistent_container_name_enabled = false; + [$job, $reflection] = containerNamingJob($application); + $reflection->getProperty('container_name')->setValue($job, 'application-uuid-20260908T141530'); + + $containers = new Collection([ + ['Names' => 'application-uuid-20260101T000000'], + ['Names' => 'application-uuid-20260908T141530'], + ]); + + expect($reflection->getMethod('oldGeneratedContainerNames')->invoke($job, $containers)->all()) + ->toBe(['application-uuid-20260101T000000']); +}); + +it('keeps a container that another deployment started during the rolling update', function () { + $application = applicationWithContainerNaming(); + $application->settings->is_consistent_container_name_enabled = false; + [$job, $reflection] = containerNamingJob($application); + $reflection->getProperty('container_name')->setValue($job, 'application-uuid-20260908T141530'); + $reflection->getProperty('containerNamesBeforeRollingUpdate')->setValue($job, ['application-uuid-20260101T000000']); + + $containers = new Collection([ + ['Names' => 'application-uuid-20260101T000000'], + ['Names' => 'application-uuid-20260908T141530'], + ['Names' => 'application-uuid-20260908T141548'], + ]); + + expect($reflection->getMethod('oldGeneratedContainerNames')->invoke($job, $containers)->all()) + ->toBe(['application-uuid-20260101T000000']); +});