diff --git a/script/todo/proxmox_menu.py b/script/todo/proxmox_menu.py index d45274e..76754d6 100644 --- a/script/todo/proxmox_menu.py +++ b/script/todo/proxmox_menu.py @@ -1121,35 +1121,27 @@ class ProxmoxMenuMixin: fh.write("\n".join(entete) + "\n") return chemin - def _pve_alias_names(self, nom, chaine, locaux, rebond=""): - """UN seul nom pour l'entrée ~/.ssh/config, et lequel. + def _pve_alias_names(self, nom, chaine, locaux=(), rebond=""): + """UN seul nom pour l'entrée ~/.ssh/config : « hôte+vm ». - Deux noms sur la même ligne « Host » — le court et le chaîné - « hôte+vm » — étaient un doublon dès que le court était libre : ssh - n'a besoin que d'un nom, et le second n'ajoutait qu'une façon de plus - d'écrire la même adresse. Rapporté. + Deux noms sur la même ligne « Host » — le chaîné et le court — + étaient un doublon : ssh n'a besoin que d'un nom, et le second + n'ajoutait qu'une façon de plus d'écrire la même adresse. Rapporté. - Le court quand il est LIBRE, c'est celui qu'on tape ; le chaîné - sinon, car un nom déjà pris désigne une AUTRE machine — une VM locale - du même nom, ou la VM d'un autre hôte Proxmox. Vécu : « ssh » partait - vers la machine locale qui partageait le nom. + Reste à choisir lequel, et c'est le chaîné. Prendre le nom court + quand il se trouvait libre donnait un parc INCOHÉRENT : sur un même + déploiement, deux VM recevaient « hôte+vm » — leurs noms étaient pris + par des domaines locaux — et la troisième son nom court. Rapporté + aussi. Une convention qui dépend de ce qui traîne dans le fichier + n'est pas une convention. - « Pris » se juge sur le ProxyJump du bloc et non sur sa seule - présence : notre propre entrée, réécrite à chaque déploiement, se - serait autrement prise pour une rivale — et le nom aurait basculé - d'une fois sur l'autre. + Le chaîné est donc systématique. Il dit où la machine vit, il ne peut + rien voler à un domaine local, et deux VM du même nom sur deux hôtes + Proxmox différents se distinguent d'elles-mêmes. - Rend (noms, volé) — `volé` nomme ce qui a forcé le nom chaîné, pour - que l'appelant le dise plutôt que de laisser la surprise.""" - if nom in locaux: - return [chaine], t("a local VM") - bloc = self._ssh_config_block(nom) - notre = ( - not bloc - or chaine in bloc.get("names", ()) - or (rebond and bloc.get("proxyjump") == rebond) - ) - return ([nom], "") if notre else ([chaine], "~/.ssh/config") + Rend (noms, volé) — la seconde valeur reste pour l'appelant, qui + signale au passage un nom qu'une VM locale porte aussi.""" + return [chaine], (t("a local VM") if nom in locaux else "") def _pve_set_timezone(self, cible, spec): """Pose le fuseau DANS la VM, par ssh. diff --git a/script/todo/qemu_install_monitor.py b/script/todo/qemu_install_monitor.py index c2a9849..8e7e9ae 100644 --- a/script/todo/qemu_install_monitor.py +++ b/script/todo/qemu_install_monitor.py @@ -1500,6 +1500,25 @@ def read_pvestats(vms, now=None) -> dict: return _read_pvestats(vms, now)[0] +def drop_local_twins(stats, vms) -> dict: + """Retire des relevés LOCAUX ceux d'une VM qui vit ailleurs. + + « virsh domstats » indexe par NOM, et un nom se partage : une VM posée + sur un Proxmox distant héritait des chiffres du domaine local homonyme. + Vécu sur trois VM — « erplibre-ubuntu-2604 » affichait 1,5 Gio de RAM sur + 12 et 58 Gio de disque sur 65, tout cela appartenant à la machine locale + du même nom, pendant que la vraie tournait avec 3 Gio et 25. + + Retirés AVANT d'ajouter ceux de l'hôte : ainsi un hôte muet laisse la + colonne VIDE — ce qui est vrai — au lieu de la remplir avec la mauvaise + machine. Une colonne vide se remarque ; une colonne juste et fausse, non. + """ + for vm in vms or (): + if vm.get("pve"): + stats.pop(vm.get("name"), None) + return stats + + def _read_pvestats(vms, now=None): """({nom: relevé}, succès). Un appel par hôte, mis en cache PVE_STATS_INTERVAL secondes. @@ -1547,17 +1566,25 @@ def _read_pvestats(vms, now=None): == target ) ] - code, sortie = pve.run( + _code, sortie = pve.run( {"target": target, "sudo": sudo, "jump": info.get("jump", "")}, pve_stats_cmd(siennes), 40, ) - # « 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): + # Le code de sortie ne prouve RIEN, dans AUCUN sens. La commande + # est une SUITE (pvesh ; echo ; du ; echo ; boucle) et son code est + # celui du DERNIER maillon — la sonde Odoo. Un pvesh en panne rendait + # donc 0, « l'hôte a répondu, la VM n'y est plus », et trois tours + # plus tard la poubelle ; c'est ce qu'on avait corrigé. Mais + # l'exiger à 0 était l'erreur SYMÉTRIQUE : tant qu'Odoo n'écoute pas + # — c'est-à-dire pendant TOUTE l'installation, précisément quand on + # regarde — la boucle finit en échec et le relevé, parfait, était + # jeté. Mesuré sur trois VM : colonnes vides côté Proxmox, et les + # lignes qui avaient un homonyme LOCAL affichaient ses chiffres. + # + # Ce qui prouve une réponse, c'est une LISTE de ressources + # analysable. Rien d'autre, et surtout pas le code. + if _resources_parsable(sortie): ok = True releves = parse_pvestats(sortie) ouverts = parse_odoo_probe(sortie) @@ -2367,6 +2394,7 @@ def run_monitor(manifest_path: str, run_app: bool = True): # calcule sur les relevés successifs, donc il faut échantillonner # à chaque tour (2 s) et non au rythme lent des états. stats = parse_domstats(read_domstats()) + drop_local_twins(stats, vms) # Les VM d'un hôte Proxmox distant : virsh ne les voit pas, leurs # colonnes restaient vides. Même forme de relevé, donc la suite ne # change pas d'un iota. diff --git a/script/todo/todo.py b/script/todo/todo.py index fef5f1c..0249191 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -1705,40 +1705,6 @@ 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. Un nom court déjà déclaré peut être notre - propre entrée qu'on réécrit — auquel cas il n'y a rien de volé — ou - celle d'une autre machine, et c'est le ProxyJump qui les distingue. - - {"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 "". diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index aeb0358..6e5c986 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -15,7 +15,6 @@ aucun hôte Proxmox n'est joint. import asyncio import sys -import os import unittest sys.argv = ["todo.py"] @@ -735,92 +734,56 @@ class TestLEcranDUneVmProxmox(unittest.TestCase): class TestUnSeulNomDansSshConfig(unittest.TestCase): - """L'entrée portait DEUX noms sur sa ligne « Host » : le nom chaîné - « hôte+vm » et le nom court. + """L'entrée portait DEUX noms sur sa ligne « Host », puis le mauvais. - Rapporté : « Host erplibre-proxmox-9+erplibre-arch-latest - erplibre-arch-latest ». Le second est un doublon dès que le premier - suffit — ssh n'a besoin que d'un nom, et le doubler n'ajoute qu'une façon - de plus d'écrire la même adresse. + D'abord le doublon : « Host erplibre-proxmox-9+erplibre-arch-latest + erplibre-arch-latest ». ssh n'a besoin que d'un nom, et le second + n'ajoutait qu'une façon de plus d'écrire la même adresse. - Un seul, donc, et le bon : le court quand il est LIBRE, le chaîné quand - il désignerait une autre machine. « Pris » se juge sur le ProxyJump du - bloc, pas sur sa seule présence — sinon notre propre entrée, réécrite à - chaque déploiement, se prendrait pour une rivale et le nom basculerait - d'une fois sur l'autre.""" + Puis le choix. Prendre le nom COURT quand il se trouvait libre donnait un + parc incohérent : sur un même déploiement de trois VM, deux recevaient + « hôte+vm » — leurs noms étaient pris par des domaines locaux — et la + troisième son nom court. Une convention qui dépend de ce qui traîne dans + le fichier n'est pas une convention. Le chaîné est systématique.""" - def setUp(self): + def _choisit(self, nom, locaux=()): 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_home = os.environ.get("HOME") - os.environ["HOME"] = self.maison - self.todo = TODO.__new__(TODO) + todo = TODO.__new__(TODO) + return todo._pve_alias_names(nom, f"pve9+{nom}", set(locaux), "pve9") - def tearDown(self): - import shutil - - if self._vrai_home is not None: - os.environ["HOME"] = self._vrai_home - shutil.rmtree(self.maison, ignore_errors=True) - - def _ecrit(self, noms, rebond=""): - self.todo._write_ssh_config_entry( - noms, "erplibre", "10.10.10.150", proxy_jump=rebond or None - ) - - def _lignes_host(self): - 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 _choisit(self, nom, locaux=(), rebond="pve9"): - return self.todo._pve_alias_names( - nom, f"pve9+{nom}", set(locaux), rebond - ) - - def test_a_free_name_is_written_alone(self): + def test_one_name_and_it_is_the_chained_one(self): noms, vole = self._choisit("erplibre-arch-latest") - self.assertEqual(noms, ["erplibre-arch-latest"]) - self.assertFalse(vole) - self._ecrit(noms, "pve9") - self.assertEqual(self._lignes_host(), ["Host erplibre-arch-latest"]) - - def test_redeploying_the_same_vm_keeps_the_same_name(self): - # Le piège du correctif : notre propre bloc déclare déjà le nom. - self._ecrit(["erplibre-arch-latest"], "pve9") - noms, vole = self._choisit("erplibre-arch-latest") - self.assertEqual(noms, ["erplibre-arch-latest"], "le nom a basculé") + self.assertEqual(noms, ["pve9+erplibre-arch-latest"]) self.assertFalse(vole) - def test_a_local_vm_keeps_its_name(self): - # Vécu : « ssh » partait vers la machine locale du même nom. - noms, vole = self._choisit( + def test_a_fleet_gets_one_single_convention(self): + # Le défaut rapporté : trois VM du même déploiement, deux nommées + # d'une façon et la troisième d'une autre. + noms = [ + self._choisit(n, locaux=("erplibre-ubuntu-2604",))[0][0] + for n in ( + "erplibre-ubuntu-2604", + "erplibre-arch-latest", + "erplibre-proxmox-9", + ) + ] + self.assertTrue( + all(n.startswith("pve9+") for n in noms), + f"un parc, une convention : {noms}", + ) + + def test_a_local_namesake_is_still_named(self): + # Le nom chaîné ne lui vole rien, mais on le DIT : c'est ce qui + # explique pourquoi « ssh » va ailleurs. + _noms, vole = self._choisit( "erplibre-arch-latest", locaux=("erplibre-arch-latest",) ) - self.assertEqual(noms, ["pve9+erplibre-arch-latest"]) self.assertTrue(vole) - def test_another_proxmox_host_keeps_its_name(self): - self._ecrit(["erplibre-ubuntu-2604"], "pve7") - noms, vole = self._choisit("erplibre-ubuntu-2604") - self.assertEqual(noms, ["pve9+erplibre-ubuntu-2604"]) - self.assertEqual(vole, "~/.ssh/config") - self._ecrit(noms, "pve9") - # Les deux machines cohabitent, chacune sous son nom. - self.assertEqual( - self._lignes_host(), - ["Host erplibre-ubuntu-2604", "Host pve9+erplibre-ubuntu-2604"], - ) - def test_no_deploy_path_writes_two_names_anymore(self): import re from pathlib import Path as P @@ -1008,10 +971,9 @@ class TestLeSuivi(unittest.TestCase): nom ) todo._ssh_private_key = lambda k: None - # Hermétique : le choix du nom lit ~/.ssh/config et la liste des - # domaines locaux. Sans ces deux bouchons, le test dépendrait de - # la machine qui le lance. - todo._ssh_config_block = lambda nom: {} + # Hermétique : le choix du nom lit la liste des domaines + # locaux. Sans ce bouchon, le test dépendrait de la machine qui + # le lance. todo._qemu_list_domains = lambda: [] todo._pve_guest_ip = lambda vmid, attente=120: "" todo._qemu_install_erplibre_monitored = lambda *a, **k: None @@ -1036,16 +998,16 @@ class TestLeSuivi(unittest.TestCase): return ecrites cmd = {"branch": "develop", "cmd": "make x", "label": "X"} - # UN nom : le court, puisque rien ne le porte déjà. Le chaîné - # « hôte+vm » ne sort que lorsqu'il faut départager (voir + # UN nom, et le chaîné : « hôte+vm » dit où la machine vit et ne + # dépend pas de ce qui traîne dans ~/.ssh/config (voir # TestUnSeulNomDansSshConfig). - self.assertEqual(essai(False, cmd, False), [["vm-a"]]) + self.assertEqual(essai(False, cmd, False), [["pve1+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"]]) # 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"]]) 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. diff --git a/test/test_qemu_monitor_pve.py b/test/test_qemu_monitor_pve.py index ef94522..d22f84e 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -230,6 +230,80 @@ class TestLesAutresCheminsVersLaPoubelle(unittest.TestCase): self.assertGreaterEqual(mon.PVE_ABSENCES_AVANT_EFFACEE, 2) +class TestTroisVmSurUnProxmox(unittest.TestCase): + """Rapporté à l'usage : sur trois VM d'un même Proxmox, une seule avait + ses colonnes vides — et les deux autres montraient les chiffres d'une + AUTRE machine. + + Deux fautes, dont une était le miroir d'un correctif précédent.""" + + def test_a_reading_stands_even_when_odoo_is_not_up_yet(self): + # Le code de sortie de la suite distante est celui de son DERNIER + # maillon : la sonde Odoo. Tant qu'Odoo n'écoute pas — c'est-à-dire + # pendant TOUTE l'installation, précisément quand on regarde — la + # boucle finit en échec et le relevé, parfait, était jeté. + # + # On avait corrigé l'erreur inverse (code 0 pris pour une réponse) ; + # exiger 0 était la même faute, retournée. + sortie = ( + '[{"vmid":101,"name":"vm-a","status":"running","maxmem":1024,' + '"mem":512,"maxdisk":2048,"diskwrite":10}]\n' + "---ERPLIBRE-DU---\n---ERPLIBRE-ODOO---\n" + ) + mon._PVE_CACHE.update({"at": 0.0, "stats": {}, "ok": False}) + vm = {"name": "vm-a", "pve": {"target": "h", "sudo": "", "vmid": 101}} + with mock.patch( + "script.proxmox.proxmox_deploy.run", return_value=(1, sortie) + ): + stats, ok = mon.read_pvestats_detail([vm], now=10.0) + self.assertTrue( + ok, "un code non nul ne réfute pas une réponse lisible" + ) + self.assertIn("vm-a", stats) + + 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}) + vm = {"name": "vm-a", "pve": {"target": "h", "sudo": "", "vmid": 101}} + with mock.patch( + "script.proxmox.proxmox_deploy.run", + return_value=(0, "permission denied\n---ERPLIBRE-DU---\n"), + ): + _stats, ok = mon.read_pvestats_detail([vm], now=20.0) + self.assertFalse(ok) + + def test_a_remote_vm_never_borrows_a_local_namesake(self): + # « virsh domstats » indexe par NOM, et un nom se partage. Mesuré : + # « erplibre-ubuntu-2604 » sur Proxmox affichait 1,5 Gio sur 12 et + # 58 Gio de disque — ceux de la machine locale du même nom — quand la + # vraie tournait avec 3 Gio et 25. + locaux = { + "erplibre-ubuntu-2604": { + "ram_used": 1 << 30, + "ram_total": 12 << 30, + }, + "erplibre-arch-latest": {"ram_used": 5, "ram_total": 9}, + "vm-locale": {"ram_used": 7, "ram_total": 8}, + } + vms = [ + {"name": "erplibre-ubuntu-2604", "pve": {"vmid": 100}}, + {"name": "vm-locale"}, + ] + reste = mon.drop_local_twins(dict(locaux), vms) + self.assertNotIn("erplibre-ubuntu-2604", reste) + # Une VM locale garde les siens, et une VM étrangère au manifeste + # n'est pas touchée. + self.assertIn("vm-locale", reste) + self.assertIn("erplibre-arch-latest", reste) + + def test_a_silent_host_leaves_the_column_empty(self): + # Vide, c'est vrai. Une colonne vide se remarque ; une colonne juste + # et fausse, non — c'est ce qui a fait remonter le défaut. + stats = {"vm-a": {"ram_used": 1, "ram_total": 2}} + mon.drop_local_twins(stats, [{"name": "vm-a", "pve": {"vmid": 1}}]) + self.assertEqual(stats, {}) + + class TestEffacerDepuisUnSuiviRouvert(unittest.TestCase): """Le suivi se ROUVRE sur un manifeste passé — c'est fait pour.