diff --git a/app/Console/Commands/Init.php b/app/Console/Commands/Init.php index 01f372b90f..2177d0747a 100644 --- a/app/Console/Commands/Init.php +++ b/app/Console/Commands/Init.php @@ -6,6 +6,7 @@ use App\Enums\ActivityTypes; use App\Enums\ApplicationDeploymentStatus; use App\Jobs\CheckHelperImageJob; use App\Jobs\PullChangelog; +use App\Jobs\SyncGithubDeploymentJob; use App\Models\ApplicationDeploymentQueue; use App\Models\Environment; use App\Models\InstanceSettings; @@ -106,16 +107,24 @@ class Init extends Command echo "Continuing with initialization - cleanup errors will not prevent Coolify from starting\n"; } try { - $updatedCount = ApplicationDeploymentQueue::whereIn('status', [ + $stuckDeployments = ApplicationDeploymentQueue::whereIn('status', [ ApplicationDeploymentStatus::IN_PROGRESS->value, ApplicationDeploymentStatus::QUEUED->value, - ])->update([ + ])->get(['id', 'application_id', 'parent_deployment_uuid']); + $updatedCount = ApplicationDeploymentQueue::whereKey($stuckDeployments->modelKeys())->update([ 'status' => ApplicationDeploymentStatus::FAILED->value, ]); if ($updatedCount > 0) { echo "Marked {$updatedCount} stuck deployments as failed\n"; } + + // The query update above fires no model event, so GitHub would show them in progress forever. + try { + $stuckDeployments->each(fn (ApplicationDeploymentQueue $deployment) => SyncGithubDeploymentJob::dispatchFor($deployment)); + } catch (\Throwable $e) { + echo "Could not queue the GitHub deployment sync of stuck deployments: {$e->getMessage()}\n"; + } } catch (\Throwable $e) { echo "Could not cleanup inprogress deployments: {$e->getMessage()}\n"; } diff --git a/app/Jobs/SyncGithubDeploymentJob.php b/app/Jobs/SyncGithubDeploymentJob.php index 2a3cc9702b..aa2afa0631 100644 --- a/app/Jobs/SyncGithubDeploymentJob.php +++ b/app/Jobs/SyncGithubDeploymentJob.php @@ -33,13 +33,25 @@ class SyncGithubDeploymentJob implements ShouldBeEncrypted, ShouldQueue public $backoff = 10; + /** + * Seconds that one request waits for GitHub. A slow GitHub must not hold a queue worker. + */ + public const HTTP_TIMEOUT_SECONDS = 10; + + /** + * Longer than the worst case of all requests with their retries, shorter than the lock + * expiry, so the per-deployment lock is held while the job runs. + */ + public $timeout = 100; + private const LOCK_EXPIRES_AFTER = 120; private const LOCK_RELEASE_DELAY = 5; public function __construct(public int $deploymentQueueId, public bool $deactivate = false) { - $this->onQueue('high'); + // Not the deployment queue: a slow GitHub must not delay deployments. + $this->onQueue('default'); } /** @@ -136,7 +148,7 @@ class SyncGithubDeploymentJob implements ShouldBeEncrypted, ShouldQueue 'description' => $this->description($state), 'log_url' => $this->logUrl($deployment, $application), 'environment_url' => $state === 'success' ? $this->environmentUrl($deployment, $application) : null, - ])); + ]), timeout: self::HTTP_TIMEOUT_SECONDS); ApplicationDeploymentQueue::whereKey($deployment->id)->update(['github_deployment_state' => $state]); } @@ -174,7 +186,7 @@ class SyncGithubDeploymentJob implements ShouldBeEncrypted, ShouldQueue 'required_contexts' => [], 'transient_environment' => $isPreview, 'production_environment' => ! $isPreview, - ]); + ], timeout: self::HTTP_TIMEOUT_SECONDS); return (int) $response['data']->get('id'); } @@ -190,7 +202,7 @@ class SyncGithubDeploymentJob implements ShouldBeEncrypted, ShouldQueue $commit = filled($deployment->commit) && $deployment->commit !== 'HEAD' ? $deployment->commit : null; if ($deployment->pull_request_id !== 0) { - $pullRequest = githubApi($source, "repos/{$repository}/pulls/{$deployment->pull_request_id}")['data']; + $pullRequest = githubApi($source, "repos/{$repository}/pulls/{$deployment->pull_request_id}", timeout: self::HTTP_TIMEOUT_SECONDS)['data']; $headRepository = (string) data_get($pullRequest, 'head.repo.full_name', ''); $baseRepository = (string) data_get($pullRequest, 'base.repo.full_name', $repository); $headBranch = data_get($pullRequest, 'head.ref'); diff --git a/app/Models/GithubApp.php b/app/Models/GithubApp.php index 8735818465..41ffb79eb6 100644 --- a/app/Models/GithubApp.php +++ b/app/Models/GithubApp.php @@ -67,7 +67,7 @@ class GithubApp extends BaseModel */ private ?array $installationTokenInMemory = null; - public function installationToken(): string + public function installationToken(?int $timeout = null): string { $credentials = implode('|', [$this->api_url, $this->app_id, $this->installation_id, $this->private_key_id]); $cached = $this->installationTokenInMemory; @@ -76,7 +76,7 @@ class GithubApp extends BaseModel return $cached['token']; } - $token = generateGithubToken($this, 'installation'); + $token = generateGithubToken($this, 'installation', $timeout); // GitHub installation tokens expire after one hour; renew them well before that. $this->installationTokenInMemory = [ diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index 9dc05b26c6..54d7e06171 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -5,6 +5,7 @@ use App\Enums\ApplicationDeploymentStatus; use App\Enums\CancelPreviousDeployments; use App\Jobs\ApplicationDeploymentJob; use App\Jobs\CancelSupersededDeploymentJob; +use App\Jobs\SyncGithubDeploymentJob; use App\Jobs\VolumeCloneJob; use App\Models\Application; use App\Models\ApplicationDeploymentQueue; @@ -126,6 +127,12 @@ function queue_application_deployment(Application $application, string $deployme foreach ($admission['superseded'] as $kind => $cancelled) { foreach ($cancelled as $old) { $old->addLogEntry("Deployment cancelled: the newer deployment {$deployment_uuid} (commit {$commit}) replaces it.", 'stderr'); + // The cancel is a query update, which fires no model event. + try { + SyncGithubDeploymentJob::dispatchFor($old); + } catch (Throwable $e) { + Log::warning("Failed to queue the GitHub deployment sync of superseded deployment {$old->deployment_uuid}: {$e->getMessage()}"); + } if ($kind !== 'building') { continue; } @@ -364,6 +371,12 @@ function fail_undispatchable_deployment(ApplicationDeploymentQueue $deployment, Log::warning("Deployment {$deployment->deployment_uuid} could not be started: {$exception->getMessage()}"); $deployment->refresh(); + // The query update above fires no model event. + try { + SyncGithubDeploymentJob::dispatchFor($deployment); + } catch (Throwable $e) { + Log::warning("Failed to queue the GitHub deployment sync of deployment {$deployment->deployment_uuid}: {$e->getMessage()}"); + } $deployment->addLogEntry('========================================', 'stderr'); $deployment->addLogEntry("Deployment could not be started: {$exception->getMessage()}", 'stderr'); $deployment->addLogEntry('========================================', 'stderr'); diff --git a/bootstrap/helpers/github.php b/bootstrap/helpers/github.php index ddfff8c295..ce2931be1d 100644 --- a/bootstrap/helpers/github.php +++ b/bootstrap/helpers/github.php @@ -5,6 +5,7 @@ use App\Models\PrivateKey; use Carbon\Carbon; use Carbon\CarbonImmutable; use Illuminate\Http\Client\ConnectionException; +use Illuminate\Http\Client\PendingRequest; use Illuminate\Http\Client\RequestException; use Illuminate\Http\Client\Response; use Illuminate\Support\Facades\Cache; @@ -199,7 +200,10 @@ function githubAppPrivateKey(GithubApp $source): ?PrivateKey return $privateKey?->team_id === $source->team_id ? $privateKey : null; } -function generateGithubToken(GithubApp $source, string $type) +/** + * @param int|null $timeout Seconds for each request to GitHub. Null keeps the HTTP client default. + */ +function generateGithubToken(GithubApp $source, string $type, ?int $timeout = null) { $privateKey = githubAppPrivateKey($source) ?? throw new RuntimeException('Private key not found for this GitHub App.'); @@ -218,8 +222,8 @@ function generateGithubToken(GithubApp $source, string $type) return match ($type) { 'jwt' => $jwt, - 'installation' => (function () use ($source, $jwt) { - $response = Http::GitSource($source->api_url)->withHeaders([ + 'installation' => (function () use ($source, $jwt, $timeout) { + $response = githubRequestTimeout(Http::GitSource($source->api_url), $timeout)->withHeaders([ 'Authorization' => "Bearer $jwt", 'Accept' => 'application/vnd.github.machine-man-preview+json', ])->retry(3, fn (int $attempt) => $attempt * 1000, isTransientGithubFailure(...), throw: false)->post("{$source->api_url}/app/installations/{$source->installation_id}/access_tokens"); @@ -239,9 +243,17 @@ function generateGithubToken(GithubApp $source, string $type) }; } -function generateGithubInstallationToken(GithubApp $source) +function generateGithubInstallationToken(GithubApp $source, ?int $timeout = null) { - return $source->installationToken(); + return $source->installationToken($timeout); +} + +/** + * Limits how long a request waits for GitHub, both to connect and for the whole answer. + */ +function githubRequestTimeout(PendingRequest $request, ?int $timeout): PendingRequest +{ + return $timeout === null ? $request : $request->connectTimeout($timeout)->timeout($timeout); } function generateGithubJwt(GithubApp $source) @@ -249,7 +261,7 @@ function generateGithubJwt(GithubApp $source) return generateGithubToken($source, 'jwt'); } -function githubApi(?GithubApp $source, string $endpoint, string $method = 'get', ?array $data = null, bool $throwError = true) +function githubApi(?GithubApp $source, string $endpoint, string $method = 'get', ?array $data = null, bool $throwError = true, ?int $timeout = null) { if (is_null($source)) { throw new Exception('Source is required for API calls'); @@ -259,9 +271,9 @@ function githubApi(?GithubApp $source, string $endpoint, string $method = 'get', throw new InvalidArgumentException("Unsupported source type: {$source->getMorphClass()}"); } - $request = $source->is_public + $request = githubRequestTimeout($source->is_public ? Http::GitHub($source->api_url) - : Http::GitHub($source->api_url, generateGithubInstallationToken($source)); + : Http::GitHub($source->api_url, generateGithubInstallationToken($source, $timeout)), $timeout); // Only reads are retried: a repeated POST could create a second pull request comment. if (strtolower($method) === 'get') { diff --git a/tests/Feature/Jobs/StartupExecutionCleanupTest.php b/tests/Feature/Jobs/StartupExecutionCleanupTest.php index 0b5cc4af2b..c7ae6d1d3d 100644 --- a/tests/Feature/Jobs/StartupExecutionCleanupTest.php +++ b/tests/Feature/Jobs/StartupExecutionCleanupTest.php @@ -1,7 +1,13 @@ create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $githubApp = GithubApp::create([ + 'name' => 'Private GitHub App', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'app_id' => 12345, + 'installation_id' => 67890, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + 'is_public' => false, + 'deployments' => 'write', + 'team_id' => $team->id, + ]); + $application = Application::factory()->create([ + 'environment_id' => Environment::factory()->create(['project_id' => $project->id])->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'source_type' => GithubApp::class, + 'source_id' => $githubApp->id, + ]); + $deploy = fn (ApplicationDeploymentStatus $status): ApplicationDeploymentQueue => ApplicationDeploymentQueue::create([ + 'application_id' => $application->id, + 'deployment_uuid' => (string) Str::uuid(), + 'pull_request_id' => 0, + 'commit' => 'HEAD', + 'status' => $status->value, + 'server_id' => $server->id, + ]); + $inProgress = $deploy(ApplicationDeploymentStatus::IN_PROGRESS); + $queued = $deploy(ApplicationDeploymentStatus::QUEUED); + $finished = $deploy(ApplicationDeploymentStatus::FINISHED); + + Artisan::call('app:init'); + + expect($inProgress->fresh()->status)->toBe(ApplicationDeploymentStatus::FAILED->value) + ->and($queued->fresh()->status)->toBe(ApplicationDeploymentStatus::FAILED->value) + ->and($finished->fresh()->status)->toBe(ApplicationDeploymentStatus::FINISHED->value); + Queue::assertPushed(SyncGithubDeploymentJob::class, 2); + foreach ([$inProgress, $queued] as $failed) { + Queue::assertPushed(SyncGithubDeploymentJob::class, fn (SyncGithubDeploymentJob $job): bool => $job->deploymentQueueId === $failed->id); + } +}); diff --git a/tests/Feature/SyncGithubDeploymentJobTest.php b/tests/Feature/SyncGithubDeploymentJobTest.php index 37638c0194..ab8d47d1f8 100644 --- a/tests/Feature/SyncGithubDeploymentJobTest.php +++ b/tests/Feature/SyncGithubDeploymentJobTest.php @@ -19,6 +19,7 @@ use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Http\Client\Request; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Notification; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Str; use Livewire\Livewire; @@ -115,6 +116,33 @@ it('queues a sync when the status of a deployment changes', function () { Queue::assertPushed(SyncGithubDeploymentJob::class, fn (SyncGithubDeploymentJob $job): bool => $job->deploymentQueueId === $deployment->id); }); +it('queues the sync on the default queue, not on the deployment queue', function () { + $deployment = githubDeployment($this->application, ['status' => ApplicationDeploymentStatus::QUEUED->value]); + + $deployment->update(['status' => ApplicationDeploymentStatus::IN_PROGRESS->value]); + + Queue::assertPushedOn('default', SyncGithubDeploymentJob::class); + expect(deployment_queue())->not->toBe('default'); +}); + +it('gives up on a slow GitHub request after a short timeout', function () { + $timeouts = []; + Http::fake(function (Request $request, array $options) use (&$timeouts) { + $timeouts[$request->url()] = [$options['timeout'] ?? null, $options['connect_timeout'] ?? null]; + + return null; + }); + + syncGithubDeployment(githubDeployment($this->application)); + + expect($timeouts)->not->toBeEmpty(); + foreach ($timeouts as $url => [$timeout, $connectTimeout]) { + expect($timeout)->toBeLessThanOrEqual(SyncGithubDeploymentJob::HTTP_TIMEOUT_SECONDS, $url) + ->and($connectTimeout)->toBeLessThanOrEqual(SyncGithubDeploymentJob::HTTP_TIMEOUT_SECONDS, $url); + } + expect((new SyncGithubDeploymentJob(1))->timeout)->toBeLessThan(120); +}); + it('does not queue a sync when the GitHub App has no deployments permission', function () { $this->githubApp->update(['deployments' => null]); $deployment = githubDeployment($this->application, ['status' => ApplicationDeploymentStatus::QUEUED->value]); @@ -135,6 +163,31 @@ it('does not queue a sync for the deployment to an additional server', function Queue::assertNotPushed(SyncGithubDeploymentJob::class); }); +it('queues a sync for a deployment that fails because its job cannot be queued', function () { + Notification::fake(); + $deployment = githubDeployment($this->application); + + fail_undispatchable_deployment($deployment, new RuntimeException('queue connection refused')); + + expect($deployment->fresh()->status)->toBe(ApplicationDeploymentStatus::FAILED->value); + Queue::assertPushed(SyncGithubDeploymentJob::class, fn (SyncGithubDeploymentJob $job): bool => $job->deploymentQueueId === $deployment->id); +}); + +it('queues a sync for the deployments that a newer webhook deployment cancels', function () { + Queue::fake(); + $this->application->settings->update(['is_auto_cancel_building_deployments_enabled' => true]); + $building = githubDeployment($this->application, ['is_webhook' => true]); + $queued = githubDeployment($this->application, ['is_webhook' => true, 'status' => ApplicationDeploymentStatus::QUEUED->value]); + + queue_application_deployment($this->application, 'newer-webhook-deployment', commit: 'ddd444', is_webhook: true); + + expect($building->fresh()->status)->toBe(ApplicationDeploymentStatus::CANCELLED_BY_USER->value) + ->and($queued->fresh()->status)->toBe(ApplicationDeploymentStatus::CANCELLED_BY_USER->value); + foreach ([$building, $queued] as $cancelled) { + Queue::assertPushed(SyncGithubDeploymentJob::class, fn (SyncGithubDeploymentJob $job): bool => $job->deploymentQueueId === $cancelled->id); + } +}); + it('creates one GitHub deployment for the branch and sends each status', function () { $deployment = githubDeployment($this->application);