boban
/
CluPilotCloud
1
0
Fork 0
Code Issues Pull Requests Actions 39 Packages Projects Releases Wiki Activity

Kuendigung B1, Task 3 — Fix-Runde: der Nachtrag braucht ein Lebenszeichen 

Vier Befunde aus der Pruefung.

Der schwerste: der Nachtrags-Zweig konnte eine LAUFENDE Kundenmaschine als
abgebaut verbuchen. vmExists() ist ->get(...)->successful() ohne ->throw(), und
Proxmox antwortet auf die Konfiguration einer nicht vorhandenen VM mit 500 —
demselben Code wie ein hakender Knoten. "Gibt es nicht" und "ich konnte nicht
fragen" sahen damit gleich aus, und der Zustand entsteht im Regelbetrieb: ein
Lauf sichert, scheitert am Loeschen, und der naechste findet archive_volid
gesetzt und einen Knoten, der nichts beantwortet. Die Ablagenpruefung steht
deshalb jetzt VOR dem Zweig: nodeStorage() ruft ->throw(), laeuft sie durch, hat
der Knoten geantwortet, und erst dann ist ein "nein" aus vmExists() ein Befund
statt einer Vermutung.

Drei Ausnahmen lagen ausserhalb des try und haetten im Zeitplan die uebrigen
Instanzen mitgerissen: das Anlegen der Sperre (im Betrieb Redis), das Vermerken
des Grundes im catch, und die Freigabe im finally — die sogar am Erfolgsfall
vorbei. Alle drei abgesichert.

Die Sperrfrist war mit backupWaitSeconds + 600 knapper als der laengste Lauf
(Herunterfahren UND Sichern) und konnte kurz vor dem Loeschen auslaufen. Jetzt
shutdownWaitSeconds + backupWaitSeconds + Puffer — und sie hat endlich eine
eigene Pruefung.

Ein einzelnes Archiv ohne Zeitpunkt bleibt gueltig, mehrere nicht: sie lassen
sich nicht ordnen, und vermerkt wuerde womoeglich die Sicherung von vorgestern.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat/versandtakt
nexxo 2026-08-04 11:36:18 +02:00
parent 104ac46d45
commit 51ec6ebed2
2 changed files with 182 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

134
app/Actions/ArchiveAndTearDown.php
View File

@ -173,19 +173,25 @@ class ArchiveAndTearDown
return false;
}
// Zwei Läufe über dieselbe Instanz — der Zeitplan und ein Betreiber von
// Hand — würden zweimal sichern und zweimal löschen; der zweite fände
// dabei eine Maschine vor, die der erste gerade wegnimmt. Niemand
// wartet auf die Sperre: wer sie nicht bekommt, hat nichts
// hinzuzufügen, weil der Lauf, der sie hält, dasselbe tut. Die Frist
// deckt den ganzen Dump ab, sonst liefe sie mitten im Sichern aus.
$lock = Cache::lock('instance-teardown:'.$instance->uuid, $this->backupWaitSeconds + 600);
if (! $lock->get()) {
return false;
}
$lock = null;
try {
// Zwei Läufe über dieselbe Instanz — der Zeitplan und ein Betreiber
// von Hand — würden zweimal sichern und zweimal löschen; der zweite
// fände dabei eine Maschine vor, die der erste gerade wegnimmt.
// Niemand wartet auf die Sperre: wer sie nicht bekommt, hat nichts
// hinzuzufügen, weil der Lauf, der sie hält, dasselbe tut.
//
// Innerhalb des try, nicht davor: im Betrieb ist der Sperrspeicher
// Redis, und ein Redis, das gerade nicht da ist, wirft schon beim
// Anlegen der Sperre — vor dem try nähme dieser Wurf im Zeitplan
// alle übrigen Instanzen mit.
$lock = Cache::lock('instance-teardown:'.$instance->uuid, $this->lockSeconds());
if (! $lock->get()) {
return false;
}
return $this->tearDown($instance);
} catch (Throwable $e) {
// Keine Ausnahme entkommt: dieser Ablauf läuft im Zeitplan über
@ -194,10 +200,31 @@ class ArchiveAndTearDown
return false;
} finally {
$lock->release();
try {
$lock?->release();
} catch (Throwable) {
// Dieselbe Verbindung wie oben und derselbe Weg nach draussen —
// diesmal sogar am Erfolgsfall vorbei. Eine Sperre, die sich
// nicht freigeben lässt, läuft von selbst ab; ein Wurf von hier
// machte aus einem fertigen Abbau einen gescheiterten Zeitplan.
}
}
}
/**
* Wie lange die Sperre halten muss.
*
* Der ganze Lauf, nicht nur der Dump: im schlechtesten Fall wartet er erst
* die Frist fürs Herunterfahren und danach die fürs Sichern ab. Eine kürzere
* Sperre liefe ausgerechnet kurz vor dem Löschen aus — also genau dann, wenn
* ein zweiter Lauf am meisten Schaden anrichtet. Der Zuschlag deckt die
* Ablagenprüfung, die Nachschau und die HTTP-Umläufe dazwischen.
*/
private function lockSeconds(): int
{
return $this->shutdownWaitSeconds + $this->backupWaitSeconds + 600;
}
/**
* Der eigentliche Ablauf. Jeder Riegel wirft, der Aufrufer oben fängt und
* schreibt den Grund an den Datensatz — ein Wurf ist hier Ablaufsteuerung
@ -219,14 +246,29 @@ class ArchiveAndTearDown
$pve = $this->pve->forHost($host);
$vmid = (int) $vmid;
// Zuerst die Ablage, und das aus zwei Gründen. Der offensichtliche: eine
// Instanz, deren Archiv nirgends hin kann, wird gar nicht erst
// angefasst. Der wichtigere: `nodeStorage()` ruft `->throw()` und ist
// damit das Lebenszeichen des Knotens, das der Nachtrags-Zweig
// darunter braucht.
$storage = $this->archiveStorage($pve, $node);
// Der Weg zurück aus einem Absturz zwischen Löschen und Buchen: die
// Maschine ist fort, das Archiv steht aber schon am Datensatz. Ohne
// diesen Zweig versuchte jeder weitere Lauf, einen Geist
// herunterzufahren, und die Instanz bliebe für immer im Fehler stehen.
// Bewusst an `archive_volid` gebunden und nicht an `vmExists()` allein:
// die Frage beantwortet auch ein hakendes Proxmox mit „nein", und ohne
// ein verbrieftes Archiv wäre das „schon weg" eine Maschine, die
// niemand mehr anfasst.
//
// Zwei Bedingungen, weil `vmExists()` allein die Frage nicht beantwortet:
// es ist `->get(…)->successful()` OHNE `->throw()`, und Proxmox antwortet
// auf die Konfiguration einer nicht vorhandenen VM mit 500 — demselben
// Code, den ein hakender Knoten liefert (siehe `isTemplate()` im
// Client). „Gibt es nicht" und „ich konnte nicht fragen" sehen also
// gleich aus. Ein verbrieftes `archive_volid` schliesst die Maschine
// aus, die nie archiviert wurde; die Ablagenprüfung darüber schliesst
// den Knoten aus, der gerade gar nichts beantwortet. Ohne beides würde
// hier eine LAUFENDE Kundenmaschine als abgebaut verbucht und danach
// nie wieder angesehen — der Zustand entsteht im Regelbetrieb, sobald
// ein Lauf nach dem Sichern am Löschen scheitert.
if ($instance->archive_volid !== null && ! $pve->vmExists($node, $vmid)) {
$instance->update(['torn_down_at' => now(), 'teardown_error' => null]);
@ -239,10 +281,6 @@ class ArchiveAndTearDown
return true;
}
// Vor dem Herunterfahren: eine Instanz, deren Archiv nirgends hin
// kann, wird gar nicht erst angefasst.
$storage = $this->archiveStorage($pve, $node);
$shutdownUpid = $pve->shutdownVm($node, $vmid, $this->shutdownWaitSeconds);
$this->awaitTask($pve, $node, $shutdownUpid, $this->shutdownWaitSeconds, 'Das Herunterfahren');
@ -404,12 +442,24 @@ class ArchiveAndTearDown
$ctime = $juengstes['ctime'] ?? null;
if ($ctime === null) {
// Eine echte Ablage nennt zu jedem Eintrag ein `ctime`; fehlt es,
// ist das Alter des Archivs schlicht nicht feststellbar. Dann wird
// es angenommen statt abgelehnt — es LIEGT ein Archiv dieser VM da,
// und der Fall, den diese Aktion zu verhindern hat, ist das Löschen
// ohne jedes Archiv. Laut genug, dass es auffällt, wenn es je öfter
// vorkommt.
// Ohne `ctime` ist das Alter eines Archivs nicht feststellbar. Ein
// EINZELNER solcher Eintrag wird angenommen statt abgelehnt: es
// liegt ein Archiv dieser VM da, und der Fall, den diese Aktion zu
// verhindern hat, ist das Löschen ohne jedes Archiv.
//
// MEHRERE dagegen nicht. Auf `local` liegen neben dem frischen Dump
// die nächtlichen Sicherungen derselben VM; ohne Zeitpunkt lassen
// sie sich nicht ordnen, und vermerkt würde irgendeiner davon —
// womöglich der von vorgestern. Eine Reihe, die sich nicht sortieren
// lässt, ist keine Grundlage für ein Löschen.
if (count($eintraege) > 1) {
throw new RuntimeException(
'Auf "'.$storage.'" liegen '.count($eintraege).' Archive für VM '.$vmid
.', und keines nennt einen Zeitpunkt. Welches davon die Sicherung dieses Laufes '
.'ist, lässt sich nicht sagen; es wird nichts gelöscht.'
);
}
Log::warning('Abbau: das Archiv nennt keinen Zeitpunkt, sein Alter ist ungeprüft.', [
'vmid' => $vmid,
'storage' => $storage,
@ -440,15 +490,33 @@ class ArchiveAndTearDown
return $volid;
}
/** Der Grund gehört an den Datensatz, wo ein Betreiber ihn ohne Logdurchsicht findet. */
/**
* Der Grund gehört an den Datensatz, wo ein Betreiber ihn ohne
* Logdurchsicht findet.
*
* Beide Schritte einzeln abgesichert, und das ist kein Zierrat: diese
* Methode läuft IM `catch`. Ein Wurf von hier — eine Datenbank, die gerade
* nicht schreibt — ersetzte den gefangenen Fehlschlag und käme doch nach
* draussen, wo er im Zeitplan die übrigen Instanzen mitrisse.
*/
private function recordFailure(Instance $instance, Throwable $e): void
{
$instance->update(['teardown_error' => $e->getMessage()]);
try {
$instance->update(['teardown_error' => $e->getMessage()]);
} catch (Throwable) {
// Der Grund liess sich nicht vermerken. Mehr als der Eintrag unten
// geht dann nicht — und gelöscht hat dieser Lauf ohnehin nichts.
}
Log::error('Abbau einer gekündigten Instanz gestoppt.', [
'instance' => $instance->uuid,
'vmid' => $instance->vmid,
'reason' => $e->getMessage(),
]);
try {
Log::error('Abbau einer gekündigten Instanz gestoppt.', [
'instance' => $instance->uuid,
'vmid' => $instance->vmid,
'reason' => $e->getMessage(),
]);
} catch (Throwable) {
// Auch das Protokoll ist nicht erreichbar. Dann schweigt dieser
// eine Lauf, statt den ganzen Zeitplan mitzunehmen.
}
}
}

83
tests/Feature/Cancellation/ArchiveAndTearDownTest.php
View File

@ -6,6 +6,7 @@ use App\Models\Instance;
use App\Services\Proxmox\FakeProxmoxClient;
use App\Services\Proxmox\ProxmoxClient;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Cache;
/**
* Eine Instanz im Zustand NACH dem Laufzeitende, so wie ihn
@ -69,7 +70,8 @@ it('loescht auch dann nicht, wenn das Herunterfahren scheitert', function () {
expect(app(ArchiveAndTearDown::class)($instance))->toBeFalse()
->and($pve->backupCalls)->toBe([])
->and($pve->deletedVmids)->toBe([]);
->and($pve->deletedVmids)->toBe([])
->and($instance->fresh()->teardown_error)->not->toBeNull();
});
it('faellt nicht ueber eine Instanz, die schon abgebaut ist', function () {
@ -166,7 +168,8 @@ it('fährt gar nichts herunter, wenn die Ablage keine Sicherungen aufnimmt', fun
expect(app(ArchiveAndTearDown::class)($instance))->toBeFalse()
->and($pve->shutdownCalls)->toBe([])
->and($pve->backupCalls)->toBe([])
->and($pve->deletedVmids)->toBe([]);
->and($pve->deletedVmids)->toBe([])
->and($instance->fresh()->teardown_error)->toContain('local');
});
it('sichert nicht, wenn der Gast nach dem Herunterfahren weiterläuft', function () {
@ -227,6 +230,28 @@ it('merkt sich das frische Archiv, nicht das danebenliegende alte', function ()
->and($instance->fresh()->archived_at)->not->toBeNull();
});
it('löscht nicht gegen mehrere Archive, von denen keines einen Zeitpunkt nennt', function () {
// Auf `local` liegen neben dem frischen Dump die nächtlichen Sicherungen
// derselben VM. Fehlt allen der Zeitpunkt, lassen sie sich nicht ordnen —
// vermerkt würde irgendeines, womöglich das von vorgestern. Eine Reihe, die
// sich nicht sortieren lässt, ist keine Grundlage für ein Löschen. Ein
// EINZELNES Archiv ohne Zeitpunkt bleibt dagegen gültig; das ist der Fall
// der ersten Prüfung dieser Datei.
$pve = new FakeProxmoxClient;
$pve->backups['201'] = [
['volid' => 'local:backup/vzdump-qemu-201-a.vma.zst', 'size' => 4096],
['volid' => 'local:backup/vzdump-qemu-201-b.vma.zst', 'size' => 4096],
];
app()->instance(ProxmoxClient::class, $pve);
$instance = abgelaufeneInstanz(vmid: 201);
expect(app(ArchiveAndTearDown::class)($instance))->toBeFalse()
->and($pve->deletedVmids)->toBe([])
->and($instance->fresh()->torn_down_at)->toBeNull()
->and($instance->fresh()->archive_volid)->toBeNull()
->and($instance->fresh()->teardown_error)->not->toBeNull();
});
it('löscht nicht, wenn das Archiv keine Kennung trägt', function () {
// Ein Archiv, dessen Ort nicht notiert werden kann, ist in zwölf Monaten
// keins: niemand findet es wieder, und niemand räumt es je auf.
@ -375,6 +400,60 @@ it('trägt den Abbau nach, wenn die Maschine schon fort ist und das Archiv fests
->and($instance->fresh()->torn_down_at)->not->toBeNull();
});
it('bucht keine Maschine als abgebaut, wenn der Knoten gar nicht antwortet', function () {
// Die Eingangslage entsteht im Regelbetrieb: ein Lauf hat gesichert und ist
// am Löschen gescheitert, weil der Knoten hängt. Am nächsten Tag steht
// `archive_volid` am Datensatz, und `vmExists()` — ein GET ohne `->throw()` —
// antwortet wegen desselben hakenden Knotens mit „nein". Ohne ein
// Lebenszeichen davor würde hier eine LAUFENDE Kundenmaschine als abgebaut
// verbucht und danach nie wieder angesehen.
$pve = new class extends FakeProxmoxClient
{
public function nodeStorage(string $node): array
{
throw new RuntimeException('Proxmox: 595 Connection refused');
}
};
app()->instance(ProxmoxClient::class, $pve);
$instance = abgelaufeneInstanz(vmid: 201);
$instance->update([
'archive_volid' => 'local:backup/vzdump-qemu-201.vma.zst',
'archived_at' => now()->subDay(),
'teardown_error' => 'Proxmox: VM is locked (backup)',
]);
expect(app(ArchiveAndTearDown::class)($instance))->toBeFalse()
->and($pve->deletedVmids)->toBe([])
->and($instance->fresh()->torn_down_at)->toBeNull()
->and($instance->fresh()->teardown_error)->toContain('Connection refused');
});
it('lässt keinen zweiten Lauf über dieselbe Instanz', function () {
// Zwei Läufe nebeneinander — der Zeitplan und ein Betreiber von Hand —
// würden zweimal sichern und zweimal löschen; der zweite fände dabei eine
// Maschine vor, die der erste gerade wegnimmt. Hier hält bereits jemand die
// Sperre, und der Lauf hat nichts hinzuzufügen.
$pve = new FakeProxmoxClient;
$pve->backups['201'] = [['volid' => 'local:backup/vzdump-qemu-201.vma.zst', 'ctime' => now()->timestamp]];
app()->instance(ProxmoxClient::class, $pve);
$instance = abgelaufeneInstanz(vmid: 201);
$fremdeSperre = Cache::lock('instance-teardown:'.$instance->uuid, 60);
expect($fremdeSperre->get())->toBeTrue();
expect(app(ArchiveAndTearDown::class)($instance))->toBeFalse()
->and($pve->shutdownCalls)->toBe([])
->and($pve->backupCalls)->toBe([])
->and($pve->deletedVmids)->toBe([])
->and($instance->fresh()->torn_down_at)->toBeNull();
// Und die fremde Sperre steht noch: der abgewiesene Lauf gibt im `finally`
// nicht frei, was ihm nie gehört hat.
expect(Cache::lock('instance-teardown:'.$instance->uuid, 60)->get())->toBeFalse();
$fremdeSperre->release();
});
it('rührt eine Instanz ohne Maschine nicht an', function () {
// Ein Datensatz ohne VMID hat nichts, was sich sichern oder löschen ließe.
// Er wird nicht stillschweigend als abgebaut gebucht: das wäre ein Haken an