diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 057b6ec158..fb74d1ded1 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/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})."); diff --git a/app/Models/Application.php b/app/Models/Application.php index 3b7de984ac..6d6cacf5fd 100644 --- a/app/Models/Application.php +++ b/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 + */ + 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-`), so one application can run the image of the other. diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index 908273043f..39f9702ff6 100644 --- a/bootstrap/helpers/applications.php +++ b/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 $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. diff --git a/tests/Feature/BuildServerConcurrentBuildsTest.php b/tests/Feature/BuildServerConcurrentBuildsTest.php new file mode 100644 index 0000000000..8428fb00f9 --- /dev/null +++ b/tests/Feature/BuildServerConcurrentBuildsTest.php @@ -0,0 +1,149 @@ + 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); +}); diff --git a/tests/Feature/BuildServerFallbackPolicyTest.php b/tests/Feature/BuildServerFallbackPolicyTest.php index f7a366e1c4..b9fdcd5717 100644 --- a/tests/Feature/BuildServerFallbackPolicyTest.php +++ b/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(); +});