Behebe 4 kritische Fehler in ConfigureInstanceMail

1. Übergebe $run->uuid statt $run->id an AdvanceRunJob (KRITISCH: Lauf
   wird nie gefahren, da HasUuid eine separate uuid erzeugt)
2. Benutze STATUS_PENDING statt STATUS_RUNNING beim Anlegen
3. Fuege hasRunInFlight-Schutz gegen doppelte Laeufe ein
4. Stelle Umlaute in Betreibertexten her, halte Code umlautfrei
5. Reorganisiere handle() nach RefreshHostFirewall-Muster
6. Erweitere Tests um Queue::assertPushed() fuer AdvanceRunJob mit uuid

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
claude/nice-moser-521659
nexxo 2026-08-03 20:33:50 +02:00
parent d861c34c26
commit 41554da7c0
2 changed files with 87 additions and 25 deletions

View File

@ -33,10 +33,10 @@ class ConfigureInstanceMail extends Command
public function handle(): int
{
$dryRun = (bool) $this->option('dry-run');
$gestartet = 0;
$started = 0;
/** @var array<string, int> Grund => Anzahl */
$uebersprungen = [];
/** @var array<string, int> reason => count */
$skipped = [];
$query = Instance::query()->where('status', 'active');
@ -45,23 +45,18 @@ class ConfigureInstanceMail extends Command
}
foreach ($query->with('host')->get() as $instance) {
if ($instance->host === null) {
$this->warn("{$instance->uuid}: kein Host — uebersprungen");
$uebersprungen['kein Host'] = ($uebersprungen['kein Host'] ?? 0) + 1;
$reason = $this->reasonToSkip($instance);
if ($reason !== null) {
$skipped[$reason] = ($skipped[$reason] ?? 0) + 1;
continue;
}
if (blank($instance->vmid)) {
$this->warn("{$instance->uuid}: keine VMID — uebersprungen");
$uebersprungen['keine VMID'] = ($uebersprungen['keine VMID'] ?? 0) + 1;
continue;
}
$started++;
if ($dryRun) {
$this->line("{$instance->uuid}: wuerde nachgetragen");
$gestartet++;
$this->line("{$instance->uuid}: würde nachgetragen");
continue;
}
@ -70,7 +65,7 @@ class ConfigureInstanceMail extends Command
'pipeline' => 'instance-mail',
'subject_type' => Instance::class,
'subject_id' => $instance->id,
'status' => ProvisioningRun::STATUS_RUNNING,
'status' => ProvisioningRun::STATUS_PENDING,
'current_step' => 0,
'context' => [
'instance_id' => $instance->id,
@ -79,18 +74,58 @@ class ConfigureInstanceMail extends Command
],
]);
AdvanceRunJob::dispatch($run->id);
AdvanceRunJob::dispatch($run->uuid);
$this->info("{$instance->uuid}: Lauf {$run->id} gestartet");
$gestartet++;
$started++;
}
$this->newLine();
$this->line($dryRun ? "{$gestartet} Instanz(en) waeren nachgetragen worden." : "{$gestartet} Lauf/Laeufe gestartet.");
foreach ($uebersprungen as $grund => $anzahl) {
$this->line("uebersprungen ({$grund}): {$anzahl}");
foreach ($skipped as $reason => $count) {
$this->line("übersprungen ({$reason}): {$count}");
}
$this->info($dryRun
? "Probelauf: {$started} Instanz(en) bekämen den Mailversand, ".array_sum($skipped).' übersprungen. Nichts wurde geändert.'
: "{$started} Lauf/Läufe gestartet, ".array_sum($skipped).' übersprungen.');
return self::SUCCESS;
}
/**
* Warum diese Instanz uebergangen wird, oder null.
*
* Der Grund IST der Bericht: „2 uebergangen" allein ist keine Auskunft,
* mit der jemand etwas anfangen kann „2 haben keinen Host" und „2 laufen
* bereits" sehen von hier aus gleich aus, sind aber völlig verschiedene
* Reparaturen.
*/
private function reasonToSkip(Instance $instance): ?string
{
if ($instance->host === null) {
return 'kein Host';
}
if (blank($instance->vmid)) {
return 'keine VMID';
}
if ($this->hasRunInFlight($instance)) {
return 'anderer Lauf aktiv';
}
return null;
}
/**
* JEDER Lauf gegen diese Instanz, absichtlich weit gefasst: das hier ist ein
* Reparatur-Durchgang. Er läuft wieder, und eine heute übergangene Instanz ist
* beim nächsten Aufruf dran. Nichts geht durch Warten verloren.
*/
private function hasRunInFlight(Instance $instance): bool
{
return ProvisioningRun::query()
->where('subject_type', Instance::class)
->where('subject_id', $instance->id)
->inFlight()
->exists();
}
}

View File

@ -3,6 +3,7 @@
use App\Models\Host;
use App\Models\Instance;
use App\Models\ProvisioningRun;
use App\Provisioning\Jobs\AdvanceRunJob;
use Illuminate\Support\Facades\Queue;
// QUEUE_CONNECTION ist in dieser Suite `sync`: ohne das hier faehrt jedes
@ -29,13 +30,21 @@ function aktiveInstanzMitHost(array $attributes = []): Instance
}
it('startet je aktiver Instanz einen Lauf', function () {
aktiveInstanzMitHost();
aktiveInstanzMitHost(['vmid' => 102]);
$i1 = aktiveInstanzMitHost();
$i2 = aktiveInstanzMitHost(['vmid' => 102]);
Instance::factory()->create(['status' => 'closed']);
$this->artisan('clupilot:configure-instance-mail')->assertSuccessful();
expect(ProvisioningRun::where('pipeline', 'instance-mail')->count())->toBe(2);
$runs = ProvisioningRun::where('pipeline', 'instance-mail')->get();
expect($runs->count())->toBe(2)
->and($runs[0]->status)->toBe(ProvisioningRun::STATUS_PENDING)
->and($runs[1]->status)->toBe(ProvisioningRun::STATUS_PENDING);
// Der Auftrag muss mit der uuid des Laufs dispatched werden, nicht mit id.
// Ohne uuid findet AdvanceRunJob keine Zeile und der Lauf steht für immer auf running.
Queue::assertPushed(AdvanceRunJob::class, fn ($job) => $job->runUuid === $runs[0]->uuid);
Queue::assertPushed(AdvanceRunJob::class, fn ($job) => $job->runUuid === $runs[1]->uuid);
});
it('faengt unter --dry-run gar nichts an', function () {
@ -77,3 +86,21 @@ it('ueberspringt eine Instanz ohne vmid und sagt es', function () {
expect(ProvisioningRun::where('pipeline', 'instance-mail')->count())->toBe(0);
});
it('ueberspringt eine Instanz mit laufendem Lauf und sagt es', function () {
$instance = aktiveInstanzMitHost();
// Ein bereits laufender Lauf gegen diese Instanz
ProvisioningRun::factory()->create([
'subject_type' => Instance::class,
'subject_id' => $instance->id,
'status' => ProvisioningRun::STATUS_RUNNING,
]);
$this->artisan('clupilot:configure-instance-mail')
->expectsOutputToContain('anderer Lauf aktiv')
->assertSuccessful();
// Kein neuer Lauf angelegt
expect(ProvisioningRun::where('pipeline', 'instance-mail')->count())->toBe(0);
});