From c2d82b542d85ec94704ee3ce58a2b43524782700 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:20:10 +0200 Subject: [PATCH] fix(github-runners): do not reconcile servers with disabled runners ReconcileGithubRunnersJob connected over SSH every minute to every server with a runner configuration, also when the runners were disabled. - Reconcile a server only when its runners are enabled or it still has active runner executions, so jobs that run after disabling still end --- app/Jobs/ReconcileGithubRunnersJob.php | 6 +- ...concileGithubRunnersDisabledConfigTest.php | 116 ++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php diff --git a/app/Jobs/ReconcileGithubRunnersJob.php b/app/Jobs/ReconcileGithubRunnersJob.php index 61566e9e6e..073b25e57e 100644 --- a/app/Jobs/ReconcileGithubRunnersJob.php +++ b/app/Jobs/ReconcileGithubRunnersJob.php @@ -10,6 +10,7 @@ use App\Services\GithubRunner\GithubRunnerContainer; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeUnique; use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; @@ -42,8 +43,11 @@ class ReconcileGithubRunnersJob implements ShouldBeUnique, ShouldQueue public function handle(): void { + // Disabling keeps running jobs, so a disabled server is reconciled until its last runner is done. Server::query() - ->whereHas('githubRunnerConfig') + ->where(fn (Builder $query) => $query + ->whereHas('githubRunnerConfig', fn (Builder $config) => $config->where('is_enabled', true)) + ->orWhereHas('githubRunnerExecutions', fn (Builder $execution) => $execution->whereIn('status', GithubRunnerStatus::occupying()))) ->with('settings') ->get() ->filter(fn (Server $server) => $server->isFunctional()) diff --git a/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php b/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php new file mode 100644 index 0000000000..48cc9a95ce --- /dev/null +++ b/tests/Feature/ReconcileGithubRunnersDisabledConfigTest.php @@ -0,0 +1,116 @@ + 0]); + Server::flushIdentityMap(); + Queue::fake(); + Process::fake(['*' => Process::result(output: '')]); + + $this->team = Team::factory()->create(); + $rsaKey = openssl_pkey_new(['private_key_bits' => 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA]); + openssl_pkey_export($rsaKey, $pemKey); + $privateKey = PrivateKey::create(['name' => 'Runner key', 'private_key' => $pemKey, 'team_id' => $this->team->id]); + $this->githubApp = GithubApp::create([ + 'name' => 'reconcile-disabled-app', + 'organization' => 'acme', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'custom_user' => 'git', + 'custom_port' => 22, + 'app_id' => 2222, + 'installation_id' => 2223, + 'webhook_secret' => 'secret', + 'private_key_id' => $privateKey->id, + 'team_id' => $this->team->id, + 'is_system_wide' => false, + 'runner_group_id' => 7, + ]); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +function reconcileDisabledTestServer(Team $team, GithubApp $githubApp, bool $enabled): 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, + 'force_disabled' => false, + ]); + GithubRunnerConfig::create([ + 'server_id' => $server->id, + 'github_app_id' => $githubApp->id, + 'labels' => ['coolify'], + 'max_runners' => 2, + 'docker_mode' => 'dind', + 'is_enabled' => $enabled, + ]); + + return $server->fresh(); +} + +function reconcileDisabledTestCommandFor(Server $server): Closure +{ + return fn (PendingProcess $process) => str_contains($process->command, $server->ip); +} + +it('reconciles a server with enabled runners', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: true); + + (new ReconcileGithubRunnersJob)->handle(); + + Process::assertRan(reconcileDisabledTestCommandFor($server)); +}); + +it('does not connect to a server whose runners are disabled and have no active runners left', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: false); + GithubRunnerExecution::create([ + 'github_app_id' => $this->githubApp->id, + 'server_id' => $server->id, + 'trigger_workflow_job_id' => 3001, + 'labels' => ['self-hosted', 'coolify'], + 'status' => GithubRunnerStatus::Completed, + ]); + + (new ReconcileGithubRunnersJob)->handle(); + + Process::assertDidntRun(reconcileDisabledTestCommandFor($server)); +}); + +it('keeps reconciling a disabled server until its running jobs finish', function () { + $server = reconcileDisabledTestServer($this->team, $this->githubApp, enabled: false); + $running = GithubRunnerExecution::create([ + 'github_app_id' => $this->githubApp->id, + 'server_id' => $server->id, + 'github_runner_config_id' => $server->githubRunnerConfig->id, + 'trigger_workflow_job_id' => 3002, + 'labels' => ['self-hosted', 'coolify'], + 'status' => GithubRunnerStatus::Running, + 'started_at' => now(), + ]); + + (new ReconcileGithubRunnersJob)->handle(); + + Process::assertRan(reconcileDisabledTestCommandFor($server)); + expect($running->fresh()->status)->toBe(GithubRunnerStatus::Failed); +});