fix(engine): reset step timer on retry so timed-out steps recover
A retried step (manual or after a timeout) now gets a fresh started_at, so RunRunner no longer immediately re-detects the timeout and burns the retry budget without executing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>feat/portal-design
parent
62c4412623
commit
8a2a2ee695
|
|
@ -36,6 +36,7 @@ class HostDetail extends Component
|
||||||
'status' => ProvisioningRun::STATUS_RUNNING,
|
'status' => ProvisioningRun::STATUS_RUNNING,
|
||||||
'attempt' => 0,
|
'attempt' => 0,
|
||||||
'next_attempt_at' => now(),
|
'next_attempt_at' => now(),
|
||||||
|
'started_at' => now(), // reset the step timer so it doesn't re-time-out instantly
|
||||||
'error' => null,
|
'error' => null,
|
||||||
]);
|
]);
|
||||||
$this->host->update(['status' => 'onboarding']);
|
$this->host->update(['status' => 'onboarding']);
|
||||||
|
|
|
||||||
|
|
@ -114,6 +114,7 @@ class RunRunner
|
||||||
|
|
||||||
$run->status = ProvisioningRun::STATUS_WAITING;
|
$run->status = ProvisioningRun::STATUS_WAITING;
|
||||||
$run->next_attempt_at = now()->addSeconds($result->afterSeconds);
|
$run->next_attempt_at = now()->addSeconds($result->afterSeconds);
|
||||||
|
$run->started_at = now(); // fresh per-attempt timer, else a timed-out step re-times-out
|
||||||
$run->save();
|
$run->save();
|
||||||
$this->record($run, $step, 'retry', $result->reason);
|
$this->record($run, $step, 'retry', $result->reason);
|
||||||
}
|
}
|
||||||
|
|
@ -123,6 +124,7 @@ class RunRunner
|
||||||
{
|
{
|
||||||
$run->status = ProvisioningRun::STATUS_WAITING;
|
$run->status = ProvisioningRun::STATUS_WAITING;
|
||||||
$run->next_attempt_at = now()->addSeconds($result->afterSeconds);
|
$run->next_attempt_at = now()->addSeconds($result->afterSeconds);
|
||||||
|
$run->started_at = now();
|
||||||
$run->save();
|
$run->save();
|
||||||
$this->record($run, $step, 'info', $result->reason);
|
$this->record($run, $step, 'info', $result->reason);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,7 @@ it('retries a failed run from the detail page', function () {
|
||||||
$host = Host::factory()->create(['status' => 'error']);
|
$host = Host::factory()->create(['status' => 'error']);
|
||||||
$run = ProvisioningRun::factory()->forHost($host)->create([
|
$run = ProvisioningRun::factory()->forHost($host)->create([
|
||||||
'status' => 'failed', 'current_step' => 3, 'error' => 'boom',
|
'status' => 'failed', 'current_step' => 3, 'error' => 'boom',
|
||||||
|
'started_at' => now()->subHour(),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
Livewire::actingAs(admin())->test(HostDetail::class, ['host' => $host])->call('retry');
|
Livewire::actingAs(admin())->test(HostDetail::class, ['host' => $host])->call('retry');
|
||||||
|
|
@ -80,6 +81,7 @@ it('retries a failed run from the detail page', function () {
|
||||||
$run->refresh();
|
$run->refresh();
|
||||||
expect($run->status)->toBe('running')
|
expect($run->status)->toBe('running')
|
||||||
->and($run->error)->toBeNull()
|
->and($run->error)->toBeNull()
|
||||||
|
->and($run->started_at->greaterThan(now()->subMinute()))->toBeTrue()
|
||||||
->and($host->fresh()->status)->toBe('onboarding');
|
->and($host->fresh()->status)->toBe('onboarding');
|
||||||
Queue::assertPushed(AdvanceRunJob::class);
|
Queue::assertPushed(AdvanceRunJob::class);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -130,6 +130,20 @@ it('retries when a step exceeds its max duration', function () {
|
||||||
->and($run->events()->where('message', 'like', '%timed out%')->exists())->toBeTrue();
|
->and($run->events()->where('message', 'like', '%timed out%')->exists())->toBeTrue();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('re-executes a step after a timeout retry instead of timing out again', function () {
|
||||||
|
bindPipeline(['test' => [FakeAdvanceStep::class]]);
|
||||||
|
$run = ProvisioningRun::factory()->create([
|
||||||
|
'pipeline' => 'test', 'status' => 'running',
|
||||||
|
'started_at' => now()->subMinutes(10), 'max_attempts' => 5,
|
||||||
|
]);
|
||||||
|
|
||||||
|
app(RunRunner::class)->advance($run); // times out -> retry (timer reset)
|
||||||
|
expect($run->fresh()->status)->toBe('waiting');
|
||||||
|
|
||||||
|
app(RunRunner::class)->advance($run->fresh()); // now actually executes -> completes
|
||||||
|
expect($run->fresh()->status)->toBe('completed');
|
||||||
|
});
|
||||||
|
|
||||||
it('is a no-op while the run lock is held', function () {
|
it('is a no-op while the run lock is held', function () {
|
||||||
bindPipeline(['test' => [FakeAdvanceStep::class, FakeAdvanceStep::class]]);
|
bindPipeline(['test' => [FakeAdvanceStep::class, FakeAdvanceStep::class]]);
|
||||||
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'current_step' => 0]);
|
$run = ProvisioningRun::factory()->create(['pipeline' => 'test', 'current_step' => 0]);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue