From 41554da7c0a9d65b217c6de842ec94680269ad66 Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 20:33:50 +0200 Subject: [PATCH] Behebe 4 kritische Fehler in ConfigureInstanceMail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Commands/ConfigureInstanceMail.php | 79 +++++++++++++------ .../ConfigureInstanceMailCommandTest.php | 33 +++++++- 2 files changed, 87 insertions(+), 25 deletions(-) diff --git a/app/Console/Commands/ConfigureInstanceMail.php b/app/Console/Commands/ConfigureInstanceMail.php index 41d29d9..e7c42aa 100644 --- a/app/Console/Commands/ConfigureInstanceMail.php +++ b/app/Console/Commands/ConfigureInstanceMail.php @@ -33,10 +33,10 @@ class ConfigureInstanceMail extends Command public function handle(): int { $dryRun = (bool) $this->option('dry-run'); - $gestartet = 0; + $started = 0; - /** @var array Grund => Anzahl */ - $uebersprungen = []; + /** @var array 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(); + } } diff --git a/tests/Feature/Console/ConfigureInstanceMailCommandTest.php b/tests/Feature/Console/ConfigureInstanceMailCommandTest.php index 37ca0f3..fb85552 100644 --- a/tests/Feature/Console/ConfigureInstanceMailCommandTest.php +++ b/tests/Feature/Console/ConfigureInstanceMailCommandTest.php @@ -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); +});