From 0bb61d39bd39ea27b1a87489e37943b0ccd864d5 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 1 Aug 2026 13:27:49 +0200 Subject: [PATCH] Codex-Runde 1: drei P1 an der Rueckfahrkarte, plus ein Gateway ohne via MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Die Ruecknahme konnte Erfolg melden, ohne einen zu haben. Drei Wege dorthin, alle behoben: - Sie schaltete den entmachteten Netzverwalter nicht wieder ein. Die Sicherung umfasst nur /etc/network/interfaces*; kam die Verbindung von cloud-init, networkd oder NetworkManager, spielte die Ruecknahme eine Datei zurueck, die die Maschine nie getragen hat, und liess den Verwalter abgeschaltet. Genau der tote Host, den der Zeitgeber verhindern soll. disown_network_manager hinterlaesst jetzt eine Notiz (WAS entmachtet, WELCHE Strophen verdraengt), die das Ruecknahme-Skript beim Feuern liest — aufgeschrieben statt eingebacken, weil der Zeitgeber vor dem Entmachten gestellt wird. - Sie setzte 'rolled-back' auch, wenn tar oder ifreload scheiterten. Das urspruengliche network.sh hatte dafuer set -e; beim Umbau ist es verlorengegangen. Jetzt bricht jeder Fehlschlag ab, bevor die Marke entsteht — CluPilot pollt dann bis zur Frist statt 'ist zurueck' zu glauben. - Eine verdraengte interfaces.d-Strophe wurde nur umbenannt. Der Stern in 'source interfaces.d/*' fasst sie weiter; das versteckte die Kollision vor dem Leser, statt sie zu loesen. Jetzt wandert sie aus dem Verzeichnis heraus, und die Ruecknahme holt sie zurueck. Dazu ein eigener Fund: das Gateway wurde mit awk '{print }' gelesen. Bei 'default dev ens3 scope link' ist das der KARTENNAME, woraus 'gateway ens3' in der Strophe wuerde. default_gateway() liest jetzt hinter dem via, und eine Standardroute ohne via wandert als eigene up-Zeile mit, statt verlorenzugehen. Zurueckgewiesen: der P2 zu extra_routes. 'ip route show dev X' laesst das dev-Feld WEG (im Container nachgemessen), das angehaengte 'dev vmbr0' ist also richtig. Co-Authored-By: Claude Opus 5 --- deploy/bootstrap/lib/bridge.sh | 123 +++++++++-- .../Feature/Provisioning/BridgeScriptTest.php | 196 +++++++++++++++--- 2 files changed, 282 insertions(+), 37 deletions(-) diff --git a/deploy/bootstrap/lib/bridge.sh b/deploy/bootstrap/lib/bridge.sh index d519bb5..f35db54 100644 --- a/deploy/bootstrap/lib/bridge.sh +++ b/deploy/bootstrap/lib/bridge.sh @@ -87,6 +87,24 @@ detect_primary_interface() { | awk '{ for (i = 1; i < NF; i++) if ($i == "dev") { print $(i+1); exit } }' } +# Das Gateway der Standardroute — hinter dem `via`, nie an fester Feldnummer. +# +# `default dev ens3 scope link` hat keins. Ein `awk '{ print $3 }'` lieferte dort +# den KARTENNAMEN, und daraus würde `gateway ens3` in der Strophe: `ifreload` +# scheitert, der Zeitgeber räumt ab — und abgeräumt wird ein Host, der zu bauen +# gewesen wäre. Leer heißt hier ehrlich leer. +default_gateway() { + "$CLUPILOT_IP" -4 route show default 2>/dev/null \ + | awk '{ for (i = 1; i < NF; i++) if ($i == "via") { print $(i+1); exit } }' +} + +# Gibt es überhaupt eine Standardroute? Ohne `via` ist sie eine Link-Route, und +# die muss die Brücke als eigene `up`-Zeile mitnehmen, sonst verliert der Host +# seinen Weg nach draußen. +has_default_route() { + "$CLUPILOT_IP" -4 route show default 2>/dev/null | grep -q . +} + # Ist das eine physische Karte? # # `bridge_ports` auf einem Bond oder einer bestehenden Bridge ist falsch — die @@ -140,9 +158,16 @@ detect_network_style() { fi _cidr="$("$CLUPILOT_IP" -4 -o addr show dev "$_iface" scope global 2>/dev/null | awk '{ print $4; exit }')" - _gw="$("$CLUPILOT_IP" -4 route show default 2>/dev/null | awk '{ print $3; exit }')" + _gw="$(default_gateway)" _prefix="${_cidr##*/}" + # Kein Gateway: dann gibt es weder eins im eigenen Subnetz noch eines + # außerhalb, und `routed` zu melden hieße `pointopoint` auf nichts. + if [ -z "$_gw" ]; then + printf 'subnet' + return 0 + fi + # /32 heißt: das eigene Subnetz besteht aus der eigenen Adresse. Ein Gateway # darin kann es nicht geben. if [ "$_prefix" = '32' ]; then @@ -247,6 +272,14 @@ write_bridge_stanza() { _ip4="${_cidr%%/*}" _mac="$(cat "${CLUPILOT_SYS_NET}/${_iface}/address" 2>/dev/null)" + # Ohne Gateway gibt es kein pointopoint-Ziel. Nach detect_network_style kann + # das nicht mehr vorkommen — write_bridge_stanza wird aber auch direkt + # aufgerufen, und `pointopoint` auf einen leeren Wert ist eine Strophe, die + # `ifreload` ablehnt. + if [ -z "$_gw" ] && [ "$_style" = 'routed' ]; then + _style='subnet' + fi + case "$_style" in dhcp) _inet="iface ${CLUPILOT_BRIDGE} inet dhcp" @@ -263,8 +296,17 @@ write_bridge_stanza() { ;; *) _inet="iface ${CLUPILOT_BRIDGE} inet static" - _addr=" address ${_cidr} + _addr=" address ${_cidr}" + if [ -n "$_gw" ]; then + _addr="${_addr} gateway ${_gw}" + elif has_default_route; then + # Standardroute ohne `via` — eine Link-Route. Sie hat kein + # Gateway, das in eine `gateway`-Zeile passte, und ginge sonst + # beim Umbau verloren. + _addr="${_addr} + up ip route add default dev ${CLUPILOT_BRIDGE} || true" + fi ;; esac @@ -351,8 +393,22 @@ foreign_network_manager() { } # Entmachtet den erkannten Verwalter — im gesicherten Stand, also unter dem -# Zeitgeber. Was hier schiefgeht, holt er zurück. +# Zeitgeber. +# +# Und hinterlässt, WAS entmachtet wurde. Das ist die Hälfte, die zuerst fehlte: +# die Sicherung umfasst nur `/etc/network/interfaces*`, also könnte die Rücknahme +# einen abgeschalteten cloud-init nicht wieder einschalten. Auf einer Maschine, +# deren Verbindung von cloud-init kam, spielte sie damit eine Datei zurück, die +# die Maschine nie getragen hat — und ließe den Verwalter, der es tat, +# abgeschaltet. Genau der tote Host, den der Zeitgeber verhindern soll. +# +# Aufgeschrieben statt ins Rücknahme-Skript gebacken, weil der Zeitgeber VOR +# dieser Funktion gestellt wird. Das Skript liest die Notiz erst, wenn es feuert. disown_network_manager() { + mkdir -p "$CLUPILOT_WORK_DIR" + printf '%s' "${1:-}" > "${CLUPILOT_WORK_DIR}/disowned" + : > "${CLUPILOT_WORK_DIR}/moved-aside" + case "${1:-}" in cloud-init) mkdir -p "${CLUPILOT_ETC}/cloud/cloud.cfg.d" @@ -377,16 +433,21 @@ disown_network_manager() { # Der kleinere Verwandte: die Strophe behält `source interfaces.d/*`. Bleibt # dort eine Datei liegen, die dieselbe Karte beansprucht, hat der Host zwei # Stellen, die seine Adresse vergeben. + # + # HERAUS aus dem Verzeichnis, nicht bloß umbenannt. Der Stern in + # `source .../interfaces.d/*` fasst auch eine umbenannte Datei — Umbenennen + # allein löst die Kollision also nicht, es versteckt sie nur vor dem Leser. _iface="${2:-}" + _aside="${CLUPILOT_WORK_DIR}/verdraengt" + if [ -n "$_iface" ] && [ -d "$CLUPILOT_INTERFACES_D" ]; then for _f in "$CLUPILOT_INTERFACES_D"/*; do [ -f "$_f" ] || continue - case "$_f" in - *.von-clupilot-beiseitegelegt) continue ;; - esac if grep -qE "iface[[:space:]]+${_iface}[[:space:]]" "$_f" 2>/dev/null; then - mv "$_f" "${_f}.von-clupilot-beiseitegelegt" - log "Kollidierende Strophe beiseitegelegt: ${_f}" + mkdir -p "$_aside" + mv "$_f" "${_aside}/$(basename "$_f")" || continue + printf '%s\n' "$_f" >> "${CLUPILOT_WORK_DIR}/moved-aside" + log "Kollidierende Strophe aus dem Quellverzeichnis genommen: ${_f}" fi done fi @@ -517,19 +578,57 @@ render_rollback_script() { # Netzkonfiguration von VOR der Bruecke zurueck, weil sie binnen ${_minutes} # Minuten nicht abbestellt wurde — und abbestellen kann CluPilot nur, wenn es # den Host ueber den Tunnel wieder erreicht. +# Das Urteil zuerst, damit auch ein halb gegluecktes Zurueckspielen einen Grund +# hinterlaesst. printf 'failed' > '${CLUPILOT_WORK_DIR}/state' printf 'Die Bruecke nahm den Host vom Netz; die Netzkonfiguration von vorher wurde zurueckgespielt.' > '${CLUPILOT_WORK_DIR}/note' -tar xzf '${CLUPILOT_NET_BACKUP}.tar.gz' -C / +# Zuerst den entmachteten Netzverwalter wieder einschalten. Die Sicherung +# umfasst nur /etc/network/interfaces* — kam die Verbindung dieser Maschine von +# cloud-init, networkd oder NetworkManager, hilft das Zurueckspielen allein +# nichts, solange der Verwalter abgeschaltet bleibt. +_m="\$(cat '${CLUPILOT_WORK_DIR}/disowned' 2>/dev/null)" +case "\$_m" in + cloud-init) + rm -f '${CLUPILOT_ETC}/cloud/cloud.cfg.d/99-clupilot-disable-network.cfg' + ;; + networkd) + systemctl unmask systemd-networkd 2>/dev/null || true + systemctl enable --now systemd-networkd.socket systemd-networkd 2>/dev/null || true + ;; + network-manager) + systemctl unmask NetworkManager 2>/dev/null || true + systemctl enable --now NetworkManager 2>/dev/null || true + ;; +esac + +# Verdraengte Strophen zurueck an ihren Platz. +if [ -f '${CLUPILOT_WORK_DIR}/moved-aside' ]; then + while IFS= read -r _orig; do + [ -n "\$_orig" ] || continue + _base="\$(basename "\$_orig")" + if [ -f '${CLUPILOT_WORK_DIR}/verdraengt/'"\$_base" ]; then + mv '${CLUPILOT_WORK_DIR}/verdraengt/'"\$_base" "\$_orig" + fi + done < '${CLUPILOT_WORK_DIR}/moved-aside' +fi + +# Ab hier zaehlt jeder Fehlschlag. Ohne diese Pruefungen liefe das Skript weiter +# und setzte die Marke, obwohl der Host auf einer kaputten Konfiguration steht — +# und CluPilot meldete 'der alte Zustand ist zurueck' und gaebe auf. +tar xzf '${CLUPILOT_NET_BACKUP}.tar.gz' -C / || exit 1 + if command -v ifreload >/dev/null 2>&1; then - ifreload -a + ifreload -a || exit 1 else - systemctl restart networking + systemctl restart networking || exit 1 fi logger -t clupilot 'Netzkonfiguration zurueckgespielt: die Bruecke hat den Host vom Netz genommen.' -# Erst jetzt. Vorher hiesse es: zurueckgespielt, obwohl es noch laeuft. +# Erst jetzt, und nur nach einem geglueckten Zurueckspielen. Die Marke ist das +# Signal, auf das CluPilot wartet, um aufzugeben — sie darf nie 'zurueckgespielt' +# sagen, wenn es nicht stimmt. : > '${CLUPILOT_WORK_DIR}/rolled-back' # Selbst wegraeumen, sonst liegen Unit-Dateien herum, die aussehen, als stuende diff --git a/tests/Feature/Provisioning/BridgeScriptTest.php b/tests/Feature/Provisioning/BridgeScriptTest.php index f9fa15c..81b749b 100644 --- a/tests/Feature/Provisioning/BridgeScriptTest.php +++ b/tests/Feature/Provisioning/BridgeScriptTest.php @@ -196,6 +196,58 @@ it('sagt nur dann „Brücke steht", wenn sie Route UND Adresse trägt', functio expect($out)->toBe('NEIN'); }); +it('liest kein Gateway aus einer Standardroute, die keins hat', function () { + // `default dev ens3 scope link` — kein `via`. Ein `awk '{print $3}'` liefert + // dort den KARTENNAMEN, und daraus würde `gateway ens3` in der Strophe. + // Der Zeitgeber finge das ab, aber der Host wäre umsonst gescheitert. + $ip = fakeIp($this->dir, [ + '-4 route show default' => 'default dev ens3 scope link', + ]); + + expect(runBridgeSh('default_gateway', ['CLUPILOT_IP' => $ip]))->toBe(''); +}); + +it('liest das Gateway hinter dem via, nicht an fester Feldnummer', function () { + $ip = fakeIp($this->dir, [ + '-4 route show default' => 'default via 10.0.0.1 dev ens3 proto dhcp src 10.0.0.7 metric 100', + ]); + + expect(runBridgeSh('default_gateway', ['CLUPILOT_IP' => $ip]))->toBe('10.0.0.1'); +}); + +it('hält eine gatewaylose Karte nicht für eine geroutete Einzeladresse', function () { + // Ohne Gateway gibt es weder eins im eigenen Subnetz noch eines ausserhalb. + // `routed` zu melden hiesse `pointopoint` auf nichts. + $ip = fakeIp($this->dir, [ + '-4 route show default' => 'default dev ens3 scope link', + '-4 -o addr show dev ens3 scope global' => '2: ens3 inet 10.0.0.7/24 scope global ens3', + ]); + + expect(runBridgeSh('detect_network_style ens3', ['CLUPILOT_IP' => $ip])) + ->toBe('subnet'); +}); + +it('schreibt ohne Gateway auch keine gateway-Zeile', function () { + $ip = fakeIp($this->dir, [ + '-4 route show dev ens3' => '', + '-6 -o addr show dev ens3 scope global' => '', + '-6 route show default' => '', + ]); + $sys = fakeSysNet($this->dir, 'ens3', 'de:ad:be:ef:00:07'); + $out = $this->dir.'/interfaces'; + + runBridgeSh('write_bridge_stanza ens3 subnet 10.0.0.7/24 ""', [ + 'CLUPILOT_IP' => $ip, + 'CLUPILOT_SYS_NET' => $sys, + 'CLUPILOT_INTERFACES_FILE' => $out, + ]); + + expect(file_get_contents($out)) + ->toContain('address 10.0.0.7/24') + ->not->toContain('gateway') + ->not->toContain('pointopoint'); +}); + it('hält jede Brückenfunktion an genau einer Stelle im Repo', function () { // Zwei Fassungen wären zwei Installationen, die bei jeder Proxmox-Version // nachgezogen werden müssten — und die zweite fiele erst auf, wenn jemand @@ -450,6 +502,119 @@ it('schreibt ein Rücknahme-Skript, das den Grund festhält BEVOR es zurückspie ->and($doneAt)->toBeGreaterThan($restoreAt); }); +it('legt die Marke NICHT an, wenn das Zurückspielen scheitert', function () { + // Ohne diese Prüfung laufen tar und ifreload ins Leere, das Skript macht + // weiter und setzt `rolled-back` — CluPilot meldet dann „der alte Zustand + // ist zurück" und gibt auf, während der Host auf einer kaputten + // Konfiguration steht. Das ursprüngliche network.sh hatte dafür `set -e`; + // beim Umbau ist es verlorengegangen. + $work = $this->dir.'/work'; + mkdir($work, 0o755, true); + + $script = runBridgeSh('render_rollback_script 5', [ + 'CLUPILOT_WORK_DIR' => $work, + // Ein Archiv, das es nicht gibt: tar scheitert. + 'CLUPILOT_NET_BACKUP' => $this->dir.'/gibtesnicht', + 'CLUPILOT_UNIT_DIR' => $this->dir.'/units', + ]); + file_put_contents($this->dir.'/rollback.sh', $script); + + $result = Process::run('sh '.$this->dir.'/rollback.sh'); + + expect($result->exitCode())->not->toBe(0) + ->and(file_exists($work.'/rolled-back'))->toBeFalse() + // Das Urteil steht trotzdem — es wird VOR dem Zurückspielen geschrieben. + ->and(file_get_contents($work.'/state'))->toBe('failed'); +}); + +it('schaltet die Rücknahme den entmachteten Netzverwalter wieder ein', function () { + // Sonst spielt sie eine /etc/network/interfaces zurück, die diese Maschine + // nie getragen hat, während der Verwalter, der es tat, abgeschaltet bleibt. + // Genau der tote Host, den der Zeitgeber verhindern soll. + $work = $this->dir.'/work'; + $etc = $this->dir.'/etc'; + mkdir($work, 0o755, true); + mkdir("{$etc}/cloud/cloud.cfg.d", 0o755, true); + file_put_contents("{$etc}/cloud/cloud.cfg", "datasource_list: [ Hetzner ]\n"); + + $env = [ + 'CLUPILOT_WORK_DIR' => $work, + 'CLUPILOT_ETC' => $etc, + 'CLUPILOT_NET_BACKUP' => $this->dir.'/sicherung', + 'CLUPILOT_UNIT_DIR' => $this->dir.'/units', + 'CLUPILOT_INTERFACES_D' => $this->dir.'/interfaces.d', + ]; + + // Erst entmachten, dann das Skript fahren, das der Zeitgeber gestellt hat. + runBridgeSh('disown_network_manager cloud-init ens3', $env); + expect(file_exists("{$etc}/cloud/cloud.cfg.d/99-clupilot-disable-network.cfg"))->toBeTrue(); + + // Eine Sicherung, damit tar nicht vorher scheitert. + file_put_contents($this->dir.'/leer', "x\n"); + Process::run('tar czf '.$this->dir.'/sicherung.tar.gz -C '.$this->dir.' leer'); + + $script = runBridgeSh('render_rollback_script 5', $env); + file_put_contents($this->dir.'/rollback.sh', $script); + Process::run('sh '.$this->dir.'/rollback.sh'); + + expect(file_exists("{$etc}/cloud/cloud.cfg.d/99-clupilot-disable-network.cfg"))->toBeFalse(); +}); + +it('nimmt eine verdrängte Strophe aus dem Quellverzeichnis heraus', function () { + // `source /etc/network/interfaces.d/*` fasst auch eine umbenannte Datei — + // der Stern nimmt sie mit. Umbenennen allein löst die Kollision also nicht. + $etc = $this->dir.'/etc'; + $d = $this->dir.'/interfaces.d'; + $work = $this->dir.'/work'; + mkdir($etc, 0o755, true); + mkdir($d, 0o755, true); + mkdir($work, 0o755, true); + file_put_contents("{$d}/50-cloud-init", "auto ens3\niface ens3 inet static\n address 10.0.0.7/24\n"); + file_put_contents("{$d}/99-egal", "iface eth9 inet manual\n"); + + runBridgeSh('disown_network_manager "" ens3', [ + 'CLUPILOT_ETC' => $etc, + 'CLUPILOT_INTERFACES_D' => $d, + 'CLUPILOT_WORK_DIR' => $work, + ]); + + // Nichts, was der Stern noch fassen könnte. + expect(glob("{$d}/*"))->toBe(["{$d}/99-egal"]) + ->and(file_exists("{$work}/verdraengt/50-cloud-init"))->toBeTrue() + // Und die Rücknahme muss wissen, wohin damit. + ->and(file_get_contents("{$work}/moved-aside"))->toContain("{$d}/50-cloud-init"); +}); + +it('stellt die Rücknahme eine verdrängte Strophe zurück', function () { + $etc = $this->dir.'/etc'; + $d = $this->dir.'/interfaces.d'; + $work = $this->dir.'/work'; + mkdir($etc, 0o755, true); + mkdir($d, 0o755, true); + mkdir($work, 0o755, true); + file_put_contents("{$d}/50-cloud-init", "iface ens3 inet static\n"); + + $env = [ + 'CLUPILOT_ETC' => $etc, + 'CLUPILOT_INTERFACES_D' => $d, + 'CLUPILOT_WORK_DIR' => $work, + 'CLUPILOT_NET_BACKUP' => $this->dir.'/sicherung', + 'CLUPILOT_UNIT_DIR' => $this->dir.'/units', + ]; + + runBridgeSh('disown_network_manager "" ens3', $env); + expect(file_exists("{$d}/50-cloud-init"))->toBeFalse(); + + file_put_contents($this->dir.'/leer', "x\n"); + Process::run('tar czf '.$this->dir.'/sicherung.tar.gz -C '.$this->dir.' leer'); + + $script = runBridgeSh('render_rollback_script 5', $env); + file_put_contents($this->dir.'/rollback.sh', $script); + Process::run('sh '.$this->dir.'/rollback.sh'); + + expect(file_exists("{$d}/50-cloud-init"))->toBeTrue(); +}); + it('räumt der Zeitgeber sich nach dem Feuern selbst weg', function () { // Sonst liegen Unit-Dateien herum, die aussehen, als stünde noch eine // Rücknahme aus. @@ -754,34 +919,16 @@ it('entmachtet cloud-init im gesicherten Stand', function () { mkdir("{$etc}/cloud", 0o755, true); file_put_contents("{$etc}/cloud/cloud.cfg", "datasource_list: [ Hetzner ]\n"); - runBridgeSh('disown_network_manager cloud-init ens3', ['CLUPILOT_ETC' => $etc]); + runBridgeSh('disown_network_manager cloud-init ens3', [ + 'CLUPILOT_ETC' => $etc, + // Die Notiz, aus der die Rücknahme später liest, WAS entmachtet wurde. + 'CLUPILOT_WORK_DIR' => $this->dir.'/work', + ]); expect(file_get_contents("{$etc}/cloud/cloud.cfg.d/99-clupilot-disable-network.cfg")) ->toContain('network: {config: disabled}'); }); -it('legt eine kollidierende interfaces.d-Strophe beiseite', function () { - // Die Strophe behält `source interfaces.d/*`. Bleibt dort eine Datei - // liegen, die dieselbe Karte beansprucht, hat der Host zwei Stellen, die - // seine Adresse vergeben. - $etc = $this->dir.'/etc'; - $d = $this->dir.'/interfaces.d'; - mkdir($etc, 0o755, true); - mkdir($d, 0o755, true); - file_put_contents("{$d}/50-cloud-init", "auto ens3\niface ens3 inet static\n address 10.0.0.7/24\n"); - file_put_contents("{$d}/99-egal", "iface eth9 inet manual\n"); - - runBridgeSh('disown_network_manager "" ens3', [ - 'CLUPILOT_ETC' => $etc, - 'CLUPILOT_INTERFACES_D' => $d, - ]); - - expect(file_exists("{$d}/50-cloud-init"))->toBeFalse() - ->and(file_exists("{$d}/50-cloud-init.von-clupilot-beiseitegelegt"))->toBeTrue() - // Eine Strophe für eine ANDERE Karte bleibt, wo sie ist. - ->and(file_exists("{$d}/99-egal"))->toBeTrue(); -}); - /** * Baut eine Sandkiste, in der bridge-run.sh wirklich durchläuft. * @@ -797,7 +944,7 @@ function bridgeSandbox(string $dir, bool $bridgeComesUp = true, int $handshakeAg $work = "{$dir}/work"; $bin = "{$dir}/bin"; $state = "{$dir}/state"; - foreach ([$work, $bin, $state, "{$dir}/units", "{$dir}/sbin"] as $d) { + foreach ([$work, $bin, $state, "{$dir}/units", "{$dir}/sbin", "{$dir}/etc"] as $d) { mkdir($d, 0o755, true); } @@ -857,7 +1004,6 @@ function bridgeSandbox(string $dir, bool $bridgeComesUp = true, int $handshakeAg ])); fakeSysNet($dir, 'enp0s31f6', 'a8:a1:59:00:11:22'); - mkdir("{$dir}/etc", 0o755, true); return ['work' => $work, 'calls' => "{$dir}/calls"]; }