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']); +});