fix(deployments): apply the concurrent build limit of build servers

The queue counted every running deployment against the deployment server,
and the job picked a build server at random. So the deployment server
limit capped builds on build servers, and the build server limit was
never read. Count each build on the server that runs it, reserve a build
server with a free slot when the deployment starts, and start waiting
deployments of other servers when a build server slot is freed.

Closes #7649

app/Jobs/ApplicationDeploymentJob.php

@@ -530,8 +530,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
 
         // A deployments-only server never builds. Docker image and Compose applications are exempt:
         // the first builds nothing and the second does not support build servers.
-        $mustBuildElsewhere = ! $this->server->canBuildApplications()
-            && ! in_array($this->application->build_pack, ['dockerimage', 'dockercompose'], true);
+        $mustBuildElsewhere = $this->application->mustBuildOnBuildServer($this->server);
 
         if (! $mustBuildElsewhere && ! data_get($this->application, 'settings.is_build_server_enabled')) {
             return;
@@ -542,7 +541,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
         }
 
         $team = $this->application->environment->project->team;
-        $buildServers = Server::buildServers($team->id)->whereKeyNot($this->server->id)->get();
+        $buildServers = $this->application->buildServerCandidates($this->server);
 
         if ($buildServers->isEmpty()) {
             // A restart only rebuilds when the image is missing, so it may still run on the deployment server.
@@ -559,7 +558,12 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
             return;
         }
 
-        $this->build_server = $buildServers->random();
+        // Admission reserved a build server with a free build slot. A forced start reserved none, and
+        // the reserved server can become unusable before the job runs.
+        $reservedId = $this->application_deployment_queue->build_server_id;
+        $this->build_server = ($reservedId === null ? null : $buildServers->firstWhere('id', $reservedId))
+            ?? least_busy_build_server($buildServers)
+            ?? least_busy_build_server($buildServers, onlyWithFreeSlot: false);
         // Saved now: the cancel action looks for the helper container on this server.
         $this->application_deployment_queue->update(['build_server_id' => $this->build_server->id]);
         $this->application_deployment_queue->addLogEntry("Found a suitable build server ({$this->build_server->name}).");

app/Models/Application.php

@@ -1116,6 +1116,40 @@ class Application extends BaseModel
         return data_get($this, 'environment.project.team');
     }
 
+    /**
+     * A deployments-only server cannot build this application, so a build server must build it.
+     * Docker image applications build nothing, and Compose applications do not use build servers.
+     */
+    public function mustBuildOnBuildServer(Server $server): bool
+    {
+        return ! $server->canBuildApplications()
+            && ! in_array($this->build_pack, ['dockerimage', 'dockercompose'], true);
+    }
+
+    /**
+     * The build servers that can build a deployment of this application on $server. It is empty when
+     * the deployment builds on $server, or when $server is an additional server that pulls the image.
+     *
+     * @return Collection<int, Server>
+     */
+    public function buildServerCandidates(Server $server): Collection
+    {
+        $pullsImage = str($this->docker_registry_image_name)->isNotEmpty()
+            && $this->additional_servers()->wherePivot('server_id', $server->id)->exists();
+        if ($pullsImage) {
+            return collect();
+        }
+        if (! $this->mustBuildOnBuildServer($server) && ! data_get($this, 'settings.is_build_server_enabled')) {
+            return collect();
+        }
+        $teamId = $this->team()?->id;
+        if ($teamId === null) {
+            return collect();
+        }
+
+        return Server::buildServers($teamId)->whereKeyNot($server->id)->get();
+    }
+
     /**
      * Another application of the team that builds and pushes an image with the same Docker image name.
      * Both push the same tags (the commit SHA, `latest`, `pr-<id>`), so one application can run the image of the other.

bootstrap/helpers/applications.php

@@ -102,7 +102,7 @@ function queue_application_deployment(Application $application, string $deployme
         // 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.
         // After a cancelled build, CancelSupersededDeploymentJob starts it once the build is stopped.
-        $started = $superseded['building']->isEmpty() && ($no_questions_asked || next_queuable($server_id, $application_id, $commit, $pull_request_id));
+        $started = $superseded['building']->isEmpty() && ($no_questions_asked || next_queuable($deployment));
         if ($started) {
             $deployment->update([
                 'status' => ApplicationDeploymentStatus::IN_PROGRESS->value,
@@ -270,8 +270,14 @@ function start_next_queued_deployments(Application $application, ?int $finished_
         ->where('status', ApplicationDeploymentStatus::QUEUED)
         ->distinct()
         ->pluck('server_id');
+    // A build on a build server frees a slot that deployments on other servers of the team can wait for.
+    $primary_server = $application->destination?->server;
+    $build_server_team_server_ids = $primary_server && $application->buildServerCandidates($primary_server)->isNotEmpty()
+        ? Server::where('team_id', $primary_server->team_id)->pluck('id')
+        : collect();
     $server_ids = collect([$application->destination?->server_id, $finished_deployment_server_id])
         ->merge($application_queued_server_ids)
+        ->merge($build_server_team_server_ids)
         ->filter(fn ($server_id) => $server_id !== null)
         ->unique()
         ->values();
@@ -306,7 +312,7 @@ function start_queued_deployment(ApplicationDeploymentQueue $deployment, bool $f
             return false;
         }
 
-        if (! $force && ! next_queuable($current->server_id, $current->application_id, $current->commit, $current->pull_request_id)) {
+        if (! $force && ! next_queuable($current)) {
             return false;
         }
 
@@ -381,12 +387,18 @@ function fail_undispatchable_deployment(ApplicationDeploymentQueue $deployment,
     }
 }
 
-function next_queuable(string $server_id, string $application_id, string $commit = 'HEAD', int $pull_request_id = 0): bool
+/**
+ * Decide whether a queued deployment can start now. Each deployment holds one build slot of the
+ * server that builds it: its deployment server, or a build server of the team. When it builds on a
+ * build server, it reserves the least busy one with a free slot in build_server_id, which the caller
+ * saves with the status change. Call it while the application and deployment server rows are locked.
+ */
+function next_queuable(ApplicationDeploymentQueue $deployment): bool
 {
     // Check if there's already a deployment in progress for this application with the same pull_request_id
     // This allows normal deployments and PR deployments to run concurrently
-    $in_progress = ApplicationDeploymentQueue::where('application_id', $application_id)
-        ->where('pull_request_id', $pull_request_id)
+    $in_progress = ApplicationDeploymentQueue::where('application_id', $deployment->application_id)
+        ->where('pull_request_id', $deployment->pull_request_id)
         ->where('status', ApplicationDeploymentStatus::IN_PROGRESS->value)
         ->exists();
 
@@ -394,23 +406,57 @@ function next_queuable(string $server_id, string $application_id, string $commit
         return false;
     }
 
-    // Check server's concurrent build limit
     // A deleted server keeps its queued deployments; they cannot run anymore.
-    $server = Server::find($server_id);
-    if (! $server) {
+    $server = Server::find($deployment->server_id);
+    $application = Application::find($deployment->application_id);
+    if (! $server || ! $application) {
         return false;
     }
-    $concurrent_builds = $server->settings->concurrent_builds;
-    $active_deployments = ApplicationDeploymentQueue::where('server_id', $server_id)
-        ->where('status', ApplicationDeploymentStatus::IN_PROGRESS->value)
-        ->count();
 
-    if ($active_deployments >= $concurrent_builds) {
+    $buildServers = $application->buildServerCandidates($server);
+    if ($buildServers->isEmpty()) {
+        return active_builds_on_server($server->id) < $server->settings->concurrent_builds;
+    }
+
+    // Deployments of other servers can take the same build slots, so lock the build servers too.
+    Server::query()->whereKey($buildServers->modelKeys())->orderBy('id')->lockForUpdate()->get();
+    $buildServer = least_busy_build_server($buildServers);
+    if (! $buildServer) {
         return false;
     }
+    $deployment->build_server_id = $buildServer->id;
 
     return true;
 }
+
+/**
+ * The running deployments that build on a server: its own deployments without a build server,
+ * and the deployments that it builds as a build server.
+ */
+function active_builds_on_server(int $server_id): int
+{
+    return ApplicationDeploymentQueue::where('status', ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->where(fn ($query) => $query->where('build_server_id', $server_id)
+            ->orWhere(fn ($query) => $query->where('server_id', $server_id)->whereNull('build_server_id')))
+        ->count();
+}
+
+/**
+ * The build server with the fewest running builds, at random among equals. With $onlyWithFreeSlot,
+ * a server that already runs as many builds as its "Concurrent builds" setting allows is skipped,
+ * and null means that all servers are full.
+ *
+ * @param  Collection<int, Server>  $buildServers
+ */
+function least_busy_build_server(Collection $buildServers, bool $onlyWithFreeSlot = true): ?Server
+{
+    return $buildServers
+        ->map(fn (Server $buildServer) => ['server' => $buildServer, 'builds' => active_builds_on_server($buildServer->id)])
+        ->filter(fn (array $candidate) => ! $onlyWithFreeSlot || $candidate['builds'] < $candidate['server']->settings->concurrent_builds)
+        ->shuffle()
+        ->sortBy('builds')
+        ->first()['server'] ?? null;
+}
 /**
  * Start the queued deployments that can run after a cancellation. With the application of the
  * cancelled deployment, its queued deployments on all servers start too, as when a deployment ends.

tests/Feature/BuildServerConcurrentBuildsTest.php

@@ -0,0 +1,149 @@
+<?php
+
+use App\Enums\ApplicationDeploymentStatus;
+use App\Jobs\ApplicationDeploymentJob;
+use App\Models\Application;
+use App\Models\ApplicationDeploymentQueue;
+use App\Models\Environment;
+use App\Models\InstanceSettings;
+use App\Models\PrivateKey;
+use App\Models\Project;
+use App\Models\Server;
+use App\Models\StandaloneDocker;
+use App\Models\Team;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Notification;
+use Illuminate\Support\Facades\Process;
+use Illuminate\Support\Facades\Queue;
+use Illuminate\Support\Facades\Storage;
+
+uses(RefreshDatabase::class);
+
+beforeEach(function () {
+    Server::flushIdentityMap();
+    Queue::fake();
+    Notification::fake();
+    Storage::fake('ssh-keys');
+    Storage::fake('ssh-mux');
+    Process::fake(['*' => Process::result(output: '')]);
+    config([
+        'constants.ssh.mux_enabled' => false,
+        'cache.default' => 'array',
+        'app.maintenance.store' => 'array',
+    ]);
+
+    $settings = new InstanceSettings;
+    $settings->id = 0;
+    $settings->save();
+
+    $this->team = Team::factory()->create();
+    $privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]);
+    $environment = Environment::factory()->create([
+        'project_id' => Project::factory()->create(['team_id' => $this->team->id])->id,
+    ]);
+    $ip = 40;
+
+    $this->makeServer = function (int $concurrentBuilds, string $role = 'both') use ($privateKey, &$ip): Server {
+        $server = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $privateKey->id, 'ip' => '203.0.113.'.$ip++]);
+        $server->settings->update([
+            'concurrent_builds' => $concurrentBuilds,
+            'server_role' => $role,
+            'is_build_server' => $role === 'build',
+            'is_reachable' => true,
+            'is_usable' => true,
+        ]);
+
+        return $server->fresh();
+    };
+
+    $this->makeApplication = function (Server $server, bool $usesBuildServer = true) use ($environment): Application {
+        $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail();
+        $application = Application::factory()->create([
+            'environment_id' => $environment->id,
+            'destination_id' => $destination->id,
+            'destination_type' => $destination->getMorphClass(),
+        ]);
+        $application->settings->update(['is_build_server_enabled' => $usesBuildServer]);
+
+        return $application->fresh();
+    };
+
+    $this->deploy = function (Application $application): ApplicationDeploymentQueue {
+        $uuid = new_public_id();
+        queue_application_deployment($application, $uuid);
+
+        return ApplicationDeploymentQueue::where('deployment_uuid', $uuid)->firstOrFail();
+    };
+});
+
+afterEach(function () {
+    Server::flushIdentityMap();
+});
+
+test('the deployment server limit does not cap builds that run on a build server', function () {
+    $deploymentServer = ($this->makeServer)(1);
+    $buildServer = ($this->makeServer)(2, 'build');
+
+    $first = ($this->deploy)(($this->makeApplication)($deploymentServer));
+    $second = ($this->deploy)(($this->makeApplication)($deploymentServer));
+    $third = ($this->deploy)(($this->makeApplication)($deploymentServer));
+
+    expect($first->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($first->build_server_id)->toBe($buildServer->id)
+        ->and($second->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($second->build_server_id)->toBe($buildServer->id)
+        ->and($third->status)->toBe(ApplicationDeploymentStatus::QUEUED->value)
+        ->and($third->build_server_id)->toBeNull();
+});
+
+test('the build server limit holds for deployments of different deployment servers', function () {
+    $buildServer = ($this->makeServer)(1, 'build');
+
+    $first = ($this->deploy)(($this->makeApplication)(($this->makeServer)(5)));
+    $second = ($this->deploy)(($this->makeApplication)(($this->makeServer)(5)));
+
+    expect($first->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($first->build_server_id)->toBe($buildServer->id)
+        ->and($second->status)->toBe(ApplicationDeploymentStatus::QUEUED->value);
+    Queue::assertPushed(ApplicationDeploymentJob::class, 1);
+});
+
+test('a deployment reserves the build server that has a free build slot', function () {
+    $deploymentServer = ($this->makeServer)(5);
+    $buildServers = collect([($this->makeServer)(1, 'build'), ($this->makeServer)(1, 'build')]);
+
+    $first = ($this->deploy)(($this->makeApplication)($deploymentServer));
+    $second = ($this->deploy)(($this->makeApplication)($deploymentServer));
+
+    expect([$first->build_server_id, $second->build_server_id])
+        ->toEqualCanonicalizing($buildServers->pluck('id')->all())
+        ->and($second->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value);
+});
+
+test('a finished build starts the deployment of another deployment server that waits for the build server', function () {
+    $buildServer = ($this->makeServer)(1, 'build');
+    $running = ($this->deploy)(($this->makeApplication)(($this->makeServer)(5)));
+    $waiting = ($this->deploy)(($this->makeApplication)(($this->makeServer)(5)));
+    expect($waiting->status)->toBe(ApplicationDeploymentStatus::QUEUED->value);
+
+    (new ApplicationDeploymentJob($running->id))->failed(new RuntimeException('build failed'));
+
+    $waiting->refresh();
+    expect($waiting->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($waiting->build_server_id)->toBe($buildServer->id);
+    Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $waiting->id);
+});
+
+test('builds on a build server do not take the build slots of the deployment server', function () {
+    $deploymentServer = ($this->makeServer)(1);
+    ($this->makeServer)(1, 'build');
+
+    $onBuildServer = ($this->deploy)(($this->makeApplication)($deploymentServer));
+    $onDeploymentServer = ($this->deploy)(($this->makeApplication)($deploymentServer, usesBuildServer: false));
+    $waiting = ($this->deploy)(($this->makeApplication)($deploymentServer, usesBuildServer: false));
+
+    expect($onBuildServer->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($onDeploymentServer->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value)
+        ->and($onDeploymentServer->build_server_id)->toBeNull()
+        ->and($waiting->status)->toBe(ApplicationDeploymentStatus::QUEUED->value);
+});

tests/Feature/BuildServerFallbackPolicyTest.php

@@ -23,7 +23,7 @@ afterEach(function () {
     Server::flushIdentityMap();
 });
 
-function makeBuildServerSelectionJob(Team $team, Server $deploymentServer, array $applicationAttributes = [], bool $buildServerEnabled = true, bool $restartOnly = false): array
+function makeBuildServerSelectionJob(Team $team, Server $deploymentServer, array $applicationAttributes = [], bool $buildServerEnabled = true, bool $restartOnly = false, ?int $reservedBuildServerId = null): array
 {
     $project = new Project;
     $project->setRelation('team', $team);
@@ -40,6 +40,7 @@ function makeBuildServerSelectionJob(Team $team, Server $deploymentServer, array
     $application->setRelation('settings', $settings);
 
     $deploymentQueue = Mockery::mock(ApplicationDeploymentQueue::class);
+    $deploymentQueue->shouldReceive('getAttribute')->with('build_server_id')->andReturn($reservedBuildServerId)->byDefault();
     $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor();
 
     foreach ([
@@ -501,3 +502,54 @@ test('the selected build server is saved on the deployment so a cancel finds the
     expect($deployment->build_server_id)->toBe($buildServer->id)
         ->and(ApplicationDeploymentQueue::query()->find($deployment->id)->build_server_id)->toBe($buildServer->id);
 });
+
+function makeAvailableBuildServer(Team $team, int $concurrentBuilds = 1): Server
+{
+    $server = Server::factory()->create(['team_id' => $team->id]);
+    $server->settings()->update([
+        'is_reachable' => true,
+        'is_usable' => true,
+        'server_role' => 'build',
+        'is_build_server' => true,
+        'concurrent_builds' => $concurrentBuilds,
+    ]);
+
+    return $server;
+}
+
+test('the job builds on the build server that admission reserved', function () {
+    $team = Team::factory()->create();
+    $deploymentServer = Server::factory()->create(['team_id' => $team->id]);
+    makeAvailableBuildServer($team);
+    $reserved = makeAvailableBuildServer($team);
+    [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer, reservedBuildServerId: $reserved->id);
+
+    $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $reserved->id])->once()->andReturnTrue();
+    $deploymentQueue->shouldReceive('addLogEntry')->once();
+
+    invokeBuildServerSelection($job);
+
+    expect(selectedBuildServer($job)->is($reserved))->toBeTrue();
+});
+
+test('a force-started job without a reservation builds on a build server with a free build slot', function () {
+    $team = Team::factory()->create();
+    $deploymentServer = Server::factory()->create(['team_id' => $team->id]);
+    $full = makeAvailableBuildServer($team);
+    $free = makeAvailableBuildServer($team);
+    ApplicationDeploymentQueue::create([
+        'application_id' => 1,
+        'deployment_uuid' => 'running-on-full-build-server',
+        'status' => 'in_progress',
+        'server_id' => $deploymentServer->id,
+        'build_server_id' => $full->id,
+    ]);
+    [$job, $deploymentQueue] = makeBuildServerSelectionJob($team, $deploymentServer);
+
+    $deploymentQueue->shouldReceive('update')->with(['build_server_id' => $free->id])->once()->andReturnTrue();
+    $deploymentQueue->shouldReceive('addLogEntry')->once();
+
+    invokeBuildServerSelection($job);
+
+    expect(selectedBuildServer($job)->is($free))->toBeTrue();
+});