From fbe87d99d55760b06ee9fb72df28c99ee2d3784a Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 10:32:14 +0200 Subject: [PATCH] fix(engine): honour backoff timing + don't double-count Proxmox storage - RunRunner returns early for a waiting run whose next_attempt_at is in the future, so stale/duplicate jobs can't bypass backoff. - RegisterCapacity takes the largest VM-capable datastore instead of summing overlapping pools (local + local-lvm), preventing placement overcommit. Co-Authored-By: Claude Opus 4.8 --- app/Provisioning/RunRunner.php | 7 +++++++ app/Provisioning/Steps/Host/RegisterCapacity.php | 13 ++++++++++++- app/Services/Proxmox/FakeProxmoxClient.php | 3 ++- .../Provisioning/HostOnboardingEndToEndTest.php | 6 ++++++ tests/Feature/Provisioning/RunRunnerTest.php | 4 +++- 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/Provisioning/RunRunner.php b/app/Provisioning/RunRunner.php index b21d82e..692716e 100644 --- a/app/Provisioning/RunRunner.php +++ b/app/Provisioning/RunRunner.php @@ -45,6 +45,13 @@ class RunRunner return; } + // Honour backoff: a stale/duplicate job must not run a waiting run early. + if ($run->status === ProvisioningRun::STATUS_WAITING + && $run->next_attempt_at !== null + && $run->next_attempt_at->isFuture()) { + return; + } + // started_at marks when the current step began (drives the timeout). if ($run->started_at === null || $run->status === ProvisioningRun::STATUS_PENDING) { $run->started_at ??= now(); diff --git a/app/Provisioning/Steps/Host/RegisterCapacity.php b/app/Provisioning/Steps/Host/RegisterCapacity.php index b3c738c..b658e93 100644 --- a/app/Provisioning/Steps/Host/RegisterCapacity.php +++ b/app/Provisioning/Steps/Host/RegisterCapacity.php @@ -32,7 +32,18 @@ class RegisterCapacity extends HostStep $cores = (int) ($status['cpuinfo']['cpus'] ?? 0); $ramMb = (int) round((int) ($status['memory']['total'] ?? 0) / 1048576); - $totalGb = (int) round(array_sum(array_column($storage, 'total')) / 1073741824); + + // Proxmox lists overlapping pools (e.g. local + local-lvm on one disk). + // Take the largest VM-capable datastore instead of summing, so shared + // backing storage is never double-counted and placement can't overcommit. + $allocatable = array_filter($storage, function ($s) { + $content = (string) ($s['content'] ?? ''); + + return str_contains($content, 'images') || str_contains($content, 'rootdir'); + }); + $pool = $allocatable ?: $storage; + $totalBytes = $pool === [] ? 0 : max(array_map(fn ($s) => (int) ($s['total'] ?? 0), $pool)); + $totalGb = (int) round($totalBytes / 1073741824); $host->update([ 'cpu_cores' => $cores, diff --git a/app/Services/Proxmox/FakeProxmoxClient.php b/app/Services/Proxmox/FakeProxmoxClient.php index 56017c5..e91eb3d 100644 --- a/app/Services/Proxmox/FakeProxmoxClient.php +++ b/app/Services/Proxmox/FakeProxmoxClient.php @@ -20,7 +20,8 @@ class FakeProxmoxClient implements ProxmoxClient /** @var array> */ public array $storage = [ - ['storage' => 'local-lvm', 'type' => 'lvmthin', 'total' => 1099511627776], // 1 TiB + ['storage' => 'local', 'type' => 'dir', 'content' => 'iso,vztmpl,backup', 'total' => 1099511627776], + ['storage' => 'local-lvm', 'type' => 'lvmthin', 'content' => 'images,rootdir', 'total' => 1099511627776], // 1 TiB ]; public function forHost(Host $host): static diff --git a/tests/Feature/Provisioning/HostOnboardingEndToEndTest.php b/tests/Feature/Provisioning/HostOnboardingEndToEndTest.php index 6e926a7..13e8700 100644 --- a/tests/Feature/Provisioning/HostOnboardingEndToEndTest.php +++ b/tests/Feature/Provisioning/HostOnboardingEndToEndTest.php @@ -31,6 +31,9 @@ it('drives a fresh host all the way to active (mocked)', function () { if (in_array($run->status, ['completed', 'failed'], true)) { break; } + if ($run->next_attempt_at?->isFuture()) { + $run->update(['next_attempt_at' => now()]); // simulate the scheduler waking a due run + } $runner->advance($run); } @@ -73,6 +76,9 @@ it('does not duplicate external resources when a step re-runs after a crash', fu // WireGuard step and replaying it — the breadcrumb must prevent duplicates. for ($i = 0; $i < 6; $i++) { $run->refresh(); + if ($run->next_attempt_at?->isFuture()) { + $run->update(['next_attempt_at' => now()]); + } $runner->advance($run); } $run->refresh(); diff --git a/tests/Feature/Provisioning/RunRunnerTest.php b/tests/Feature/Provisioning/RunRunnerTest.php index a047e7d..68b944c 100644 --- a/tests/Feature/Provisioning/RunRunnerTest.php +++ b/tests/Feature/Provisioning/RunRunnerTest.php @@ -86,7 +86,9 @@ it('polls without consuming the retry budget', function () { app(RunRunner::class)->advance($run); expect($run->fresh()->status)->toBe('waiting')->and($run->fresh()->attempt)->toBe(0); - // A second poll must not fail the run even with max_attempts = 1. + // Make the run due again, then poll a second time: still not failed even + // with max_attempts = 1 (polls don't consume the retry budget). + $run->update(['next_attempt_at' => now()->subSecond()]); app(RunRunner::class)->advance($run->fresh()); expect($run->fresh()->status)->toBe('waiting')->and($run->fresh()->attempt)->toBe(0); });