From bf8f975ef0d3f2f03c7fdac9eea646f03c73944d Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 24 Aug 2026 14:15:03 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20proxmox=20:=20six=20d=C3=A9fauts=20trou?= =?UTF-8?q?v=C3=A9s=20par=20un=20audit,=20pas=20=C3=A0=20l'usage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trois autres chemins menaient au 🗑 sur un seul incident, et « effacée » gèle la ligne pour de bon. Un « virsh list » en échec condamnait TOUT le parc local. Un statut Proxmox hors des trois attendus — prelaunch, suspended, internal-error — passait pour une disparition. Et le code de sortie de la suite distante est celui de son DERNIER maillon : un pvesh en panne se lisait « l'hôte a répondu sans elle ». Ce qui prouve une réponse, c'est désormais une liste de ressources analysable. Le plan annonçait « 25G » quand « qm resize » recevait 20 : la marge d'ERPLibre se perdait en route, la VM naissait trop petite. « Changer l'état » choisissait par NOM, or seul le VMID est unique sur un hôte — cocher une VM en éteignait deux homonymes. L'entrée 13 volait son alias à une VM locale du même nom. Enfin le déploiement par QUESTIONS avait vieilli seul : il partage maintenant l'épilogue de l'écran, donc le guide, l'alias protégé, les colonnes vivantes et le sommaire. --- EN --- Three more paths led to 🗑 on a single incident, and "deleted" freezes the row for good. One failing "virsh list" condemned the WHOLE local fleet. A Proxmox status outside the three expected ones — prelaunch, suspended, internal-error — passed for a disappearance. And a remote pipeline's exit code is its LAST link's: a broken pvesh read as "the host answered without it". Proof of an answer is now a parsable resource list. The plan announced "25G" while "qm resize" got 20: ERPLibre's margin was lost on the way and the VM was born too small. "Change state" selected by NAME, yet only the VMID is unique on a host — ticking one VM shut down two namesakes. Menu entry 13 stole its alias from a local VM of the same name. Finally the QUESTION-driven deployment had aged alone: it now shares the screen's epilogue — guide, protected alias, live columns and summary. Assisted-by: Claude Opus 5 --- script/todo/proxmox_menu.py | 107 ++++++++++++++++++++++------ script/todo/qemu_install_monitor.py | 46 +++++++++++- test/test_proxmox_form.py | 78 ++++++++++++++++++++ test/test_qemu_monitor_pve.py | 38 ++++++++++ 4 files changed, 244 insertions(+), 25 deletions(-) diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index ac68eef..3a0ff9a 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -435,12 +435,17 @@ class ProxmoxMenuMixin: if not brut: print(t("Nothing selected.")) return - noms = [vm["name"] for vm in vms] if brut in ("all", "*"): choisies = list(vms) else: - retenus = set(self._parse_index_selection(brut, noms)) - choisies = [vm for vm in vms if vm["name"] in retenus] + # Par RANG, jamais par nom : deux VM du même hôte peuvent + # porter le même nom (seul le VMID est unique sur Proxmox), et + # cocher l'une éteignait les deux. + rangs = self._parse_index_selection( + brut, [str(i) for i in range(1, len(vms) + 1)] + ) + voulus = {int(r) for r in rangs if str(r).isdigit()} + choisies = [vm for i, vm in enumerate(vms, 1) if i in voulus] if not choisies: print(t("Nothing selected.")) return @@ -875,6 +880,23 @@ class ProxmoxMenuMixin: code, out = self._pve_show("hostname", quiet=True) return out.strip().splitlines()[0] if code == 0 and out.strip() else "" + def _pve_disk_with_margin(self, vm, spec): + """Taille du disque à créer : celle du plan, marge comprise. + + La même règle que la voie libvirt, qui ajoute ERPLIBRE_EXTRA_DISK_GB à + la demande initiale quand ERPLibre s'installe. Ici la marge se perdait + entre l'écran et « qm resize ». + """ + demande = vm.get("disk") or "" + install = spec.get("install") or {} + cmd = vm.get("install_cmd") or install.get("cmd") or "" + if not self._qemu_installs_erplibre(install.get("branch"), cmd): + return demande + gigs = self._parse_disk_gb(demande) + if not gigs: + return demande + return f"{gigs + self.ERPLIBRE_EXTRA_DISK_GB}G" + def _pve_vm_commands(self, mod, vm, spec): """Les commandes de création d'UNE VM, dans l'ordre : l'image puis « qm ». Sert à l'aperçu comme à l'exécution — un aperçu qui ne @@ -890,7 +912,12 @@ class ProxmoxMenuMixin: "name": vm["name"], "memory": vm["ram"], "vcpus": vm["vcpus"], - "disk": vm["disk"], + # La MARGE d'ERPLibre entre dans la taille réellement créée : le + # plan l'annonçait (« 25G » pour un catalogue à 20 G) et « qm + # resize » recevait 20 G. La VM naissait cinq gigaoctets trop + # petite pour ce qu'on venait de lui promettre — trouvé par + # l'audit, pas à l'usage. + "disk": self._pve_disk_with_margin(vm, spec), "storage": spec["storage"], "bridge": spec["bridge"], "image": image, @@ -1497,28 +1524,51 @@ class ProxmoxMenuMixin: if not ip: print(f" ⚠ {t('No address yet. Try [6] later.')}") return - print(f" ✓ {nom} : {ip}") - # Entrée ~/.ssh/config avec l'hôte Proxmox en REBOND : c'est ce qui - # rend la VM joignable d'ici, et c'est aussi ce qui permet au suivi - # d'installation d'y entrer (il reçoit l'alias, pas l'IP). - self._write_ssh_config_entry( - nom, - "erplibre", - ip, - identity_file=self._ssh_private_key(cle_locale), - proxy_jump=host["target"], - ) - print(f" ✓ ~/.ssh/config : ssh {nom}") + # ÉPILOGUE COMMUN avec l'écran, au lieu de le redire ici : cette voie + # avait vieilli en silence — pas de protection de l'alias contre un + # domaine local homonyme, pas de guide de connexion, pas de bloc + # « pve » (donc aucune colonne vivante dans le suivi), pas de + # sommaire. Trouvé par l'audit, jamais à l'usage. + install = None if self._is_yes_default_yes( input(f"\n{t('Install ERPLibre on it? (Y/n): ')}") ): branch = self._qemu_pick_branch() label, cmd = self._qemu_pick_install_profile(distro) print(f" {label}") - # L'ALIAS, pas l'IP : ssh y lit le ProxyJump de ~/.ssh/config. - self._qemu_install_erplibre_monitored( - [nom], branch, {nom: nom}, cmd - ) + install = {"branch": branch, "cmd": cmd, "label": label} + spec_finale = { + "host": host, + "storage": stockage, + "bridge": pont, + "res_label": "", + "vms": [ + { + "name": nom, + "vmid": vmid, + "distro": distro, + "version": version, + "arch": arch, + "ram": memoire, + "vcpus": vcpus, + "disk": disque, + "desktop": "", + "install_cmd": "", + "ipconfig": ipconfig, + } + ], + "existing": [], + "user": "erplibre", + "ssh_key": cle_locale or "", + "add_ssh_config": True, + "install": install, + "monitor": True, + "python_provider": "", + } + joignables = self._pve_after_create( + host, spec_finale, [nom], cle_locale + ) + self._pve_print_summary(spec_finale, joignables or [], "") def _pve_ssh_config(self): """Écrit une entrée ~/.ssh/config par VM de l'hôte, avec l'hôte @@ -1532,20 +1582,33 @@ class ProxmoxMenuMixin: print(f"\n{t('No running VM on this Proxmox host.')}") return cle = self._ssh_private_key(self._qemu_default_ssh_key()) + # Les domaines LOCAUX : un nom partagé avec l'un d'eux ne doit pas lui + # voler son alias — même règle que le déploiement. + locaux = set(self._qemu_list_domains()) + hote_court = (host.get("target") or "").split("@")[-1] + hote_court = re.sub(r"[^A-Za-z0-9._-]", "-", hote_court) or "pve" for vm in vms: ip = self._pve_guest_ip(vm["vmid"], attente=0) if not ip: print(f" ⚠ {vm['name']} : {t('no address, skipped')}") continue + noms = [f"{hote_court}+{vm['name']}"] + if vm["name"] not in locaux: + noms.append(vm["name"]) + else: + print( + f" ⚠ {t('A local VM already bears this name:')}" + f" {vm['name']}" + ) self._write_ssh_config_entry( - vm["name"], + noms, "erplibre", ip, identity_file=cle, proxy_jump=host["target"], ) print( - f" ✓ ssh {vm['name']} ({ip} {t('through')} {host['target']})" + f" ✓ ssh {noms[-1]} ({ip} {t('through')} {host['target']})" ) def _pve_test_vm(self): diff --git a/script/todo/qemu_install_monitor.py b/script/todo/qemu_install_monitor.py index e32931a..81bcd52 100644 --- a/script/todo/qemu_install_monitor.py +++ b/script/todo/qemu_install_monitor.py @@ -1348,6 +1348,19 @@ def pve_stats_cmd(adresses=()) -> str: ) +def _resources_parsable(text: str) -> bool: + """La sortie porte-t-elle une LISTE de ressources lisible ? + + C'est la seule preuve que l'hôte a répondu : le code de sortie est celui + du dernier maillon de la suite, pas celui de « pvesh ». + """ + brut, _, _ = (text or "").partition("---ERPLIBRE-DU---") + try: + return isinstance(json.loads(brut.strip() or "null"), list) + except ValueError: + return False + + def parse_odoo_probe(text: str) -> set: """Adresses dont le port 8069 a répondu, d'après pve_stats_cmd.""" _, _, bloc = (text or "").partition("---ERPLIBRE-ODOO---") @@ -1479,7 +1492,12 @@ def _read_pvestats(vms, now=None): pve_stats_cmd(siennes), 40, ) - if code == 0: + # « code == 0 » ne suffit PAS : la commande est une SUITE + # (pvesh ; echo ; du ; echo ; boucle), et son code est celui du DERNIER + # maillon. Un pvesh en panne rendait donc « l'hôte a répondu, la VM + # n'y est plus » — et trois tours plus tard, la poubelle. Ce qui prouve + # une réponse, c'est une LISTE de ressources analysable. + if code == 0 and _resources_parsable(sortie): ok = True releves = parse_pvestats(sortie) ouverts = parse_odoo_probe(sortie) @@ -2491,13 +2509,21 @@ def run_monitor(manifest_path: str, run_app: bool = True): # L'hôte n'a pas répondu : on ne sait RIEN. Conclure # « effacée » ici gelait la ligne sur 🗑 dès le premier # tour, pour toujours — vécu sur une VM Arch à peine - # déployée. + # déployée. Et on OUBLIE les absences déjà comptées : + # elles ne prouvent une disparition que si elles se + # SUIVENT, l'hôte répondant à chaque fois. + self._pve_absences[nom] = 0 continue releve = distants.get(nom) if releve: self._pve_absences[nom] = 0 + # PRÉSENTE dans le relevé : elle existe, quel que soit + # le mot employé. Proxmox en a d'autres que les trois + # attendus — « prelaunch », « suspended », + # « internal-error », « hibernated » — et les traduire + # en « gone » mettait à la poubelle une VM bien vivante. self._domstate[nom] = PVE_ETATS.get( - releve.get("state"), "gone" + releve.get("state"), "running" ) continue # L'hôte a répondu SANS elle : peut-être en cours de @@ -2508,6 +2534,20 @@ def run_monitor(manifest_path: str, run_app: bool = True): if self._pve_absences[nom] >= PVE_ABSENCES_AVANT_EFFACEE: self._domstate[nom] = "gone" continue + if not states: + # « virsh list » n'a rien rendu : soit l'hôte n'a plus une + # seule VM, soit l'appel a échoué (libvirtd qui redémarre, + # sudo qui expire). On ne peut pas trancher, et conclure + # « effacées » mettait TOUT le parc local à la poubelle, + # définitivement. Même règle que pour l'hôte distant : on + # compte avant de conclure. + self._pve_absences[nom] = ( + self._pve_absences.get(nom, 0) + 1 + ) + if self._pve_absences[nom] >= PVE_ABSENCES_AVANT_EFFACEE: + self._domstate[nom] = "gone" + continue + self._pve_absences[nom] = 0 self._domstate[nom] = states.get(nom, "gone") # Réarmer la période du ballon sur les VM qui tournent : sans elle # la RAM affichée serait celle du dernier rapport du pilote, et une diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index c418a83..592b7a6 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -519,6 +519,84 @@ class TestLInterpretePython(unittest.TestCase): self.assertEqual(self._ecran(mise_arches=("s390x",))["choix"], "") +class TestLeDisquePromis(unittest.TestCase): + """Le plan annonçait « 25G » et « qm resize » recevait 20 G. + + La voie libvirt ajoute la marge d'ERPLibre à la taille créée ; celle de + Proxmox la perdait entre l'écran et la commande. La VM naissait cinq + gigaoctets trop petite pour ce qu'on venait de lui promettre.""" + + def _taille(self, install, cmd_vm=""): + import sys + + sys.argv = ["todo.py"] + from script.todo.todo import TODO + + todo = TODO.__new__(TODO) + vm = {"disk": "20G", "install_cmd": cmd_vm} + return todo._pve_disk_with_margin(vm, {"install": install}) + + def test_the_margin_reaches_the_created_disk(self): + self.assertEqual( + self._taille( + { + "branch": "develop", + "cmd": "make install_os && make install_odoo_18", + } + ), + "25G", + ) + + def test_nothing_to_install_means_no_margin(self): + self.assertEqual(self._taille(None), "20G") + + def test_a_hypervisor_profile_gets_no_margin(self): + # Elle est réservée au dépôt ERPLibre, qu'un Proxmox ne clonera pas. + self.assertEqual( + self._taille( + { + "branch": "develop", + "cmd": "./script/proxmox/install_proxmox.sh", + } + ), + "20G", + ) + + +class TestDeuxVmDuMemeNom(unittest.TestCase): + """Sur Proxmox, seul le VMID est unique : deux VM du même hôte peuvent + porter le même nom. « Changer l'état » les choisissait par NOM — cocher + l'une éteignait les deux.""" + + def test_selecting_one_twin_takes_only_that_one(self): + import sys + + sys.argv = ["todo.py"] + from script.todo.todo import TODO + + todo = TODO.__new__(TODO) + vms = [ + {"vmid": 100, "name": "jumeau", "status": "running"}, + {"vmid": 101, "name": "jumeau", "status": "running"}, + ] + rangs = [str(i) for i in range(1, len(vms) + 1)] + for choix, attendu in ( + ("1", [100]), + ("2", [101]), + ("1,2", [100, 101]), + ): + voulus = { + int(r) + for r in todo._parse_index_selection(choix, rangs) + if str(r).isdigit() + } + self.assertEqual( + [vm["vmid"] for i, vm in enumerate(vms, 1) if i in voulus], + attendu, + choix, + ) + + class TestLeGuideDeConnexion(unittest.TestCase): """Une VM Proxmox n'avait AUCUN guide, quelle que soit sa distribution. diff --git a/test/test_qemu_monitor_pve.py b/test/test_qemu_monitor_pve.py index cfb0202..221cb6a 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -192,6 +192,44 @@ class TestPasDePoubelleTropTot(unittest.TestCase): self.assertEqual(mon.read_pvestats([self._vm()], now=7.0), {}) +class TestLesAutresCheminsVersLaPoubelle(unittest.TestCase): + """Trouvés par un audit, pas à l'usage : trois autres façons d'arriver au + 🗑 sur un seul incident. « Effacée » gèle la ligne pour de bon, donc + chacune valait un correctif.""" + + def test_a_broken_pvesh_is_not_an_answer(self): + # La commande est une SUITE : son code de sortie est celui du DERNIER + # maillon. Un pvesh en panne rendait « l'hôte a répondu, la VM n'y est + # plus » — et trois tours plus tard, la poubelle. + self.assertFalse( + mon._resources_parsable("permission denied\n---ERPLIBRE-DU---\n") + ) + self.assertTrue(mon._resources_parsable("[]\n---ERPLIBRE-DU---\n")) + + def test_a_failing_host_never_counts_as_an_answer(self): + mon._PVE_CACHE.update({"at": 0.0, "stats": {}, "ok": False}) + vm = {"name": "vm-a", "pve": {"target": "h", "sudo": "", "vmid": 1}} + with mock.patch( + "script.proxmox.proxmox_deploy.run", + return_value=(0, "sudo: a password is required\n"), + ): + _stats, ok = mon.read_pvestats_detail([vm], now=10.0) + self.assertFalse(ok, "code 0 ne prouve pas que pvesh a parlé") + + def test_an_unknown_proxmox_status_is_not_a_deletion(self): + # Proxmox en a d'autres que les trois attendus : « prelaunch », + # « suspended », « internal-error », « hibernated ». + for etat in ("prelaunch", "suspended", "internal-error", "hibernated"): + self.assertIsNone( + mon.PVE_ETATS.get(etat), + "l'état n'est pas dans la table : le repli doit être « la VM" + " existe », pas « gone »", + ) + + def test_the_absence_counter_is_explicit(self): + self.assertGreaterEqual(mon.PVE_ABSENCES_AVANT_EFFACEE, 2) + + class TestLaBonneMachine(unittest.TestCase): """Le pire défaut de la série : l'installation partie AILLEURS.