diff --git a/script/proxmox/proxmox_deploy.py b/script/proxmox/proxmox_deploy.py index 12d1be2..6893f74 100644 --- a/script/proxmox/proxmox_deploy.py +++ b/script/proxmox/proxmox_deploy.py @@ -426,6 +426,69 @@ def pick_bridge(bridges, voulu: str = "") -> str: INTERNAL_BRIDGE = "vmbr0" INTERNAL_CIDR = "10.10.10.1/24" +# Le réseau interne ne peut PAS être une constante : un Proxmox dans un +# Proxmox hérite du réseau interne de son parent, et 10.10.10.1 y est +# l'adresse de sa propre PASSERELLE. La poser sur son pont rend tout le /24 +# local — la passerelle devient injoignable et la machine s'isole +# instantanément, au milieu de la commande qui la configure. Vécu : « ifup » +# n'a jamais rendu la main et la VM ne répondait plus, ni en ssh ni en ping. +# +# On choisit donc un /24 que l'hôte ne connaît pas encore. La liste va du plus +# attendu au plus improbable : un parc imbriqué descend d'un cran par étage. +INTERNAL_CANDIDATES = ( + "10.10.10.1/24", + "10.10.20.1/24", + "10.10.30.1/24", + "10.10.40.1/24", + "10.20.10.1/24", + "10.30.10.1/24", + "172.31.10.1/24", + "192.168.210.1/24", +) + +# Tout ce que l'hôte sait déjà d'IPv4 : ses adresses ET ses routes. Les deux, +# parce qu'une route sans adresse locale suffit à créer le conflit — la route +# par défaut « via 10.10.10.1 » en est l'exemple exact. +USED_NETS_CMD = "ip -o -4 addr show; ip -4 route show" + + +def parse_used_nets(text: str) -> set: + """Réseaux IPv4 lus dans la sortie de USED_NETS_CMD. + + Une adresse nue compte pour un /32 : c'est honnête, et le + chevauchement avec un /24 candidat se calcule pareil. Un préfixe plus + large qu'un /24 — « 10.0.0.0/8 » — écarte donc bien tous nos candidats + en 10.x, ce qu'un test sur les trois premiers octets aurait raté.""" + import ipaddress + + nets = set() + motif = r"\b(\d{1,3}(?:\.\d{1,3}){3})(?:/(\d{1,2}))?\b" + for adresse, prefixe in re.findall(motif, text or ""): + try: + nets.add( + ipaddress.ip_network( + f"{adresse}/{prefixe or 32}", strict=False + ) + ) + except ValueError: + continue + return nets + + +def pick_internal_cidr(text: str, candidats=INTERNAL_CANDIDATES) -> str: + """Le premier candidat qui ne chevauche RIEN de ce que l'hôte connaît. + + Chaîne vide quand tous sont pris : le dire, plutôt que d'en écraser un. + Écraser, ici, c'est couper la seule voie d'accès à la machine.""" + import ipaddress + + utilises = parse_used_nets(text) + for candidat in candidats: + reseau = ipaddress.ip_network(candidat, strict=False) + if not any(reseau.overlaps(u) for u in utilises): + return candidat + return "" + def parse_bridge_config(text: str) -> dict: """/etc/network/interfaces -> {pont: {ports, address}}. @@ -509,7 +572,26 @@ def bridge_setup_cmds( # Et l'erreur d'ifup n'est PAS masquée : « 2>/dev/null » cachait # « operation failed with 'Operation not supported' » — le noyau cloud n'a # pas le module bridge, et c'est ce qu'il fallait lire. - cmds.append(f"mkdir -p /run/network; ifup {nom} || ifreload -a") + # Et SURTOUT pas « ifreload -a » en repli : il recharge TOUTES les + # interfaces, y compris celle qui porte la session ssh, et sur une image + # cloud l'interface principale est décrite ailleurs (interfaces.d, ou + # netplan) — ifupdown2 la descend alors sans la remonter. Le repli est + # donc CHIRURGICAL : on monte le pont à la main, sans toucher à rien + # d'autre. La strophe, elle, le rend persistant au prochain démarrage. + manuel = [ + f"ip link show {nom} >/dev/null 2>&1 || ip link add {nom} type bridge", + f"ip addr add {cidr} dev {nom} 2>/dev/null || true", + f"ip link set {nom} up", + ] + if uplink: + regle = f"POSTROUTING -s {reseau} -o {uplink} -j MASQUERADE" + manuel.append( + f"iptables -t nat -C {regle} 2>/dev/null" + f" || iptables -t nat -A {regle}" + ) + cmds.append( + f"mkdir -p /run/network; ifup {nom} || {{ " + "; ".join(manuel) + "; }" + ) return cmds diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index 41f62a3..9fbc126 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -648,6 +648,20 @@ class ProxmoxMenuMixin: parts = (sortie or "").split() return parts[parts.index("dev") + 1] if "dev" in parts else "" + def _pve_internal_cidr(self, host): + """Réseau du futur pont interne, CHOISI d'après l'hôte. + + Pas une constante : un Proxmox dans un Proxmox hérite du réseau + interne de son parent, et 10.10.10.1 y est l'adresse de sa propre + PASSERELLE. La poser sur son pont rend tout le /24 local, la + passerelle devient injoignable, et la machine s'isole au milieu de la + commande qui la configure. Vécu : « ifup » n'a jamais rendu la main et + la VM ne répondait plus, ni en ssh ni en ping.""" + from script.proxmox import proxmox_deploy as pve + + _c, out = pve.run(host, pve.USED_NETS_CMD, 40) + return pve.pick_internal_cidr(out) + def _pve_nat_ready(self, host): """(prêt ?, lignes à dire). La table NAT existe-t-elle sur cet hôte ? @@ -717,8 +731,11 @@ class ProxmoxMenuMixin: raison = self._pve_nat_reason(host) if raison: return "", raison + cidr = self._pve_internal_cidr(host) + if not cidr: + return "", t("No free subnet left for an internal bridge.") uplink = self._pve_uplink() - for cmd in pve.bridge_setup_cmds(uplink=uplink): + for cmd in pve.bridge_setup_cmds(cidr=cidr, uplink=uplink): code, sortie = pve.run(host, cmd, 180) if code: lignes = pve.strip_ssh_noise(sortie).strip().splitlines() @@ -741,11 +758,20 @@ class ProxmoxMenuMixin: """ from script.proxmox import proxmox_deploy as pve + host = self._pve_host(ask=False) + # Le réseau est LU sur l'hôte avant d'être proposé : l'annoncer + # 10.10.10.1/24 pour en poser un autre serait mentir sur l'écran même + # où l'on demande l'accord. + cidr = self._pve_internal_cidr(host) if host else pve.INTERNAL_CIDR print(f"\n ⚠ {t('No network bridge on this host.')}") print(f" {t('qm create needs one. Two ways:')}") + if not cidr: + print(f" ✗ {t('No free subnet left for an internal bridge.')}") + print(f" {t('do it myself (bridge-ports , needs console)')}") + return "" print( f" [1] {t('create an internal')} {pve.INTERNAL_BRIDGE}" - f" ({pve.INTERNAL_CIDR}) + NAT — {t('touches no physical NIC')}" + f" ({cidr}) + NAT — {t('touches no physical NIC')}" ) print(f" [2] {t('do it myself (bridge-ports , needs console)')}") if input(t("Choice: ")).strip() != "1": @@ -759,7 +785,6 @@ class ProxmoxMenuMixin: f" ⚠ {t('This moves the host address: do it from a console.')}" ) return "" - host = self._pve_host(ask=False) ok, lignes = self._pve_nat_ready(host) if host else (True, []) if not ok: print() @@ -768,7 +793,7 @@ class ProxmoxMenuMixin: return "" uplink = self._pve_uplink() print(f" {t('uplink for NAT')} : {uplink or t('none')}") - for cmd in pve.bridge_setup_cmds(uplink=uplink): + for cmd in pve.bridge_setup_cmds(cidr=cidr, uplink=uplink): code, _o = self._pve_show(cmd, timeout=120) if code: print(f" ✗ {t('Step failed, stopping here.')}") @@ -889,7 +914,13 @@ class ProxmoxMenuMixin: # De quoi créer le pont DEPUIS l'écran, sans invite : le pont # interne ne touche à aucune interface physique. "make_bridge": self._pve_make_internal_bridge, - "internal_bridge": (pve.INTERNAL_BRIDGE, pve.INTERNAL_CIDR), + # Le libellé « ➕ créer un interne vmbr0 (…) » doit annoncer le + # réseau qui sera RÉELLEMENT posé — il dépend de l'hôte. + "internal_bridge": ( + pve.INTERNAL_BRIDGE, + (self._pve_internal_cidr(host) if not ponts else "") + or pve.INTERNAL_CIDR, + ), "build_command": build_command, "branches": self._qemu_branch_list() or ["master"], # La branche du dépôt : c'est elle qu'on déploie le plus souvent. diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 2eeee6a..c00adb5 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -3410,6 +3410,10 @@ TRANSLATIONS = { "fr": "Aucun noyau Proxmox installé : terminer l'installation d'abord.", "en": "No Proxmox kernel installed: finish the install first.", }, + "No free subnet left for an internal bridge.": { + "fr": "Plus aucun réseau libre pour un pont interne.", + "en": "No free subnet left for an internal bridge.", + }, "No network bridge on this host.": { "fr": "Aucun pont réseau sur cet hôte.", "en": "No network bridge on this host.", diff --git a/test/test_proxmox_deploy.py b/test/test_proxmox_deploy.py index 1b37447..8285db3 100644 --- a/test/test_proxmox_deploy.py +++ b/test/test_proxmox_deploy.py @@ -241,8 +241,12 @@ class TestLeNoyau(unittest.TestCase): # ne monte jamais. montee = pve.bridge_setup_cmds("vmbr0", "10.10.10.1/24", "enp1s0")[-1] self.assertIn("mkdir -p /run/network", montee) - # Et l'erreur d'ifup n'est plus masquée : c'est elle qui explique. - self.assertNotIn("2>/dev/null", montee) + # Et l'erreur d'IFUP n'est pas masquée : c'est elle qui explique. + # Porté sur l'appel lui-même, et non sur toute la ligne : le repli qui + # suit sonde légitimement (« ip link show », « iptables -C »), et + # interdire « 2>/dev/null » partout lui interdisait d'exister. + ifup = montee[montee.index("ifup ") :].split("||")[0] + self.assertNotIn("2>", ifup) class TestLeDns(unittest.TestCase): @@ -673,5 +677,114 @@ class TestLaTableNat(unittest.TestCase): self.assertIn("uname -r", pve.NAT_CHECK_CMD) +class TestLeReseauDuPontInterne(unittest.TestCase): + """Le pont interne avait une adresse CODÉE EN DUR, 10.10.10.1/24. + + Un Proxmox dans un Proxmox hérite du réseau interne de son parent : la VM + vivait en 10.10.10.152 avec 10.10.10.1 pour PASSERELLE. Lui demander de + poser 10.10.10.1/24 sur son propre pont, c'est prendre l'adresse de sa + passerelle et rendre tout le /24 local — la machine s'isole au milieu de + la commande qui la configure. Vécu : « ifup » n'a jamais rendu la main, et + la VM ne répondait plus ni en ssh ni en ping.""" + + IMBRIQUE = ( + "2: eth0 inet 10.10.10.152/24 brd 10.10.10.255 scope global eth0\n" + "default via 10.10.10.1 dev eth0 onlink\n" + "10.10.10.0/24 dev eth0 proto kernel scope link src 10.10.10.152\n" + ) + + def test_a_nested_host_gets_another_subnet(self): + self.assertNotEqual( + pve.pick_internal_cidr(self.IMBRIQUE), "10.10.10.1/24" + ) + self.assertEqual( + pve.pick_internal_cidr(self.IMBRIQUE), "10.10.20.1/24" + ) + + def test_a_fresh_host_keeps_the_usual_one(self): + vierge = "1: lo inet 127.0.0.1/8 scope host lo\n" + self.assertEqual(pve.pick_internal_cidr(vierge), "10.10.10.1/24") + + def test_a_route_alone_is_enough_to_collide(self): + # Une route sans adresse locale suffit : c'est le cas exact de la + # route par défaut « via 10.10.10.1 ». + seule = "default via 10.10.10.1 dev eth0\n" + self.assertNotEqual(pve.pick_internal_cidr(seule), "10.10.10.1/24") + + def test_a_supernet_rules_out_everything_under_it(self): + # « 10.0.0.0/8 » couvre tous les candidats en 10.x. Un test sur les + # trois premiers octets l'aurait raté. + choisi = pve.pick_internal_cidr("10.0.0.0/8 dev x\n") + self.assertFalse(choisi.startswith("10."), choisi) + + def test_when_nothing_is_free_it_says_so(self): + tout = "\n".join( + c.replace("1/24", "0/24") for c in pve.INTERNAL_CANDIDATES + ) + self.assertEqual(pve.pick_internal_cidr(tout), "") + + def test_the_chosen_subnet_reaches_every_command(self): + cmds = pve.bridge_setup_cmds(cidr="10.10.20.1/24", uplink="eth0") + texte = "\n".join(cmds) + self.assertIn("address 10.10.20.1/24", texte) + self.assertIn("10.10.20.0/24", texte) + self.assertNotIn("10.10.10.", texte) + + +class TestLeRepliQuiNeCoupePasLaLigne(unittest.TestCase): + """« ifreload -a » en repli rechargeait TOUTES les interfaces. + + Y compris celle qui porte la session ssh — et sur une image cloud + l'interface principale est décrite ailleurs (interfaces.d, netplan), donc + ifupdown2 la descend sans la remonter. Le repli monte donc le pont à la + main, sans toucher à rien d'autre.""" + + def test_ifreload_is_gone(self): + texte = "\n".join(pve.bridge_setup_cmds(uplink="eth0")) + self.assertNotIn("ifreload", texte) + + def test_the_fallback_builds_the_bridge_itself(self): + derniere = pve.bridge_setup_cmds(cidr="10.10.20.1/24", uplink="eth0")[ + -1 + ] + self.assertIn("ifup vmbr0 ||", derniere) + self.assertIn("ip link add vmbr0 type bridge", derniere) + self.assertIn("ip addr add 10.10.20.1/24 dev vmbr0", derniere) + self.assertIn("ip link set vmbr0 up", derniere) + + def test_the_masquerade_rule_is_idempotent(self): + # « -C » avant « -A » : rejouée, la commande n'empile pas les règles. + derniere = pve.bridge_setup_cmds(uplink="eth0")[-1] + self.assertIn("iptables -t nat -C POSTROUTING", derniere) + self.assertLess( + derniere.index("-t nat -C"), derniere.index("-t nat -A") + ) + + def test_the_fallback_is_valid_shell(self): + """Exécuté pour de vrai, ip/iptables/ifup bouchonnés. + + Un repli qu'on ne sait pas exécuter s'ouvre le jour où il casse — et + celui-là tourne sur une machine qu'on ne peut plus joindre s'il rate. + """ + import subprocess + + derniere = pve.bridge_setup_cmds(cidr="10.10.20.1/24", uplink="eth0")[ + -1 + ] + bouchons = ( + 'ip() { [ "$1 $2" = "link show" ] && return 1; return 0; }\n' + "iptables() { return 1; }\n" + "ifup() { return 1; }\n" + "mkdir() { :; }\n" + ) + res = subprocess.run( + ["bash", "-c", bouchons + derniere], + capture_output=True, + text=True, + timeout=30, + ) + self.assertEqual(res.stderr, "", res.stderr) + + if __name__ == "__main__": unittest.main(verbosity=1)