From 07a9f666262e040fca899ec8dc776dc30986cadf Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 24 Aug 2026 06:37:56 -0400 Subject: [PATCH] [FIX] proxmox : l'installation partait sur la mauvaise machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rapporté, et c'est le plus grave de la série. Une VM déployée sur Proxmox sous le nom « erplibre-ubuntu-2604 » — nom déjà porté par un domaine LOCAL — a vu son installation d'ERPLibre + Odoo partir sur la VM locale. Deux causes enchaînées : l'entrée ~/.ssh/config volait l'alias de la locale, et le lanceur détaché ré-résout l'adresse par virsh à chaque tour, qui a répondu avec le domaine homonyme. Le journal l'écrivait — « → 192.168.123.118 » — sans que rien n'alerte. Une VM distante n'est plus ré-résolue : son alias est la seule vérité, puisqu'il porte le rebond. Et son alias suit la convention des VM imbriquées, « hôte+vm », le nom court n'étant ajouté que s'il est libre — l'écran le dit. S'y ajoute le sommaire final qui manquait, à l'image de QEMU/KVM : ce qui existe, son adresse, sa commande ssh, son journal. --- EN --- Reported, and the worst of the series. A VM deployed on Proxmox under the name "erplibre-ubuntu-2604" — a name already held by a LOCAL domain — had its ERPLibre + Odoo install land on the local VM. Two chained causes: the ~/.ssh/config entry stole the local one's alias, and the detached launcher re-resolves the address through virsh on every pass, which answered with the homonymous domain. The log said so — "→ 192.168.123.118" — with nothing to raise an alarm. A remote VM is no longer re-resolved: its alias is the only truth, since it carries the jump. And its alias follows the nested-VM convention, "host+vm", the short name being added only when free — the screen says so. Plus the final summary that was missing, mirroring QEMU/KVM: what exists, its address, its ssh command, its log. Assisted-by: Claude Opus 5 --- script/todo/proxmox_menu.py | 81 ++++++++++++++++++++++++++--- script/todo/qemu_install_monitor.py | 16 ++++-- script/todo/todo_i18n.py | 8 +++ test/test_proxmox_form.py | 57 ++++++++++++++++++-- test/test_qemu_deploy_monitor.py | 7 ++- test/test_qemu_monitor_pve.py | 47 +++++++++++++++++ 6 files changed, 201 insertions(+), 15 deletions(-) diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index 4dc4349..fb18eca 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -990,7 +990,8 @@ class ProxmoxMenuMixin: print(f" {ligne}") if not reussies: return - self._pve_after_create(host, spec, reussies, cle_locale) + joignables = self._pve_after_create(host, spec, reussies, cle_locale) + self._pve_print_summary(spec, joignables or [], session) @staticmethod def _pve_log_dir(): @@ -1047,6 +1048,33 @@ class ProxmoxMenuMixin: fh.write("\n".join(entete) + "\n") return chemin + def _pve_print_summary(self, spec, joignables, session): + """Sommaire final : ce qui existe, où, et comment y entrer. + + Le pendant de celui de QEMU/KVM. Sans lui, l'écran se refermait sur la + vue de progression et il ne restait rien à l'écran — ni l'adresse, ni + la commande ssh, ni le chemin du journal.""" + print(f"\n{'═' * 60}") + print(f" {t('TOTAL summary')}") + print( + f" {t('VMs deployed:')} {len(joignables)}/{len(spec['vms'])}" + f" {t('storage')} {spec.get('storage')}" + f" {t('bridge')} {spec.get('bridge')}" + ) + for vm in joignables: + print( + f" {vm['name']:<32} {t('VMID')} {vm.get('vmid', '?'):<6}" + f" {vm.get('adresse', '?')}" + ) + if vm.get("alias"): + print(f" ssh {vm['alias']}") + if spec.get("install"): + print( + f" {t('Install:')} {spec['install'].get('label') or ''}" + f" ({spec['install'].get('branch')})" + ) + print(f" {t('Log:')} {session}") + def _pve_confirm_spec(self, host, spec): """Récapitulatif puis confirmation, dans le TERMINAL. @@ -1081,6 +1109,21 @@ class ProxmoxMenuMixin: entrer dans une VM qui n'est pas sur notre réseau.""" from script.proxmox import proxmox_deploy as pve + # Les domaines LOCAUX : un nom partagé avec l'un d'eux fait dérailler + # l'alias ssh et le suivi d'installation. + locaux = set(self._qemu_list_domains()) + + def alias_chaine(nom): + """« hôte+vm », la convention déjà utilisée pour les VM + imbriquées : elle dit où la machine vit, et n'entre en conflit + avec rien.""" + hote = (host.get("target") or "").split("@")[-1] + hote = re.sub(r"[^A-Za-z0-9._-]", "-", hote) or "pve" + return f"{hote}+{nom}" + + # {nom de VM: alias à utiliser} — le suivi doit passer par l'alias + # qu'on a RÉELLEMENT écrit, pas par le nom. + alias = {} joignables = [] for vm in spec["vms"]: if vm["name"] not in reussies: @@ -1095,6 +1138,16 @@ class ProxmoxMenuMixin: ) continue print(f" ✓ {vm['name']} : {ip}") + # Un nom qui existe DÉJÀ comme domaine local est un piège : l'alias + # ~/.ssh/config serait volé à la VM locale, et le suivi + # d'installation — qui ré-résout par virsh — irait installer + # ERPLibre sur ELLE. Vécu : « erplibre-ubuntu-2604 » déployée sur + # Proxmox, installation partie sur la VM locale du même nom. + if vm["name"] in locaux: + print( + f" ⚠ {t('A local VM already bears this name:')}" + f" {t('the alias goes to')} {alias_chaine(vm['name'])}" + ) # L'entrée ~/.ssh/config est le SEUL chemin vers cette VM : elle # est derrière l'hôte Proxmox (pont interne), donc son adresse # n'est pas routable d'ici et seul le rebond y mène. Décochée @@ -1105,23 +1158,36 @@ class ProxmoxMenuMixin: if not spec.get("add_ssh_config") and besoin: print(f" → {t('~/.ssh/config written anyway (install)')}") if spec.get("add_ssh_config") or besoin: + # Deux noms, comme pour les VM imbriquées : le nom CHAÎNÉ + # « hôte+vm » dit où la machine vit et ne peut rien voler, et + # le nom court n'est ajouté que s'il ne désigne pas déjà une + # VM locale. + noms_alias = [alias_chaine(vm["name"])] + if vm["name"] not in locaux: + noms_alias.append(vm["name"]) self._write_ssh_config_entry( - vm["name"], + noms_alias, spec.get("user") or "erplibre", ip, identity_file=self._ssh_private_key(cle_locale), proxy_jump=host["target"], ) - print(f" ✓ ~/.ssh/config : ssh {vm['name']}") + alias[vm["name"]] = noms_alias[-1] + print(f" ✓ ~/.ssh/config : ssh {noms_alias[-1]}") + vm["adresse"] = ip + vm["alias"] = alias.get(vm["name"], vm["name"]) joignables.append(vm) install = spec.get("install") + # Rendu à l'appelant pour son sommaire : lui seul sait ce qui a été + # RÉELLEMENT joint. + resultat = list(joignables) # Le suivi vient du DÉPLOIEMENT, pas de l'installation — même règle # qu'en QEMU/KVM. Sans elle, la case « Suivre l'installation » ne # commandait rien : décochée, le tableau de bord s'ouvrait quand # même ; cochée sans rien à installer, il ne s'ouvrait jamais. suivi = spec.get("monitor", True) if not joignables or not (install or suivi): - return + return resultat noms = [vm["name"] for vm in joignables] if install: print(f" {install.get('label') or ''}") @@ -1156,11 +1222,11 @@ class ProxmoxMenuMixin: self._qemu_install_erplibre_monitored( noms, branche, - {n: n for n in noms}, + {n: alias.get(n, n) for n in noms}, finale, pve=cartes_pve, ) - return + return resultat # Sans suivi mais avec quelque chose à installer : en série, sortie à # l'écran. C'est le pendant exact de la voie QEMU/KVM. print(f"\n{t('Installing ERPLibre on each VM')} ({branche})…") @@ -1169,10 +1235,11 @@ class ProxmoxMenuMixin: vm["name"], cle_locale, branche, - pve.ip_from_ipconfig(vm.get("ipconfig") or "") or vm["name"], + alias.get(vm["name"], vm["name"]), vm.get("install_cmd") or commun, False, ) + return resultat def _pve_deploy_prompts(self, dry_run=False): """Déploie une VM SUR l'hôte Proxmox choisi, par questions. diff --git a/script/todo/qemu_install_monitor.py b/script/todo/qemu_install_monitor.py index 5bd6a46..972da02 100644 --- a/script/todo/qemu_install_monitor.py +++ b/script/todo/qemu_install_monitor.py @@ -85,9 +85,18 @@ def _launch_one( log_path: str, name: str = "", installs: bool = True, + pve: bool = False, ) -> None: """Lance une install SSH DÉTACHÉE : attend le sshd, exécute, journalise - la sortie puis écrit le marqueur de fin avec le code de sortie.""" + la sortie puis écrit le marqueur de fin avec le code de sortie. + + `pve` : la VM vit sur un hôte Proxmox. On ne RÉ-RÉSOUT alors PAS son + adresse par virsh — et c'est vital. Vécu le 24 août 2026 : une VM + « erplibre-ubuntu-2604 » déployée sur Proxmox portait le nom d'un domaine + LOCAL existant ; la ré-résolution a trouvé le domaine local et + l'installation d'ERPLibre + Odoo est partie sur la mauvaise machine, sans + que rien ne le dise. Pour une VM distante, l'alias ~/.ssh/config est la + seule vérité : il porte le rebond par l'hôte.""" # Sonde de disponibilité : on attend que sshd réponde ET que cloud-init # soit TERMINÉ, via des connexions COURTES successives (jusqu'à ~20 min : # une architecture ÉMULÉE, s390x/arm64 sur hôte x86, boote lentement). @@ -188,12 +197,12 @@ def _launch_one( f'echo " {msg_moved} $ip -> $n" >> {log_q}; fi; ' '[ -n "$n" ] && ip="$n"; ' ) - if name + if name and not pve else "" ) wrapper = ( f"ip={shlex.quote(ip)}; " - f"{vsh if name else ''}" + f"{vsh if name and not pve else ''}" f"echo {shlex.quote('== ' + msg_wait + ' ==')} >> {log_q}; " f"echo {shlex.quote(' ' + msg_slow)} >> {log_q}; " f"seen=0; " @@ -290,6 +299,7 @@ def launch_installs(vms: list[dict], branch: str, remote_cmd: str) -> str: log_path, vm["name"], installs=bool(branch), + pve=bool(vm.get("pve")), ) entree = { "name": vm["name"], diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 48037c4..3e28aed 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -3986,6 +3986,14 @@ TRANSLATIONS = { "fr": "Fichiers orphelins :", "en": "Orphan files:", }, + "A local VM already bears this name:": { + "fr": "Une VM locale porte déjà ce nom :", + "en": "A local VM already bears this name:", + }, + "the alias goes to": { + "fr": "l'alias devient", + "en": "the alias goes to", + }, "s ssh · c copy log · C copy all · q quit": { "fr": "s ssh · c copier le log · C copier tout · q quitter", "en": "s ssh · c copy log · C copy all · q quit", diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index a4decb3..2ac97a5 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -575,14 +575,63 @@ class TestLeSuivi(unittest.TestCase): return ecrites cmd = {"branch": "develop", "cmd": "make x", "label": "X"} - # Décoché mais une installation demandée : écrite quand même. - self.assertEqual(essai(False, cmd, False), ["vm-a"]) + # Deux noms : le chaîné « hôte+vm », qui dit où la machine vit, et le + # nom court quand aucun domaine local ne le porte déjà. + self.assertEqual(essai(False, cmd, False), [["pve1+vm-a", "vm-a"]]) # Décoché, suivi demandé : le suivi entre aussi par le rebond. - self.assertEqual(essai(False, None, True), ["vm-a"]) + self.assertEqual(essai(False, None, True), [["pve1+vm-a", "vm-a"]]) # Décoché et rien à faire dans la VM : le choix est respecté. self.assertEqual(essai(False, None, False), []) # Coché : écrite, évidemment. - self.assertEqual(essai(True, None, False), ["vm-a"]) + self.assertEqual(essai(True, None, False), [["pve1+vm-a", "vm-a"]]) + + def test_a_local_vm_of_the_same_name_keeps_its_alias(self): + """Le piège qui a fait installer ERPLibre sur la MAUVAISE machine. + + Une VM déployée sur Proxmox sous un nom déjà porté par un domaine + LOCAL volait son alias ~/.ssh/config, et le suivi — qui ré-résolvait + l'adresse par virsh — partait installer sur la locale.""" + import contextlib + import io + import sys + + sys.argv = ["todo.py"] + from script.todo.todo import TODO + + todo = TODO.__new__(TODO) + ecrites = [] + todo._qemu_list_domains = lambda: ["vm-a"] + todo._write_ssh_config_entry = lambda noms, *a, **k: ecrites.append( + noms + ) + todo._ssh_private_key = lambda k: None + todo._pve_guest_ip = lambda vmid, attente=120: "" + vus = {} + todo._qemu_install_erplibre_monitored = ( + lambda noms, br, ipmap, cmd, **k: vus.update(ipmap=ipmap) + ) + spec = { + "host": {"target": "erplibre@pve1"}, + "vms": [ + { + "name": "vm-a", + "vmid": 100, + "ipconfig": "ip=10.10.10.150/24,gw=10.10.10.1", + "install_cmd": "", + } + ], + "add_ssh_config": True, + "user": "erplibre", + "install": None, + "monitor": True, + } + with contextlib.redirect_stdout(io.StringIO()) as sortie: + todo._pve_after_create(spec["host"], spec, ["vm-a"], "") + # SEUL le nom chaîné est écrit : l'alias court reste à la VM locale. + self.assertEqual(ecrites, [["pve1+vm-a"]]) + # Et le suivi passe par ce nom-là, jamais par « vm-a ». + self.assertEqual(vus["ipmap"], {"vm-a": "pve1+vm-a"}) + self.assertIn("pve1+vm-a", sortie.getvalue()) def test_ticked_with_an_install_opens_it(self): vus = self._apres_creation( diff --git a/test/test_qemu_deploy_monitor.py b/test/test_qemu_deploy_monitor.py index fb17d2c..9ea30b8 100644 --- a/test/test_qemu_deploy_monitor.py +++ b/test/test_qemu_deploy_monitor.py @@ -212,8 +212,13 @@ class TestLeJournal(unittest.TestCase): vus = [] vrai_launch, vrai_dir = mon._launch_one, mon.session_dir + # « **kw » et non une liste figée : chaque paramètre ajouté au + # lanceur (comme « pve ») casserait sinon ce test, qui ne vérifie + # pourtant que le prologue du journal. mon._launch_one = ( - lambda ip, cmd, log, name="", installs=True: vus.append(installs) + lambda ip, cmd, log, name="", installs=True, **kw: vus.append( + installs + ) ) # session_dir détournée : sans cela le test écrivait de VRAIES sessions # dans ~/.erplibre/qemu-install, qui polluaient l'historique que diff --git a/test/test_qemu_monitor_pve.py b/test/test_qemu_monitor_pve.py index 30a4ea0..4d522a6 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -131,6 +131,53 @@ class TestLAppel(unittest.TestCase): self.assertEqual(mon.read_pvestats(self._vms(1), now=1.0), {}) +class TestLaBonneMachine(unittest.TestCase): + """Le pire défaut de la série : l'installation partie AILLEURS. + + Vécu le 24 août 2026. Une VM déployée sur Proxmox sous le nom + « erplibre-ubuntu-2604 » — nom déjà porté par un domaine LOCAL. Le + lanceur détaché ré-résout l'adresse de la VM à chaque tour par virsh, qui + a répondu avec le domaine local : ERPLibre + Odoo se sont installés sur la + MAUVAISE machine, et le journal l'affichait sans que rien n'alerte + (« → 192.168.123.118 »). + + Pour une VM distante, l'alias ~/.ssh/config est la seule vérité : il + porte le rebond par l'hôte Proxmox. + """ + + def _wrapper(self, **kw): + """Le script du lanceur, capturé sans rien exécuter.""" + vus = {} + vrai = mon.subprocess.Popen + + class FauxPopen: + def __init__(self, argv, *a, **k): + vus["argv"] = argv + + mon.subprocess.Popen = FauxPopen + try: + mon._launch_one("cible", "echo bonjour", "/dev/null", "vm-a", **kw) + finally: + mon.subprocess.Popen = vrai + return vus["argv"][-1] + + def test_a_local_vm_still_gets_its_address_refreshed(self): + # Le bail change en cours de route (cloud-init renomme l'hôte) : la + # ré-résolution est indispensable pour une VM LOCALE. + script = self._wrapper(pve=False) + self.assertIn("virsh", script) + + def test_a_proxmox_vm_is_never_re_resolved(self): + # C'est le correctif : aucun appel à virsh, donc aucun risque de + # tomber sur un domaine local homonyme. + script = self._wrapper(pve=True) + self.assertNotIn("virsh", script) + + def test_the_target_stays_the_alias(self): + script = self._wrapper(pve=True) + self.assertIn("ip=cible", script) + + class TestLEtat(unittest.TestCase): """Une VM absente de « virsh list » passait pour EFFACÉE."""