diff --git a/app/Provisioning/Steps/Host/EnsureNetworkBridge.php b/app/Provisioning/Steps/Host/EnsureNetworkBridge.php index 7a2d022..f066b7f 100644 --- a/app/Provisioning/Steps/Host/EnsureNetworkBridge.php +++ b/app/Provisioning/Steps/Host/EnsureNetworkBridge.php @@ -106,7 +106,19 @@ class EnsureNetworkBridge extends HostStep // Already there means touch nothing. The same shortcut as "the template // exists and reports template: 1" — and here it matters more, because // rebuilding a working network is the one thing that can end this badly. + // + // It is also the success path. Reaching this line means SSH arrived over + // the tunnel AND vmbr0 carries the default route with an address — that + // is bridge_proven, asked from the outside, which is the strongest form + // of the question there is. Only now is the timer thrown away, and only + // if this run is the one that armed it: without a deadline in the + // context, those unit files belong to somebody else. if ($state['up']) { + if ($run->context('bridge_deadline') !== null) { + $this->cancelRollback(); + $run->forgetContext('bridge_deadline'); + } + return StepResult::advance(); } @@ -314,17 +326,103 @@ class EnsureNetworkBridge extends HostStep return (string) file_get_contents(base_path('deploy/bootstrap/'.$relative)); } - /** @param array{state:string, alive:string, phase:string, note:string, rolledback:string} $status */ + /** + * Wait for the rollback to complete, and only then fail. + * + * Not immediately: while the timer is still pending, the machine is in the + * middle of a change. A fail() would leave it there, and the next person to + * look would find half a host with no explanation of why. + * + * The rollback script writes `failed` plus a reason BEFORE it restores, so + * even a partial restore leaves a verdict, and touches `rolled-back` as its + * LAST action. That file is the signal: it is only there once the old + * configuration is genuinely back. + * + * The wait is capped by the step's deadline, which is three times the + * timer's fuse — so this cannot poll forever. + * + * @param array{state:string, alive:string, phase:string, note:string, rolledback:string} $status + */ private function awaitRollback(ProvisioningRun $run, array $status, string $reason): StepResult { - return $this->giveUp($run, $reason); + $deadline = $run->context('bridge_deadline'); + $expired = $deadline !== null && now()->greaterThan(Carbon::parse($deadline)); + + if ($status['rolledback'] !== 'yes' && ! $expired) { + return StepResult::poll(20, 'die Brücke trägt nicht — warte auf die Rücknahme des Zeitgebers'); + } + + return $this->giveUp( + $run, + $reason.' The rollback timer has put the previous network configuration back'. + ($status['rolledback'] === 'yes' + ? '' + : ' (or is about to — its fuse is '.self::ROLLBACK_MINUTES.' minutes)'). + ', so the machine is reachable on its old settings.' + ); } + /** + * Throw the return ticket away — and never before the bridge has been + * checked. Reaching the caller means SSH arrived over the tunnel AND vmbr0 + * carries the default route with an address, which is bridge_proven asked + * from the outside, the strongest form of the question there is. + */ + private function cancelRollback(): void + { + $this->shell->run(implode("\n", [ + ': clupilot-bridge-cancel', + 'U=clupilot-network-rollback', + 'systemctl stop "$U.timer" 2>/dev/null || true', + 'rm -f "/etc/systemd/system/$U.timer" "/etc/systemd/system/$U.service" "/usr/local/sbin/$U.sh"', + 'systemctl daemon-reload 2>/dev/null || true', + ])); + } + + /** + * Fail, and leave nothing behind that would make a retry read this verdict + * again — the status on the host and the deadline in the run, which is what + * execute() reads to tell a first visit from a lost one. + * + * `bridge_attempts` deliberately SURVIVES: it is the cap on how often + * CluPilot will drive a live host's network into a rollback, and clearing it + * here would make that cap meaningless. + * + * The rollback timer is deliberately NOT cancelled. It is the return ticket; + * if it is still armed, that is because nobody has proven the host came back + * — and cancelling it while giving up would leave the machine in exactly the + * state the timer exists for. + */ private function giveUp(ProvisioningRun $run, string $reason): StepResult { $run->forgetContext('bridge_deadline'); - return StepResult::fail($reason); + $tail = trim($this->shell->run( + ': clupilot-bridge-log'."\n".'tail -n 25 '.self::WORK_DIR.'/bridge.log 2>/dev/null' + )->stdout); + + // Stop the driver BEFORE the status files go, never after. Clearing them + // makes the step retryable, and a retry meeting a driver still running + // gets two of them on one /etc/network/interfaces, each with a timer of + // its own — one cancelling what the other armed. + // + // The whole process group (`-$P`), because the driver spends its time + // waiting on an ifreload. The cmdline guard is against a recycled pid: + // this file can be minutes old, and killing a stranger's process group + // would be a far worse bug than the one being handled. + $this->shell->run(implode("\n", [ + ': clupilot-bridge-reset', + 'W='.self::WORK_DIR, + 'P=$(cat "$W/pid" 2>/dev/null)', + 'if [ -n "$P" ] && kill -0 "$P" 2>/dev/null && tr "\\0" " " < /proc/"$P"/cmdline 2>/dev/null | grep -q bridge-run; then', + ' kill -TERM -"$P" 2>/dev/null || kill -TERM "$P" 2>/dev/null', + ' i=0; while [ "$i" -lt 10 ] && kill -0 "$P" 2>/dev/null; do sleep 1; i=$((i + 1)); done', + ' kill -KILL -"$P" 2>/dev/null || kill -KILL "$P" 2>/dev/null', + 'fi', + 'rm -f "$W/state" "$W/pid" "$W/note" "$W/rolled-back"', + ])); + + return StepResult::fail($tail === '' ? $reason : $reason.' Last lines: '.$tail); } /** diff --git a/tests/Feature/Provisioning/HostStepsTest.php b/tests/Feature/Provisioning/HostStepsTest.php index cee5a8b..14e7979 100644 --- a/tests/Feature/Provisioning/HostStepsTest.php +++ b/tests/Feature/Provisioning/HostStepsTest.php @@ -1813,3 +1813,170 @@ it('startet keinen zweiten Treiber, solange der erste lebt', function () { ->and($result->reason)->toContain('umstellen') ->and($s['shell']->ran('clupilot-bridge-start'))->toBeFalse(); }); + +it('bestellt den Zeitgeber erst ab, nachdem die Brücke nachgeprüft ist', function () { + // Reihenfolge, nicht Geschmack. Abbestellen heißt: die Rückfahrkarte + // wegwerfen. Wer das vor der Nachprüfung tut, tut es auf Verdacht. + // + // Dass dieser Zweig überhaupt erreicht wird, IST der Beweis: SSH kam über + // den Tunnel an, und vmbr0 trägt Standardroute und Adresse. Das ist + // bridge_proven, von außen gefragt. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, ['bridge_deadline' => now()->addMinutes(10)->toIso8601String()]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: true); + + $result = app(EnsureNetworkBridge::class)->execute($run); + + $stateAt = $cancelAt = null; + foreach ($s['shell']->recorded() as $i => $command) { + if ($stateAt === null && str_contains($command, 'clupilot-bridge-state')) { + $stateAt = $i; + } + if ($cancelAt === null && str_contains($command, 'clupilot-bridge-cancel')) { + $cancelAt = $i; + } + } + + expect($result->type)->toBe('advance') + ->and($stateAt)->not->toBeNull() + ->and($cancelAt)->not->toBeNull() + ->and($stateAt)->toBeLessThan($cancelAt) + // Und der Termin ist weg, sonst liest ein späterer Besuch ihn als + // „mitten in einer Umstellung". + ->and($run->fresh()->context('bridge_deadline'))->toBeNull(); +}); + +it('bestellt nichts ab, wenn die Brücke schon vor diesem Lauf da war', function () { + // Ohne Termin hat dieser Lauf nie einen Zeitgeber gestellt. Ein `stop` und + // ein `rm` auf fremde Unit-Dateien wären ein Eingriff ohne Anlass. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: true); + + expect(app(EnsureNetworkBridge::class)->execute($run)->type)->toBe('advance') + ->and($s['shell']->ran('clupilot-bridge-cancel'))->toBeFalse(); +}); + +it('wartet auf die Rücknahme, statt einen halb umgestellten Host liegen zu lassen', function () { + // Solange der Zeitgeber aussteht, steckt die Maschine mitten in einer + // Umstellung. Ein fail() ließe sie dort liegen, und der Nächste, der + // hinsieht, fände einen halben Host ohne Erklärung. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, ['bridge_deadline' => now()->addMinutes(10)->toIso8601String()]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + scriptBridgeStatus($s['shell'], 'failed', note: 'Tunnel steht nicht', rolledBack: false); + + $result = app(EnsureNetworkBridge::class)->execute($run); + + expect($result->type)->toBe('poll') + // Der Termin bleibt stehen, sonst gilt der nächste Besuch als erster und + // startet einen zweiten Treiber. + ->and($run->fresh()->context('bridge_deadline'))->not->toBeNull(); +}); + +it('scheitert mit dem Grund des Treibers, sobald die Rücknahme durch ist', function () { + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, ['bridge_deadline' => now()->addMinutes(10)->toIso8601String()]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + scriptBridgeStatus($s['shell'], 'failed', note: 'Tunnel steht nicht', rolledBack: true); + + $result = app(EnsureNetworkBridge::class)->execute($run); + + expect($result->type)->toBe('fail') + ->and($result->reason)->toContain('Tunnel steht nicht') + ->and($run->fresh()->context('bridge_deadline'))->toBeNull() + ->and($s['shell']->ran('clupilot-bridge-reset'))->toBeTrue(); +}); + +it('bestellt den Zeitgeber beim Aufgeben NICHT ab', function () { + // Er ist die Rückfahrkarte. Steht er noch, hat er seinen Grund — und wer ihn + // beim Aufgeben abbestellt, lässt den Host genau in dem Zustand stehen, für + // den er gestellt wurde. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, ['bridge_deadline' => now()->subMinute()->toIso8601String()]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + scriptBridgeStatus($s['shell'], 'running', alive: true, phase: 'nachsehen'); + + expect(app(EnsureNetworkBridge::class)->execute($run)->type)->toBe('fail') + ->and($s['shell']->ran('clupilot-bridge-cancel'))->toBeFalse(); +}); + +it('versucht es höchstens zweimal je Lauf', function () { + // Der zweite ist für den Fall, dass der Betreiber zwischen den Versuchen + // etwas repariert hat. Danach die Handarbeit-Meldung — ein dritter Versuch + // gegen dieselbe Maschine wäre nur dieselbe Viertelstunde noch einmal. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, ['bridge_attempts' => 2]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + + $result = app(EnsureNetworkBridge::class)->execute($run); + + expect($result->type)->toBe('fail') + ->and($result->reason)->toContain('by hand') + ->and($s['shell']->ran('clupilot-bridge-start'))->toBeFalse(); +}); + +it('erlaubt einem Betreiber, nach einem Fehlschlag von vorn anzufangen', function () { + // giveUp() löscht den Termin — sonst macht die Erst-Besuch-Prüfung aus jedem + // Retry nach einem Fehlschlag einen sofortigen zweiten Fehlschlag, ohne dass + // der Host überhaupt angefasst wurde. Der ZÄHLER bleibt dagegen stehen, + // sonst ist der Deckel von zwei Versuchen keiner. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + $run = hostRun($host, [ + 'bridge_deadline' => now()->addMinutes(10)->toIso8601String(), + 'bridge_attempts' => 1, + ]); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + scriptBridgeStatus($s['shell'], 'failed', note: 'irgendwas', rolledBack: true); + + app(EnsureNetworkBridge::class)->execute($run); + + expect($run->fresh()->context('bridge_deadline'))->toBeNull() + ->and($run->fresh()->context('bridge_attempts'))->toBe(1); +}); + +it('schickt nur Shell, die eine Shell auch parsen kann', function () { + // FakeRemoteShell führt keinen dieser Befehle aus. Ein verrutschtes + // Anführungszeichen käme sonst zuerst auf einem echten Host zur Ausführung — + // und der Schritt, den es kaputtmacht, ist der, der das Netz umstellt. + $s = fakeServices(); + $host = Host::factory()->active()->create(); + + $run = hostRun($host); + proveTunnel($run, $host); + scriptBridgeState($s['shell'], up: false); + app(EnsureNetworkBridge::class)->execute($run); // state + start + + scriptBridgeStatus($s['shell'], 'failed', note: 'nope', rolledBack: true); + app(EnsureNetworkBridge::class)->execute( + hostRun($host, ['bridge_deadline' => now()->addMinutes(10)->toIso8601String()]) + ); // state + status + log + reset + + scriptBridgeState($s['shell'], up: true); + app(EnsureNetworkBridge::class)->execute( + hostRun($host, ['bridge_deadline' => now()->addMinutes(10)->toIso8601String()]) + ); // state + cancel + + $checked = 0; + foreach ($s['shell']->recorded() as $command) { + expect(Process::input($command)->run('sh -n')->successful()) + ->toBeTrue("kein gültiges POSIX-sh:\n".$command); + $checked++; + } + + expect($checked)->toBeGreaterThanOrEqual(6); +});