From dbc3fb91bfbb8db719158bf1c6c98ef31afc788f Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:10:48 +0200 Subject: [PATCH] fix(cdn): fetch versions.json once per hour, spread evenly across instances - Retry CDN requests only for transient errors and honor Retry-After (Cloudflare error 1200 sends 60 s), with jitter. - Read helper and Sentinel versions from the local versions.json, which the hourly update check refreshes, instead of fetching per job/server. - Replace the random 0-600 s delay with a stable per-instance offset inside the cron interval (max 1 h). - Manual update checks and upgrades make one CDN attempt only. Co-Authored-By: Claude Opus 5.5 --- app/Actions/Server/UpdateCoolify.php | 4 +- app/Console/Kernel.php | 46 +++++++++------- app/Jobs/CheckForUpdatesJob.php | 10 +++- app/Jobs/CheckHelperImageJob.php | 21 ++------ app/Livewire/Settings/Updates.php | 2 +- bootstrap/helpers/shared.php | 10 +--- bootstrap/helpers/versions.php | 51 +++++++++++++++++- tests/Feature/HelperVersionTest.php | 32 +++++++++--- tests/Feature/ScheduleOnOneServerTest.php | 32 ++++++++++-- .../Server/CheckAndStartSentinelJobTest.php | 5 +- tests/Unit/CdnHttpRetryTest.php | 52 +++++++++++++++++++ tests/Unit/CheckForUpdatesJobTest.php | 11 ++++ 12 files changed, 214 insertions(+), 62 deletions(-) create mode 100644 tests/Unit/CdnHttpRetryTest.php diff --git a/app/Actions/Server/UpdateCoolify.php b/app/Actions/Server/UpdateCoolify.php index ddfc6578e4..f0701977b1 100644 --- a/app/Actions/Server/UpdateCoolify.php +++ b/app/Actions/Server/UpdateCoolify.php @@ -6,7 +6,6 @@ use App\Jobs\DatabaseBackupJob; use App\Models\Server; use App\Models\StandalonePostgresql; use App\Services\CoolifyUpgradeStatus; -use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Sleep; use Lorisleiva\Actions\Concerns\AsAction; @@ -37,7 +36,8 @@ class UpdateCoolify // Fetch fresh version from CDN instead of using cache try { - $response = Http::retry(3, 1000)->timeout(10) + // A manual update runs in the web request, so it does not wait for a retry; it falls back to the cache. + $response = cdn_http(tries: $manual_update ? 1 : 3) ->get(config('constants.coolify.versions_url')); if ($response->successful()) { diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 0dc3b5f35b..cd63d3502f 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -20,12 +20,13 @@ use App\Jobs\ServerManagerJob; use App\Jobs\UpdateCoolifyJob; use App\Models\InstanceSettings; use App\Services\ScheduledJobDeliveryService; +use Cron\CronExpression; use Illuminate\Console\Scheduling\Schedule; use Illuminate\Foundation\Console\Kernel as ConsoleKernel; class Kernel extends ConsoleKernel { - private const CDN_JITTER_SECONDS = 600; + private const MAX_CDN_DELAY_SECONDS = 3600; private Schedule $scheduleInstance; @@ -98,8 +99,8 @@ class Kernel extends ConsoleKernel $this->scheduleInstance->command('horizon:snapshot')->everyFiveMinutes(); $this->scheduleInstance->command('cleanup:unreachable-servers')->daily()->onOneServer(); - $this->scheduleInstance->job($this->withCdnJitter(new PullTemplatesFromCDN))->cron($this->updateCheckFrequency)->timezone($this->instanceTimezone)->onOneServer(); - $this->scheduleInstance->job($this->withCdnJitter(new PullChangelog))->cron($this->updateCheckFrequency)->timezone($this->instanceTimezone)->onOneServer(); + $this->scheduleInstance->job($this->withCdnDelay(new PullTemplatesFromCDN, $this->updateCheckFrequency))->cron($this->updateCheckFrequency)->timezone($this->instanceTimezone)->onOneServer(); + $this->scheduleInstance->job($this->withCdnDelay(new PullChangelog, $this->updateCheckFrequency))->cron($this->updateCheckFrequency)->timezone($this->instanceTimezone)->onOneServer(); $this->scheduleInstance->job(new CleanupInstanceStuffsJob)->everyTwoMinutes()->onOneServer(); $this->scheduleUpdates(); @@ -107,8 +108,6 @@ class Kernel extends ConsoleKernel // Server Jobs $this->scheduleInstance->job(new ServerManagerJob)->everyMinute()->onOneServer(); - $this->pullImages(); - // Scheduled Jobs (Backups & Tasks) $this->scheduleScheduledJobManager(); @@ -122,14 +121,6 @@ class Kernel extends ConsoleKernel } } - private function pullImages(): void - { - $this->scheduleInstance->job($this->withCdnJitter(new CheckHelperImageJob)) - ->cron($this->updateCheckFrequency) - ->timezone($this->instanceTimezone) - ->onOneServer(); - } - /** * Run the manager from the scheduler, not from a queue worker. A busy queue could delay it * past the catch-up window, and then due backups and tasks would be skipped. @@ -169,14 +160,14 @@ class Kernel extends ConsoleKernel private function scheduleUpdates(): void { - $this->scheduleInstance->job($this->withCdnJitter(new CheckForUpdatesJob)) + $this->scheduleInstance->job($this->withCdnDelay(new CheckForUpdatesJob, $this->updateCheckFrequency)) ->cron($this->updateCheckFrequency) ->timezone($this->instanceTimezone) ->onOneServer(); if ($this->settings->is_auto_update_enabled) { $autoUpdateFrequency = $this->settings->auto_update_frequency; - $this->scheduleInstance->job($this->withCdnJitter(new UpdateCoolifyJob)) + $this->scheduleInstance->job($this->withCdnDelay(new UpdateCoolifyJob, $autoUpdateFrequency)) ->cron($autoUpdateFrequency) ->timezone($this->instanceTimezone) ->onOneServer(); @@ -184,12 +175,29 @@ class Kernel extends ConsoleKernel } /** - * All instances share the same default cron, so without a random delay they all request the CDN - * at the same second and Cloudflare rejects the burst (error 1200). + * All instances share the same default cron, so without a delay they all request the CDN at the + * same second and Cloudflare rejects the burst (error 1200). Each instance gets a stable offset + * inside the cron interval (at most one hour), so the requests spread evenly and each instance + * still runs at a regular interval. */ - private function withCdnJitter(object $job): object + private function withCdnDelay(object $job, string $cron): object { - return $job->delay(random_int(0, self::CDN_JITTER_SECONDS)); + return $job->delay(self::cdnDelaySeconds($cron)); + } + + public static function cdnDelaySeconds(string $cron): int + { + try { + $expression = new CronExpression($cron); + $nextRun = $expression->getNextRunDate(); + $interval = $expression->getNextRunDate($nextRun)->getTimestamp() - $nextRun->getTimestamp(); + } catch (\Throwable) { + $interval = self::MAX_CDN_DELAY_SECONDS; + } + $window = max(1, min($interval, self::MAX_CDN_DELAY_SECONDS)); + $instanceId = (string) (config('app.id') ?: config('app.key')); + + return crc32('cdn-delay:'.$instanceId) % $window; } protected function commands(): void diff --git a/app/Jobs/CheckForUpdatesJob.php b/app/Jobs/CheckForUpdatesJob.php index 01a0e69166..2c16b4475e 100644 --- a/app/Jobs/CheckForUpdatesJob.php +++ b/app/Jobs/CheckForUpdatesJob.php @@ -13,13 +13,21 @@ class CheckForUpdatesJob implements ShouldBeEncrypted, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + /** A manual check runs in the web request, so it does not wait for CDN retries. */ + public int $cdnTries = 3; + + public function __construct(int $cdnTries = 3) + { + $this->cdnTries = $cdnTries; + } + public function handle(): void { try { if (isDev() || isCloud()) { return; } - $versions = refresh_versions_file(); + $versions = refresh_versions_file($this->cdnTries); if ($versions === null) { return; } diff --git a/app/Jobs/CheckHelperImageJob.php b/app/Jobs/CheckHelperImageJob.php index 56118a238a..8b8dd16f06 100644 --- a/app/Jobs/CheckHelperImageJob.php +++ b/app/Jobs/CheckHelperImageJob.php @@ -8,31 +8,18 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; -use Illuminate\Support\Facades\Http; -use Illuminate\Support\Facades\Log; class CheckHelperImageJob implements ShouldBeEncrypted, ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; - public $timeout = 1000; - public function __construct() {} + /** + * Reads the local versions.json. CheckForUpdatesJob refreshes it from the CDN every hour. + */ public function handle(): void { - $response = Http::retry(3, 1000, throw: false)->get(config('constants.coolify.versions_url')); - if ($response->failed()) { - // The CDN can be briefly unavailable. The next scheduled run tries again, so do not notify. - Log::warning('CheckHelperImageJob could not fetch versions', ['status' => $response->status()]); - - return; - } - - $latest_version = data_get($response->json(), 'coolify.helper.version'); - $settings = instanceSettings(); - if (version_compare($latest_version, $settings->helper_version, '>')) { - $settings->update(['helper_version' => $latest_version]); - } + update_helper_version(get_versions_data()); } } diff --git a/app/Livewire/Settings/Updates.php b/app/Livewire/Settings/Updates.php index aba7521dd9..27fe705423 100644 --- a/app/Livewire/Settings/Updates.php +++ b/app/Livewire/Settings/Updates.php @@ -159,7 +159,7 @@ class Updates extends Component public function checkManually() { $this->authorize('update', $this->settings); - CheckForUpdatesJob::dispatchSync(); + CheckForUpdatesJob::dispatchSync(cdnTries: 1); $this->dispatch('updateAvailable'); $settings = instanceSettings(); if ($settings->new_version_available) { diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 0ba73757eb..48aa00a907 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -703,14 +703,8 @@ function get_route_parameters(): array function get_latest_sentinel_version(): string { - try { - $response = Http::get(config('constants.coolify.versions_url')); - $versions = $response->json(); - - return data_get($versions, 'coolify.sentinel.version'); - } catch (Throwable) { - return '0.0.0'; - } + // Read the local versions.json, which CheckForUpdatesJob refreshes every hour. Do not ask the CDN for each server. + return data_get(get_versions_data(), 'coolify.sentinel.version') ?? '0.0.0'; } function get_latest_version_of_coolify(): string { diff --git a/bootstrap/helpers/versions.php b/bootstrap/helpers/versions.php index d6a714cc9e..ef6e34f295 100644 --- a/bootstrap/helpers/versions.php +++ b/bootstrap/helpers/versions.php @@ -1,5 +1,8 @@ connectTimeout(10) + ->retry( + $tries, + fn (int $attempt, Throwable $exception) => cdn_retry_delay_ms($attempt, $exception), + fn (Throwable $exception) => $exception instanceof ConnectionException + || ($exception instanceof RequestException + && ($exception->response->status() === 429 || $exception->response->serverError())), + throw: false, + ); +} + +/** + * Wait as long as the Retry-After header says (Cloudflare sends 60 seconds with error 1200), + * or 2^attempt seconds without it. A random part of up to the same length is added, so instances + * that failed in the same burst do not retry in the same second. + */ +function cdn_retry_delay_ms(int $attempt, Throwable $exception): int +{ + $retryAfter = $exception instanceof RequestException ? (int) $exception->response->header('Retry-After') : 0; + $seconds = $retryAfter > 0 ? min($retryAfter, 60) : 2 ** $attempt; + + return ($seconds + random_int(0, $seconds)) * 1000; +} + /** * Fetch versions.json from the CDN and write it to the local file of this node. * The Coolify version never goes below the cached or the running version. @@ -61,9 +95,9 @@ function invalidate_versions_cache(): void * * @return array|null The written versions data, or null if the CDN fetch failed */ -function refresh_versions_file(): ?array +function refresh_versions_file(int $tries = 3): ?array { - $response = Http::timeout(10)->retry(3, 1000, throw: false)->get(config('constants.coolify.versions_url')); + $response = cdn_http($tries)->get(config('constants.coolify.versions_url')); $versions = $response->json(); if ($response->failed() || ! is_array($versions)) { Log::warning('Could not fetch versions.json from the CDN, using the local file', ['status' => $response->status()]); @@ -106,6 +140,19 @@ function refresh_versions_file(): ?array File::put(base_path('versions.json'), json_encode($versions, JSON_PRETTY_PRINT)); invalidate_versions_cache(); + update_helper_version($versions); return $versions; } + +/** + * Store the helper image version of the versions data when it is newer than the stored one. + */ +function update_helper_version(?array $versions): void +{ + $latestVersion = data_get($versions, 'coolify.helper.version'); + $settings = instanceSettings(); + if ($latestVersion && version_compare($latestVersion, (string) $settings->helper_version, '>')) { + $settings->update(['helper_version' => $latestVersion]); + } +} diff --git a/tests/Feature/HelperVersionTest.php b/tests/Feature/HelperVersionTest.php index 6c203887fb..908b3ba484 100644 --- a/tests/Feature/HelperVersionTest.php +++ b/tests/Feature/HelperVersionTest.php @@ -4,8 +4,11 @@ use App\Jobs\CheckHelperImageJob; use App\Models\InstanceSettings; use App\Models\Team; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Notification; +use Illuminate\Support\Sleep; uses(RefreshDatabase::class); @@ -19,13 +22,9 @@ beforeEach(function () { ]); }); -it('uses a newer helper version after fetching it from the CDN', function () { +it('uses a newer helper version from the local versions file without asking the CDN', function () { Http::preventStrayRequests(); - Http::fake([ - 'https://cdn.example.com/coolify/versions.json' => Http::response([ - 'coolify' => ['helper' => ['version' => '1.0.18']], - ]), - ]); + Cache::put('coolify:versions:all', ['coolify' => ['helper' => ['version' => '1.0.18']]], 3600); (new CheckHelperImageJob)->handle(); @@ -33,6 +32,21 @@ it('uses a newer helper version after fetching it from the CDN', function () { ->and(getHelperVersion())->toBe('1.0.18'); }); +it('stores the helper version when the update check refreshes versions.json from the CDN', function () { + File::partialMock()->shouldReceive('put')->once()->with(base_path('versions.json'), Mockery::type('string')); + config(['constants.coolify.version' => '4.0.0']); + Http::preventStrayRequests(); + Http::fake([ + 'https://cdn.example.com/coolify/versions.json' => Http::response([ + 'coolify' => ['v4' => ['version' => '4.0.0'], 'helper' => ['version' => '1.0.19']], + ]), + ]); + + refresh_versions_file(); + + expect(InstanceSettings::findOrFail(0)->helper_version)->toBe('1.0.19'); +}); + it('does not use a stored helper version older than the bundled version', function () { InstanceSettings::findOrFail(0)->update(['helper_version' => '1.0.16']); @@ -43,12 +57,14 @@ it('keeps the stored helper version and does not notify when the CDN is unavaila Team::factory()->create(['id' => 0])->discordNotificationSettings()->update(['discord_enabled' => true]); InstanceSettings::findOrFail(0)->update(['helper_version' => '1.0.17']); Notification::fake(); + Sleep::fake(); + File::partialMock()->shouldNotReceive('put'); Http::preventStrayRequests(); Http::fake([ - 'https://cdn.example.com/coolify/versions.json' => Http::response('error code: 1200', 503), + 'https://cdn.example.com/coolify/versions.json' => Http::response('error code: 1200', 503, ['Retry-After' => '60']), ]); - (new CheckHelperImageJob)->handle(); + expect(refresh_versions_file())->toBeNull(); expect(InstanceSettings::findOrFail(0)->helper_version)->toBe('1.0.17'); Notification::assertNothingSent(); diff --git a/tests/Feature/ScheduleOnOneServerTest.php b/tests/Feature/ScheduleOnOneServerTest.php index c195d831ba..f437602c69 100644 --- a/tests/Feature/ScheduleOnOneServerTest.php +++ b/tests/Feature/ScheduleOnOneServerTest.php @@ -1,5 +1,6 @@ whereKey(0)->update(['is_auto_update_enabled' => true]); + config(['app.id' => 'instance-a']); Queue::fake(); - $jobs = [CheckHelperImageJob::class, CheckForUpdatesJob::class, PullTemplatesFromCDN::class, PullChangelog::class, UpdateCoolifyJob::class]; + $jobs = [CheckForUpdatesJob::class, PullTemplatesFromCDN::class, PullChangelog::class, UpdateCoolifyJob::class]; $schedule = app(Schedule::class); + $expectedDelay = ConsoleKernel::cdnDelaySeconds('0 * * * *'); foreach ($jobs as $job) { $event = collect($schedule->events())->first(fn ($e) => $e->description === $job); expect($event)->not->toBeNull("[{$job}] is not scheduled"); $event->run(app()); - Queue::assertPushed($job, fn ($pushed) => is_int($pushed->delay) && $pushed->delay >= 0 && $pushed->delay <= 600); + Queue::assertPushed($job, fn ($pushed) => $pushed->delay === $expectedDelay); } + + expect(collect($schedule->events())->first(fn ($e) => $e->description === CheckHelperImageJob::class))->toBeNull(); +}); + +it('spreads the CDN delay of many instances over the full hour', function () { + $delays = collect(range(1, 2000))->map(function (int $i) { + config(['app.id' => bin2hex(random_bytes(16))]); + + return ConsoleKernel::cdnDelaySeconds('0 * * * *'); + }); + + expect($delays->min())->toBeGreaterThanOrEqual(0) + ->and($delays->max())->toBeLessThan(3600); + // Each 10-minute window of the hour gets about one sixth of the instances. + $delays->groupBy(fn (int $delay) => intdiv($delay, 600)) + ->each(fn (Collection $window) => expect($window->count())->toBeGreaterThan(250)->toBeLessThan(420)); +}); + +it('keeps the CDN delay inside a cron interval shorter than one hour', function () { + config(['app.id' => 'instance-b']); + + expect(ConsoleKernel::cdnDelaySeconds('*/15 * * * *'))->toBeLessThan(900) + ->and(ConsoleKernel::cdnDelaySeconds('0 0 * * *'))->toBeLessThan(3600); }); it('does not schedule Stripe subscription reconciliation automatically', function () { diff --git a/tests/Feature/Server/CheckAndStartSentinelJobTest.php b/tests/Feature/Server/CheckAndStartSentinelJobTest.php index cb541b3519..ac2213e736 100644 --- a/tests/Feature/Server/CheckAndStartSentinelJobTest.php +++ b/tests/Feature/Server/CheckAndStartSentinelJobTest.php @@ -21,7 +21,8 @@ beforeEach(function () { Storage::fake('ssh-mux'); config()->set('constants.ssh.max_retries', 1); config()->set('constants.ssh.mux_enabled', false); - Http::fake(['*' => Http::response(['coolify' => ['sentinel' => ['version' => '1.0.0']]])]); + Http::preventStrayRequests(); + Cache::put('coolify:versions:all', ['coolify' => ['sentinel' => ['version' => '1.0.0']]], 3600); $user = User::factory()->create(); $team = $user->teams()->first(); @@ -109,6 +110,7 @@ it('does not restart a Sentinel whose health check is still starting', function it('restarts a running Sentinel that stopped pushing, at most once per hour', function () { Cache::flush(); + Cache::put('coolify:versions:all', ['coolify' => ['sentinel' => ['version' => '1.0.0']]], 3600); $this->server->forceFill(['sentinel_updated_at' => now()->subMinutes(15), 'sentinel_waiting_since' => null])->save(); Process::fake([ '*docker ps*' => Process::result(output: 'running Up 2 hours (healthy)'), @@ -122,6 +124,7 @@ it('restarts a running Sentinel that stopped pushing, at most once per hour', fu it('does not restart a Sentinel that started recently and has not pushed yet', function () { Cache::flush(); + Cache::put('coolify:versions:all', ['coolify' => ['sentinel' => ['version' => '1.0.0']]], 3600); $this->server->forceFill(['sentinel_updated_at' => now()->subDays(5), 'sentinel_waiting_since' => now()->subMinutes(3)])->save(); Process::fake([ '*docker ps*' => Process::result(output: 'running Up 3 minutes (healthy)'), diff --git a/tests/Unit/CdnHttpRetryTest.php b/tests/Unit/CdnHttpRetryTest.php new file mode 100644 index 0000000000..740461d28c --- /dev/null +++ b/tests/Unit/CdnHttpRetryTest.php @@ -0,0 +1,52 @@ +sleptSeconds = []; + Sleep::whenFakingSleep(fn (CarbonInterval $duration) => $this->sleptSeconds[] = $duration->totalSeconds); +}); + +it('waits for the Retry-After of a Cloudflare 1200 response before it retries', function () { + Http::fakeSequence() + ->push('error code: 1200', 503, ['Retry-After' => '60']) + ->push(['coolify' => ['helper' => ['version' => '1.0.0']]], 200); + + $response = cdn_http()->get('https://cdn.example.com/coolify/versions.json'); + + expect($response->successful())->toBeTrue(); + Http::assertSentCount(2); + expect($this->sleptSeconds)->toHaveCount(1) + ->and($this->sleptSeconds[0])->toBeGreaterThanOrEqual(60)->toBeLessThanOrEqual(120); +}); + +it('backs off exponentially when the response has no Retry-After', function () { + Http::fakeSequence() + ->push('', 502) + ->push('', 502) + ->push('', 502); + + $response = cdn_http()->get('https://cdn.example.com/coolify/versions.json'); + + expect($response->status())->toBe(502); + Http::assertSentCount(3); + expect($this->sleptSeconds)->toHaveCount(2) + ->and($this->sleptSeconds[0])->toBeGreaterThanOrEqual(2)->toBeLessThanOrEqual(4) + ->and($this->sleptSeconds[1])->toBeGreaterThanOrEqual(4)->toBeLessThanOrEqual(8); +}); + +it('does not retry a client error', function () { + Http::fakeSequence()->push('', 404); + + $response = cdn_http()->get('https://cdn.example.com/coolify/versions.json'); + + expect($response->status())->toBe(404); + Http::assertSentCount(1); + Sleep::assertNeverSlept(); +}); diff --git a/tests/Unit/CheckForUpdatesJobTest.php b/tests/Unit/CheckForUpdatesJobTest.php index 6b9706e4c4..45d1f61942 100644 --- a/tests/Unit/CheckForUpdatesJobTest.php +++ b/tests/Unit/CheckForUpdatesJobTest.php @@ -8,6 +8,7 @@ use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; +use Illuminate\Support\Sleep; use Tests\TestCase; uses(TestCase::class, RefreshDatabase::class); @@ -171,3 +172,13 @@ it('keeps the local versions.json when the CDN fetch fails', function () { expect(refresh_versions_file())->toBeNull(); }); + +it('makes only one CDN request for a manual check, so the web request does not wait for retries', function () { + Sleep::fake(); + Http::fake(['*' => Http::response('error code: 1200', 503, ['Retry-After' => '60'])]); + + (new CheckForUpdatesJob(cdnTries: 1))->handle(); + + Http::assertSentCount(1); + Sleep::assertNeverSlept(); +});