From db9472ef69ba22c81eb9b37b4f7e3e6f4738d199 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Tue, 25 Aug 2026 00:43:10 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20proxmox=20:=20l'ancienne=20entr=C3=A9e?= =?UTF-8?q?=20ssh=20s'en=20va=20avec=20la=20convention?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le nom chaîné devient systématique, mais les entrées écrites AVANT portent le nom court — et rien ne les retirerait : elles ne déclarent pas le nom qu'on écrit maintenant. Deux blocs mèneraient à la même machine, exactement ce qu'on venait d'enlever. Le ProxyJump tranche : un bloc qui rebondit par CET hôte est le nôtre, on le retire. Celui d'une VM locale homonyme n'en a pas, et on n'y touche jamais ; celui d'un autre hôte Proxmox non plus. Le drapeau Odoo gagne son test au passage. Il tombait pour la même raison que les colonnes vides — la sonde est le dernier maillon de la suite distante, et un parc où une seule VM n'a pas d'Odoo, un hyperviseur imbriqué par exemple, finit en échec. Vérifié sur les trois VM : l'hôte rend bien « ODOO » pour les deux qui écoutent, et le navigateur répondait 303 pendant que la colonne disait « — ». --- EN --- The chained name becomes systematic, but entries written BEFORE carry the short one — and nothing would retire them: they do not declare the name we now write. Two blocks would lead to the same machine, exactly what we had just removed. The ProxyJump decides: a block hopping through THIS host is ours, so it goes. A local namesake's has none, and is never touched; another Proxmox host's neither. The Odoo flag gains its test along the way. It failed for the same reason as the empty columns — the probe is the remote pipeline's last link, and a fleet where a single VM has no Odoo, a nested hypervisor for instance, ends in failure. Verified on all three VMs: the host does return "ODOO" for the two that listen, and the browser answered 303 while the column said "—". Assisted-by: Claude Opus 5 --- script/todo/proxmox_menu.py | 18 +++++ script/todo/todo.py | 127 ++++++++++++++++++++++++++++++++-- test/test_proxmox_form.py | 75 ++++++++++++++++++++ test/test_qemu_monitor_pve.py | 47 +++++++++++++ 4 files changed, 263 insertions(+), 4 deletions(-) diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index 76754d6..150955f 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -1143,6 +1143,20 @@ class ProxmoxMenuMixin: signale au passage un nom qu'une VM locale porte aussi.""" return [chaine], (t("a local VM") if nom in locaux else "") + def _pve_alias_perime(self, nom, rebond): + """Le nom court à RETIRER, s'il désigne encore cette VM-ci. + + La convention a changé — le nom court d'abord, puis « hôte+vm » — et + rien ne retirerait l'ancien bloc : il ne porte pas le nom qu'on + écrit. Deux entrées mèneraient alors à la même machine, ce qu'on + venait justement d'enlever. + + Le ProxyJump tranche : un bloc qui rebondit par CET hôte est le nôtre. + Celui d'une VM locale homonyme n'en a pas, et on n'y touche donc + jamais.""" + bloc = self._ssh_config_block(nom) + return [nom] if bloc and bloc.get("proxyjump") == rebond else [] + def _pve_set_timezone(self, cible, spec): """Pose le fuseau DANS la VM, par ssh. @@ -1384,6 +1398,9 @@ class ProxmoxMenuMixin: ip, identity_file=self._ssh_private_key(cle_locale), proxy_jump=host["target"], + also_drop=self._pve_alias_perime( + vm["name"], host["target"] + ), ) alias[vm["name"]] = noms_alias[0] print(f" ✓ ~/.ssh/config : ssh {noms_alias[0]}") @@ -1713,6 +1730,7 @@ class ProxmoxMenuMixin: ip, identity_file=cle, proxy_jump=host["target"], + also_drop=self._pve_alias_perime(vm["name"], host["target"]), ) print( f" ✓ ssh {noms[0]} ({ip} {t('through')} {host['target']})" diff --git a/script/todo/todo.py b/script/todo/todo.py index 0249191..4ac7732 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -1430,13 +1430,24 @@ class TODO( return "".join(out) def _write_ssh_config_entry( - self, host, user, ip, proxy_jump=None, identity_file=None + self, + host, + user, + ip, + proxy_jump=None, + identity_file=None, + also_drop=(), ): """Écrit/remplace un bloc « Host » dans ~/.ssh/config. `host` peut être une liste de noms : ils partagent alors un seul bloc. - Sert aux VM imbriquées, joignables par leur nom court ET par leur nom - chaîné « parent+enfant », qui montre où elles vivent. + + `also_drop` : noms dont le bloc doit DISPARAÎTRE sans être réécrit. + Sert quand une convention de nommage change : l'ancienne entrée ne + désigne pas le nom qu'on écrit, donc rien ne la retirerait, et deux + blocs finiraient par mener à la même machine — ce qu'on venait + justement d'enlever. L'appelant vérifie que l'ancien bloc est BIEN le + sien avant de le nommer ici. `proxy_jump` : alias du rebond pour une VM imbriquée, dont l'IP n'est joignable que depuis son hôte. OpenSSH enchaîne les ProxyJump tout @@ -1452,7 +1463,9 @@ class TODO( if os.path.exists(cfg): with open(cfg, encoding="utf-8") as fh: existing = fh.read() - existing = self._ssh_config_drop_hosts(existing, names).rstrip("\n") + existing = self._ssh_config_drop_hosts( + existing, names + [n for n in also_drop if n not in names] + ).rstrip("\n") block = ( f"Host {' '.join(names)}\n" f" HostName {ip}\n" @@ -1705,6 +1718,40 @@ class TODO( pass return names + @classmethod + def _ssh_config_block(cls, name): + """Le bloc « Host … » qui déclare `name`, ou {}. + + Rend ses noms ET ses directives : savoir qu'un nom est pris ne suffit + pas, il faut savoir PAR QUI. Le ProxyJump distingue notre propre + entrée — celle d'une VM derrière tel hôte — de celle d'une machine + qui se trouve porter le même nom. + + {"names": [...], "proxyjump": "...", "hostname": "..."}.""" + path = os.path.expanduser("~/.ssh/config") + try: + with open(path, encoding="utf-8") as fh: + contenu = fh.read() + except OSError: + return {} + bloc = None + for line in contenu.splitlines(): + if re.match(r"^[ \t]*Host[ \t]+", line): + if bloc is not None: + return bloc + noms = line.split()[1:] + bloc = {"names": noms} if name in noms else None + continue + if bloc is None: + continue + # Une ligne non indentée et non vide clôt le bloc. + if line.strip() and not line[:1].isspace(): + return bloc + mots = line.split() + if len(mots) >= 2 and mots[0].lower() in ("proxyjump", "hostname"): + bloc[mots[0].lower()] = mots[1] + return bloc or {} + @staticmethod def _ssh_config_user(host): """`User` déclaré pour cet hôte dans ~/.ssh/config, ou "". @@ -3556,8 +3603,80 @@ class TODO( extra = None if analyse.get("asks_expect"): extra = ["--expect", self._monitoring_expect(kind)] + if analyse.get("writes"): + self._monitoring_write_flow(analyse, target) + return monitoring.run_analysis(analyse, target, extra=extra) + def _monitoring_write_flow(self, analyse, database): + """La seule analyse qui écrit : montrer, puis demander. + + On lance TOUJOURS la marche à blanc d'abord, et l'on demande + ensuite. Une question posée avant de savoir ce qui sera touché + n'est pas un consentement : c'est un pari. Le rapport dit combien + de modèles et de colonnes, et lesquels sont traduits ou uniques. + + La confirmation redemande le NOM de la base. Une frappe sur « o » + se donne par réflexe ; recopier « chezlepro_neutralize_upgrade_18 » + oblige à regarder ce qu'on détruit. + """ + from script.analyse import monitoring + + choix = self._monitoring_anonymize_options() + if choix is None: + return + print() + if monitoring.run_analysis(analyse, database, extra=choix) == 2: + return + print() + print( + f"⚠️ {t('This DESTROYS the data of')} '{database}'" + f" — {t('there is no undo.')}" + ) + tape = input( + f"💬 {t('Type the database name to confirm (empty to cancel): ')}" + ).strip() + if tape != database: + print(f"↩️ {t('Cancelled: nothing was written.')}") + return + monitoring.run_analysis( + analyse, database, extra=choix + ["--apply", "--confirm", database] + ) + + def _monitoring_anonymize_options(self): + """Le mode et ses listes, ou None si l'on renonce.""" + print() + print(f"[1] {t('Hybrid: the default personal-data models, adjusted')}") + print(f"[2] {t('Whitelist: only the models I name')}") + print(f"[3] {t('Blacklist: every model except those I name')}") + print(f"[0] {t('Back')}") + answer = click.prompt(t("Command:")) + print() + mode = {"1": "hybrid", "2": "whitelist", "3": "blacklist"}.get(answer) + if not mode: + return None + extra = ["--mode", mode] + invite = ( + t("Models to ADD, comma separated (empty for none): ") + if mode != "blacklist" + else t("Models to EXCLUDE, comma separated: ") + ) + noms = input(f"💬 {invite}").strip() + if noms: + extra += ["--exclude" if mode == "blacklist" else "--models", noms] + elif mode == "whitelist": + print(f"❌ {t('A whitelist with no model would do nothing.')}") + return None + mots = input( + f"💬 {t('Python file declaring MOTS (empty for the built-in): ')}" + ).strip() + if mots: + if not os.path.isfile(os.path.expanduser(mots)): + print(f"❌ {t('No such file: ')}{mots}") + return None + extra += ["--words", os.path.expanduser(mots)] + return extra + def _monitoring_expect(self, kind): """Copie de développement, ou instance en service ? diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index 6e5c986..9413265 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -795,6 +795,81 @@ class TestUnSeulNomDansSshConfig(unittest.TestCase): ) +class TestLAncienNomSEnVa(unittest.TestCase): + """La convention a changé : les entrées écrites AVANT portent le nom + court, et rien ne les retirerait — elles ne portent pas le nom qu'on + écrit maintenant. Deux blocs mèneraient à la même machine, ce qu'on + venait justement d'enlever.""" + + def setUp(self): + import os + import sys + import tempfile + + sys.argv = ["todo.py"] + from script.todo.todo import TODO + + self.maison = tempfile.mkdtemp() + os.makedirs(os.path.join(self.maison, ".ssh")) + self._vrai = os.environ.get("HOME") + os.environ["HOME"] = self.maison + self.todo = TODO.__new__(TODO) + + def tearDown(self): + import os + import shutil + + if self._vrai is not None: + os.environ["HOME"] = self._vrai + shutil.rmtree(self.maison, ignore_errors=True) + + def _hosts(self): + import os + + with open( + os.path.join(self.maison, ".ssh/config"), encoding="utf-8" + ) as fh: + return [ + ligne.rstrip() for ligne in fh if ligne.startswith("Host ") + ] + + def test_the_old_short_entry_is_retired(self): + # L'état d'avant : une entrée écrite sous l'ancienne convention. + self.todo._write_ssh_config_entry( + ["vm-a"], "erplibre", "10.10.10.151", proxy_jump="pve9" + ) + perime = self.todo._pve_alias_perime("vm-a", "pve9") + self.assertEqual(perime, ["vm-a"]) + self.todo._write_ssh_config_entry( + ["pve9+vm-a"], + "erplibre", + "10.10.10.151", + proxy_jump="pve9", + also_drop=perime, + ) + self.assertEqual(self._hosts(), ["Host pve9+vm-a"]) + + def test_a_local_vm_of_the_same_name_is_left_alone(self): + # Sans ProxyJump vers cet hôte, le bloc n'est pas le nôtre : on n'y + # touche pas, même s'il porte exactement ce nom. + self.todo._write_ssh_config_entry(["vm-a"], "erplibre", "192.168.1.9") + self.assertEqual(self.todo._pve_alias_perime("vm-a", "pve9"), []) + self.todo._write_ssh_config_entry( + ["pve9+vm-a"], + "erplibre", + "10.10.10.151", + proxy_jump="pve9", + also_drop=self.todo._pve_alias_perime("vm-a", "pve9"), + ) + self.assertEqual(self._hosts(), ["Host vm-a", "Host pve9+vm-a"]) + + def test_another_hosts_vm_is_left_alone(self): + self.todo._write_ssh_config_entry( + ["vm-a"], "erplibre", "10.0.0.9", proxy_jump="pve7" + ) + self.assertEqual(self.todo._pve_alias_perime("vm-a", "pve9"), []) + + 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 d22f84e..1010820 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -261,6 +261,53 @@ class TestTroisVmSurUnProxmox(unittest.TestCase): ) self.assertIn("vm-a", stats) + def test_the_odoo_flag_survives_a_partly_closed_fleet(self): + """Le cas rapporté, et il est le cas NORMAL. + + La sonde est le dernier maillon : elle boucle sur toutes les adresses + et son code est celui de la DERNIÈRE. Un parc où une seule VM n'a pas + d'Odoo — un hyperviseur imbriqué, par exemple — finit donc en échec, + et le relevé entier partait, drapeaux Odoo compris. Le navigateur, lui, + répondait 303.""" + sortie = ( + '[{"vmid":100,"name":"vm-a","status":"running","maxmem":1024,' + '"mem":512,"maxdisk":2048,"diskwrite":10},' + '{"vmid":102,"name":"pve-imbrique","status":"running",' + '"maxmem":1024,"mem":512,"maxdisk":2048,"diskwrite":10}]\n' + "---ERPLIBRE-DU---\n" + "---ERPLIBRE-ODOO---\nODOO 10.10.10.150\n" + ) + mon._PVE_CACHE.update({"at": 0.0, "stats": {}, "ok": False}) + vms = [ + { + "name": "vm-a", + "pve": { + "target": "h", + "sudo": "", + "vmid": 100, + "addr": "10.10.10.150", + }, + }, + { + "name": "pve-imbrique", + "pve": { + "target": "h", + "sudo": "", + "vmid": 102, + "addr": "10.10.10.152", + }, + }, + ] + with mock.patch( + "script.proxmox.proxmox_deploy.run", return_value=(1, sortie) + ): + stats, ok = mon.read_pvestats_detail(vms, now=30.0) + self.assertTrue(ok) + self.assertTrue(stats["vm-a"]["odoo"], "la VM qui répond doit être 🟢") + self.assertFalse( + stats["pve-imbrique"]["odoo"], "un hyperviseur n'a pas d'Odoo" + ) + def test_a_broken_pvesh_is_still_refuted(self): # L'autre sens tient toujours : sans liste analysable, pas de réponse. mon._PVE_CACHE.update({"at": 0.0, "stats": {}, "ok": False})