fix(engine-b): address Codex round 6 (crash-window idempotency)
- Clone: recover via vmExists() when the task ref was lost, instead of re-cloning the reserved vmid (which Proxmox rejects). - Deploy: persist the DB password ENCRYPTED before compose up and reuse it on retry, so a crash can't regenerate a mismatching credential. - Complete: gate credential delivery on a credentials_sent breadcrumb so a retry after a crash doesn't re-send the email. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>feat/portal-design
parent
e2b8f25e78
commit
f43ade79b0
|
|
@ -48,6 +48,12 @@ class CloneVirtualMachine extends CustomerStep
|
|||
// Clone once, keyed to the reserved vmid; record the task ref immediately.
|
||||
$upid = $run->context('clone_upid');
|
||||
if ($upid === null) {
|
||||
// Crash-recovery: if the VM already exists, a prior attempt cloned it
|
||||
// (but lost the task ref) — don't re-clone the same vmid (Proxmox rejects).
|
||||
if ($pve->vmExists($node, $vmid)) {
|
||||
return StepResult::advance();
|
||||
}
|
||||
|
||||
$upid = $pve->cloneVm($node, $templateVmid, $vmid, $name);
|
||||
$run->mergeContext(['clone_upid' => $upid]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -32,10 +32,12 @@ class CompleteProvisioning extends CustomerStep
|
|||
}
|
||||
|
||||
$encrypted = $run->context('admin_password');
|
||||
if ($encrypted !== null) {
|
||||
if ($encrypted !== null && ! $this->hasResource($run, 'credentials_sent')) {
|
||||
Notification::route('mail', $order->customer->email)->notify(
|
||||
new CloudReady($instance, (string) $instance->nc_admin_ref, Crypt::decryptString($encrypted)),
|
||||
);
|
||||
// Record delivery so a retry after a crash doesn't re-send.
|
||||
$this->recordResource($run, $instance->host, 'credentials_sent', (string) $instance->nc_admin_ref);
|
||||
$run->forgetContext('admin_password');
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ namespace App\Provisioning\Steps\Customer;
|
|||
use App\Models\ProvisioningRun;
|
||||
use App\Provisioning\StepResult;
|
||||
use App\Services\Proxmox\ProxmoxClient;
|
||||
use Illuminate\Support\Facades\Crypt;
|
||||
use Illuminate\Support\Str;
|
||||
|
||||
class DeployApplicationStack extends CustomerStep
|
||||
|
|
@ -28,10 +29,15 @@ class DeployApplicationStack extends CustomerStep
|
|||
$vmid = (int) $run->context('vmid');
|
||||
$pve = $this->pve->forHost($instance->host);
|
||||
|
||||
// Deploy once (guarded). The generated DB password is written straight
|
||||
// into the guest's .env and never persisted in the run context.
|
||||
// Deploy once (guarded). The DB password is persisted ENCRYPTED before the
|
||||
// stack starts and reused on retry, so a crash before `stack_deployed`
|
||||
// can't regenerate a password that mismatches the initialized database.
|
||||
if (! $run->context('stack_deployed')) {
|
||||
$dbPassword = Str::random(28);
|
||||
$encrypted = $run->context('db_password');
|
||||
$dbPassword = $encrypted !== null ? Crypt::decryptString($encrypted) : Str::random(28);
|
||||
if ($encrypted === null) {
|
||||
$run->mergeContext(['db_password' => Crypt::encryptString($dbPassword)]);
|
||||
}
|
||||
|
||||
$this->guest($pve, $run,
|
||||
'install -d /opt/nextcloud && printf %s '.escapeshellarg('MYSQL_PASSWORD='.$dbPassword."\n").
|
||||
|
|
@ -48,6 +54,8 @@ class DeployApplicationStack extends CustomerStep
|
|||
return StepResult::poll(15, 'waiting for application stack');
|
||||
}
|
||||
|
||||
$run->forgetContext('db_password'); // scrub once the stack is up
|
||||
|
||||
return StepResult::advance();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,6 +115,11 @@ class FakeProxmoxClient implements ProxmoxClient
|
|||
return ['status' => in_array($vmid, $this->runningVmids, true) ? 'running' : 'stopped'];
|
||||
}
|
||||
|
||||
public function vmExists(string $node, int $vmid): bool
|
||||
{
|
||||
return in_array($vmid, $this->clonedVmids, true) || in_array($vmid, $this->runningVmids, true);
|
||||
}
|
||||
|
||||
public function guestAgentPing(string $node, int $vmid): bool
|
||||
{
|
||||
return $this->guestAgentUp;
|
||||
|
|
|
|||
|
|
@ -83,6 +83,11 @@ class HttpProxmoxClient implements ProxmoxClient
|
|||
return $this->http()->get("/nodes/{$node}/qemu/{$vmid}/status/current")->throw()->json('data', []);
|
||||
}
|
||||
|
||||
public function vmExists(string $node, int $vmid): bool
|
||||
{
|
||||
return $this->http()->get("/nodes/{$node}/qemu/{$vmid}/status/current")->successful();
|
||||
}
|
||||
|
||||
public function guestAgentPing(string $node, int $vmid): bool
|
||||
{
|
||||
return $this->http()->asForm()->post("/nodes/{$node}/qemu/{$vmid}/agent/ping")->successful();
|
||||
|
|
|
|||
|
|
@ -42,6 +42,9 @@ interface ProxmoxClient
|
|||
/** @return array<string, mixed> ['status' => 'running'|'stopped', …] */
|
||||
public function vmStatus(string $node, int $vmid): array;
|
||||
|
||||
/** Whether a VM with this id already exists on the node. */
|
||||
public function vmExists(string $node, int $vmid): bool;
|
||||
|
||||
public function guestAgentPing(string $node, int $vmid): bool;
|
||||
|
||||
/** @return array{exitcode: int, out-data: string} */
|
||||
|
|
|
|||
|
|
@ -121,6 +121,19 @@ it('polls while cloning and fails on a clone error', function () {
|
|||
expect(app(CloneVirtualMachine::class)->execute($run2)->type)->toBe('fail');
|
||||
});
|
||||
|
||||
it('recovers a clone whose task ref was lost (no duplicate clone)', function () {
|
||||
$s = fakeServices();
|
||||
['run' => $run] = reservedRun([], ['vmid' => 101]);
|
||||
// Simulate a crash after the clone but before the upid was persisted:
|
||||
// vmid reserved + breadcrumb, no clone_upid, and the VM already exists.
|
||||
RunResource::create(['run_id' => $run->id, 'kind' => 'vmid', 'external_id' => '101']);
|
||||
$run->mergeContext(['vmid' => 101]);
|
||||
$s['pve']->clonedVmids[] = 101; // VM exists
|
||||
|
||||
expect(app(CloneVirtualMachine::class)->execute($run->fresh())->type)->toBe('advance')
|
||||
->and(count($s['pve']->clonedVmids))->toBe(1); // no second clone
|
||||
});
|
||||
|
||||
it('fails clone when the plan has no template', function () {
|
||||
fakeServices();
|
||||
$host = Host::factory()->active()->create(['node' => 'pve']);
|
||||
|
|
@ -193,23 +206,25 @@ it('polls network config until the guest has an address', function () {
|
|||
});
|
||||
|
||||
// 8. DeployApplicationStack
|
||||
it('deploys the stack, polling until healthy, without persisting the db password', function () {
|
||||
it('deploys the stack, polling until healthy, with an encrypted reusable db password', function () {
|
||||
$s = fakeServices();
|
||||
['run' => $run] = reservedRun();
|
||||
|
||||
// not healthy yet → poll; the db password is never in the run context
|
||||
// not healthy yet → poll; the stack is deployed and the password is encrypted
|
||||
expect(app(DeployApplicationStack::class)->execute($run)->type)->toBe('poll')
|
||||
->and($run->fresh()->context('db_password'))->toBeNull()
|
||||
->and($run->fresh()->context('stack_deployed'))->toBeTrue()
|
||||
->and($s['pve']->guestRan('docker compose up'))->toBeTrue();
|
||||
|
||||
// healthy → advance
|
||||
$s['pve']->guestScript('status.php', 0, 'ok');
|
||||
expect(app(DeployApplicationStack::class)->execute($run->fresh())->type)->toBe('advance');
|
||||
|
||||
// the persisted context never contains a plaintext DB credential
|
||||
$enc = $run->fresh()->context('db_password');
|
||||
expect($enc)->not->toBeNull();
|
||||
// the persisted context holds only the ENCRYPTED credential, never plaintext
|
||||
$raw = \DB::table('provisioning_runs')->where('id', $run->id)->value('context');
|
||||
expect($raw)->not->toContain('MYSQL_PASSWORD');
|
||||
expect($raw)->not->toContain(Crypt::decryptString($enc));
|
||||
|
||||
// healthy → advance, secret scrubbed
|
||||
$s['pve']->guestScript('status.php', 0, 'ok');
|
||||
expect(app(DeployApplicationStack::class)->execute($run->fresh())->type)->toBe('advance')
|
||||
->and($run->fresh()->context('db_password'))->toBeNull();
|
||||
});
|
||||
|
||||
// 9. ConfigureNextcloud
|
||||
|
|
@ -323,5 +338,8 @@ it('completes: activates instance+order, seeds onboarding, delivers credentials'
|
|||
->and($order->fresh()->status)->toBe('active')
|
||||
->and($instance->fresh()->onboardingTasks()->count())->toBeGreaterThan(0)
|
||||
->and($run->fresh()->context('admin_password'))->toBeNull();
|
||||
Notification::assertSentOnDemand(CloudReady::class);
|
||||
|
||||
// Re-running must NOT deliver the credentials a second time.
|
||||
app(CompleteProvisioning::class)->execute($run->fresh());
|
||||
Notification::assertSentOnDemandTimes(CloudReady::class, 1);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue