fix(engine): address Codex round 2 (poll budget, host error state, wg race)
- StepResult::poll — polling steps (reboot) wait without consuming the retry budget; the step owns its deadline. Reboot maxDuration > deadline. - Failed runs move a Host subject to 'error' via ProvisioningSubject hook (no host stuck 'onboarding'). - ConfigureWireguard allocates + reserves the wg_ip under a global lock; unique index on hosts.wg_ip as a backstop against duplicate addresses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>feat/portal-design
parent
65717bd3bd
commit
62c4412623
|
|
@ -3,11 +3,12 @@
|
||||||
namespace App\Models;
|
namespace App\Models;
|
||||||
|
|
||||||
use App\Models\Concerns\HasUuid;
|
use App\Models\Concerns\HasUuid;
|
||||||
|
use App\Provisioning\Contracts\ProvisioningSubject;
|
||||||
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
use Illuminate\Database\Eloquent\Factories\HasFactory;
|
||||||
use Illuminate\Database\Eloquent\Model;
|
use Illuminate\Database\Eloquent\Model;
|
||||||
use Illuminate\Database\Eloquent\Relations\MorphMany;
|
use Illuminate\Database\Eloquent\Relations\MorphMany;
|
||||||
|
|
||||||
class Host extends Model
|
class Host extends Model implements ProvisioningSubject
|
||||||
{
|
{
|
||||||
/** @use HasFactory<\Database\Factories\HostFactory> */
|
/** @use HasFactory<\Database\Factories\HostFactory> */
|
||||||
use HasFactory, HasUuid;
|
use HasFactory, HasUuid;
|
||||||
|
|
@ -39,6 +40,12 @@ class Host extends Model
|
||||||
return $this->morphMany(ProvisioningRun::class, 'subject');
|
return $this->morphMany(ProvisioningRun::class, 'subject');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Runner hook: a failed onboarding run moves the host to the error status. */
|
||||||
|
public function onProvisioningFailed(): void
|
||||||
|
{
|
||||||
|
$this->update(['status' => 'error']);
|
||||||
|
}
|
||||||
|
|
||||||
/** Free committable storage: total minus reserve. Instance quotas subtract in B. */
|
/** Free committable storage: total minus reserve. Instance quotas subtract in B. */
|
||||||
public function freeGb(): int
|
public function freeGb(): int
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,12 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
namespace App\Provisioning\Contracts;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A polymorphic run subject (Host, Order …) that reacts when its run fails,
|
||||||
|
* so the runner can update domain state without knowing the concrete model.
|
||||||
|
*/
|
||||||
|
interface ProvisioningSubject
|
||||||
|
{
|
||||||
|
public function onProvisioningFailed(): void;
|
||||||
|
}
|
||||||
|
|
@ -4,6 +4,7 @@ namespace App\Provisioning;
|
||||||
|
|
||||||
use App\Models\ProvisioningRun;
|
use App\Models\ProvisioningRun;
|
||||||
use App\Provisioning\Contracts\ProvisioningStep;
|
use App\Provisioning\Contracts\ProvisioningStep;
|
||||||
|
use App\Provisioning\Contracts\ProvisioningSubject;
|
||||||
use App\Provisioning\Events\StepAdvanced;
|
use App\Provisioning\Events\StepAdvanced;
|
||||||
use App\Provisioning\Jobs\AdvanceRunJob;
|
use App\Provisioning\Jobs\AdvanceRunJob;
|
||||||
use Illuminate\Support\Facades\Cache;
|
use Illuminate\Support\Facades\Cache;
|
||||||
|
|
@ -72,7 +73,8 @@ class RunRunner
|
||||||
match ($result->type) {
|
match ($result->type) {
|
||||||
StepResult::ADVANCE => $this->onAdvance($run, $step),
|
StepResult::ADVANCE => $this->onAdvance($run, $step),
|
||||||
StepResult::RETRY => $this->onRetry($run, $step, $result),
|
StepResult::RETRY => $this->onRetry($run, $step, $result),
|
||||||
StepResult::FAIL => $this->onFail($run, $step, $result),
|
StepResult::POLL => $this->onPoll($run, $step, $result),
|
||||||
|
StepResult::FAIL => $this->failRun($run, $step, $result->reason),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -105,10 +107,7 @@ class RunRunner
|
||||||
$run->attempt += 1;
|
$run->attempt += 1;
|
||||||
|
|
||||||
if ($run->attempt >= $run->max_attempts) {
|
if ($run->attempt >= $run->max_attempts) {
|
||||||
$run->status = ProvisioningRun::STATUS_FAILED;
|
$this->failRun($run, $step, $result->reason);
|
||||||
$run->error = $result->reason;
|
|
||||||
$run->save();
|
|
||||||
$this->record($run, $step, 'failed', $result->reason);
|
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -119,12 +118,27 @@ class RunRunner
|
||||||
$this->record($run, $step, 'retry', $result->reason);
|
$this->record($run, $step, 'retry', $result->reason);
|
||||||
}
|
}
|
||||||
|
|
||||||
private function onFail(ProvisioningRun $run, ProvisioningStep $step, StepResult $result): void
|
/** Poll: wait and re-run without consuming the retry budget (step owns its deadline). */
|
||||||
|
private function onPoll(ProvisioningRun $run, ProvisioningStep $step, StepResult $result): void
|
||||||
|
{
|
||||||
|
$run->status = ProvisioningRun::STATUS_WAITING;
|
||||||
|
$run->next_attempt_at = now()->addSeconds($result->afterSeconds);
|
||||||
|
$run->save();
|
||||||
|
$this->record($run, $step, 'info', $result->reason);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function failRun(ProvisioningRun $run, ProvisioningStep $step, string $reason): void
|
||||||
{
|
{
|
||||||
$run->status = ProvisioningRun::STATUS_FAILED;
|
$run->status = ProvisioningRun::STATUS_FAILED;
|
||||||
$run->error = $result->reason;
|
$run->error = $reason;
|
||||||
$run->save();
|
$run->save();
|
||||||
$this->record($run, $step, 'failed', $result->reason);
|
$this->record($run, $step, 'failed', $reason);
|
||||||
|
|
||||||
|
// Let the subject react (e.g. a Host moves to the 'error' status).
|
||||||
|
$subject = $run->subject;
|
||||||
|
if ($subject instanceof ProvisioningSubject) {
|
||||||
|
$subject->onProvisioningFailed();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function record(ProvisioningRun $run, ProvisioningStep $step, string $outcome, ?string $message): void
|
private function record(ProvisioningRun $run, ProvisioningStep $step, string $outcome, ?string $message): void
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ final class StepResult
|
||||||
{
|
{
|
||||||
public const ADVANCE = 'advance';
|
public const ADVANCE = 'advance';
|
||||||
public const RETRY = 'retry';
|
public const RETRY = 'retry';
|
||||||
|
public const POLL = 'poll';
|
||||||
public const FAIL = 'fail';
|
public const FAIL = 'fail';
|
||||||
|
|
||||||
private function __construct(
|
private function __construct(
|
||||||
|
|
@ -28,6 +29,15 @@ final class StepResult
|
||||||
return new self(self::RETRY, $afterSeconds, $reason);
|
return new self(self::RETRY, $afterSeconds, $reason);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Wait and re-run WITHOUT consuming the retry budget — for polling long
|
||||||
|
* operations (e.g. a reboot). The step enforces its own deadline via fail().
|
||||||
|
*/
|
||||||
|
public static function poll(int $afterSeconds, string $reason): self
|
||||||
|
{
|
||||||
|
return new self(self::POLL, $afterSeconds, $reason);
|
||||||
|
}
|
||||||
|
|
||||||
public static function fail(string $reason): self
|
public static function fail(string $reason): self
|
||||||
{
|
{
|
||||||
return new self(self::FAIL, 0, $reason);
|
return new self(self::FAIL, 0, $reason);
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ use App\Models\ProvisioningRun;
|
||||||
use App\Provisioning\StepResult;
|
use App\Provisioning\StepResult;
|
||||||
use App\Services\Ssh\RemoteShell;
|
use App\Services\Ssh\RemoteShell;
|
||||||
use App\Services\Wireguard\WireguardHub;
|
use App\Services\Wireguard\WireguardHub;
|
||||||
|
use Illuminate\Support\Facades\Cache;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Installs WireGuard on the host, allocates a management IP, registers the host
|
* Installs WireGuard on the host, allocates a management IP, registers the host
|
||||||
|
|
@ -37,7 +38,17 @@ class ConfigureWireguard extends HostStep
|
||||||
return StepResult::retry(20, 'could not read host WireGuard public key');
|
return StepResult::retry(20, 'could not read host WireGuard public key');
|
||||||
}
|
}
|
||||||
|
|
||||||
$wgIp = $host->wg_ip ?: $this->hub->allocateIp();
|
// Allocate + reserve the management IP atomically so concurrent
|
||||||
|
// onboarding runs can never receive the same address.
|
||||||
|
$wgIp = Cache::lock('wireguard:allocate', 30)->block(10, function () use ($host) {
|
||||||
|
$ip = $host->wg_ip ?: $this->hub->allocateIp();
|
||||||
|
if (blank($host->wg_ip)) {
|
||||||
|
$host->update(['wg_ip' => $ip]);
|
||||||
|
}
|
||||||
|
|
||||||
|
return $ip;
|
||||||
|
});
|
||||||
|
|
||||||
$privateKey = trim($this->shell->run('cat /etc/wireguard/privatekey')->stdout);
|
$privateKey = trim($this->shell->run('cat /etc/wireguard/privatekey')->stdout);
|
||||||
|
|
||||||
$this->shell->putFile('/etc/wireguard/wg0.conf', $this->renderConfig($wgIp, $privateKey));
|
$this->shell->putFile('/etc/wireguard/wg0.conf', $this->renderConfig($wgIp, $privateKey));
|
||||||
|
|
@ -46,7 +57,7 @@ class ConfigureWireguard extends HostStep
|
||||||
$this->hub->addPeer($publicKey, $wgIp);
|
$this->hub->addPeer($publicKey, $wgIp);
|
||||||
|
|
||||||
// Persist external identity BEFORE verifying/advancing (crash-safe).
|
// Persist external identity BEFORE verifying/advancing (crash-safe).
|
||||||
$host->update(['wg_ip' => $wgIp, 'wg_pubkey' => $publicKey]);
|
$host->update(['wg_pubkey' => $publicKey]);
|
||||||
$this->recordResource($run, $host, 'wg_peer', $publicKey);
|
$this->recordResource($run, $host, 'wg_peer', $publicKey);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -24,7 +24,9 @@ class RebootIntoPveKernel extends HostStep
|
||||||
|
|
||||||
public function maxDuration(): int
|
public function maxDuration(): int
|
||||||
{
|
{
|
||||||
return 900;
|
// Above the 15-min reboot deadline below, so the step's own deadline
|
||||||
|
// governs failure rather than the generic step timeout.
|
||||||
|
return 1800;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function execute(ProvisioningRun $run): StepResult
|
public function execute(ProvisioningRun $run): StepResult
|
||||||
|
|
@ -41,7 +43,7 @@ class RebootIntoPveKernel extends HostStep
|
||||||
]);
|
]);
|
||||||
$this->shell->run('systemctl reboot || reboot');
|
$this->shell->run('systemctl reboot || reboot');
|
||||||
|
|
||||||
return StepResult::retry(30, 'rebooting into the Proxmox kernel');
|
return StepResult::poll(30, 'rebooting into the Proxmox kernel');
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
|
@ -60,6 +62,6 @@ class RebootIntoPveKernel extends HostStep
|
||||||
return StepResult::fail('Host did not return on the Proxmox kernel before the deadline.');
|
return StepResult::fail('Host did not return on the Proxmox kernel before the deadline.');
|
||||||
}
|
}
|
||||||
|
|
||||||
return StepResult::retry(15, 'waiting for the Proxmox kernel');
|
return StepResult::poll(15, 'waiting for the Proxmox kernel');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use Illuminate\Database\Migrations\Migration;
|
||||||
|
use Illuminate\Database\Schema\Blueprint;
|
||||||
|
use Illuminate\Support\Facades\Schema;
|
||||||
|
|
||||||
|
return new class extends Migration
|
||||||
|
{
|
||||||
|
public function up(): void
|
||||||
|
{
|
||||||
|
// Backstop for the WireGuard allocation lock: two hosts can never share a
|
||||||
|
// management IP (nullable → many hosts may still be un-provisioned).
|
||||||
|
Schema::table('hosts', function (Blueprint $table) {
|
||||||
|
$table->unique('wg_ip');
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public function down(): void
|
||||||
|
{
|
||||||
|
Schema::table('hosts', function (Blueprint $table) {
|
||||||
|
$table->dropUnique(['wg_ip']);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
@ -140,7 +140,7 @@ it('issues the reboot on first execution', function () {
|
||||||
$s = fakeServices();
|
$s = fakeServices();
|
||||||
$run = hostRun(Host::factory()->create());
|
$run = hostRun(Host::factory()->create());
|
||||||
|
|
||||||
expect(app(RebootIntoPveKernel::class)->execute($run)->type)->toBe('retry')
|
expect(app(RebootIntoPveKernel::class)->execute($run)->type)->toBe('poll')
|
||||||
->and($run->fresh()->context('reboot_issued'))->toBeTrue()
|
->and($run->fresh()->context('reboot_issued'))->toBeTrue()
|
||||||
->and($s['shell']->ran('reboot'))->toBeTrue();
|
->and($s['shell']->ran('reboot'))->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
@ -164,7 +164,7 @@ it('keeps waiting while the old kernel is still running', function () {
|
||||||
'reboot_deadline' => now()->addMinutes(10)->toIso8601String(),
|
'reboot_deadline' => now()->addMinutes(10)->toIso8601String(),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
expect(app(RebootIntoPveKernel::class)->execute($run)->type)->toBe('retry');
|
expect(app(RebootIntoPveKernel::class)->execute($run)->type)->toBe('poll');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('fails when the host does not return before the reboot deadline', function () {
|
it('fails when the host does not return before the reboot deadline', function () {
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
|
use App\Models\Host;
|
||||||
use App\Models\ProvisioningRun;
|
use App\Models\ProvisioningRun;
|
||||||
use App\Provisioning\Events\StepAdvanced;
|
use App\Provisioning\Events\StepAdvanced;
|
||||||
use App\Provisioning\Jobs\AdvanceRunJob;
|
use App\Provisioning\Jobs\AdvanceRunJob;
|
||||||
|
|
@ -10,6 +11,7 @@ use Illuminate\Support\Facades\Event;
|
||||||
use Illuminate\Support\Facades\Queue;
|
use Illuminate\Support\Facades\Queue;
|
||||||
use Tests\Support\Steps\FakeAdvanceStep;
|
use Tests\Support\Steps\FakeAdvanceStep;
|
||||||
use Tests\Support\Steps\FakeFailStep;
|
use Tests\Support\Steps\FakeFailStep;
|
||||||
|
use Tests\Support\Steps\FakePollStep;
|
||||||
use Tests\Support\Steps\FakeRetryStep;
|
use Tests\Support\Steps\FakeRetryStep;
|
||||||
use Tests\Support\Steps\FakeThrowStep;
|
use Tests\Support\Steps\FakeThrowStep;
|
||||||
|
|
||||||
|
|
@ -77,6 +79,29 @@ it('fails immediately on a fail result', function () {
|
||||||
expect($run->status)->toBe('failed')->and($run->error)->toBe('boom');
|
expect($run->status)->toBe('failed')->and($run->error)->toBe('boom');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('polls without consuming the retry budget', function () {
|
||||||
|
bindPipeline(['test' => [FakePollStep::class]]);
|
||||||
|
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'max_attempts' => 1]);
|
||||||
|
|
||||||
|
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.
|
||||||
|
app(RunRunner::class)->advance($run->fresh());
|
||||||
|
expect($run->fresh()->status)->toBe('waiting')->and($run->fresh()->attempt)->toBe(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('marks a Host subject as error when the run fails', function () {
|
||||||
|
bindPipeline(['test' => [FakeFailStep::class]]);
|
||||||
|
$host = Host::factory()->create(['status' => 'onboarding']);
|
||||||
|
$run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'test']);
|
||||||
|
|
||||||
|
app(RunRunner::class)->advance($run);
|
||||||
|
|
||||||
|
expect($run->fresh()->status)->toBe('failed')
|
||||||
|
->and($host->fresh()->status)->toBe('error');
|
||||||
|
});
|
||||||
|
|
||||||
it('treats a thrown exception as a retry', function () {
|
it('treats a thrown exception as a retry', function () {
|
||||||
bindPipeline(['test' => [FakeThrowStep::class]]);
|
bindPipeline(['test' => [FakeThrowStep::class]]);
|
||||||
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'max_attempts' => 5]);
|
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'max_attempts' => 5]);
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,30 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
namespace Tests\Support\Steps;
|
||||||
|
|
||||||
|
use App\Models\ProvisioningRun;
|
||||||
|
use App\Provisioning\Contracts\ProvisioningStep;
|
||||||
|
use App\Provisioning\StepResult;
|
||||||
|
|
||||||
|
class FakePollStep implements ProvisioningStep
|
||||||
|
{
|
||||||
|
public function key(): string
|
||||||
|
{
|
||||||
|
return 'fake_poll';
|
||||||
|
}
|
||||||
|
|
||||||
|
public function label(): string
|
||||||
|
{
|
||||||
|
return 'Fake poll';
|
||||||
|
}
|
||||||
|
|
||||||
|
public function maxDuration(): int
|
||||||
|
{
|
||||||
|
return 3600;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function execute(ProvisioningRun $run): StepResult
|
||||||
|
{
|
||||||
|
return StepResult::poll(15, 'still waiting');
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue