[FIX] suivi : un relevé Proxmox jeté, et deux lignes qui montraient une autre machine

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.

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, un code 0 pris
pour une réponse ; exiger 0 était la même faute retournée. Seule une liste de
ressources analysable prouve une réponse.

Pendant ce temps, « virsh domstats » indexe par NOM, et un nom se partage :
les deux VM qui avaient un homonyme LOCAL affichaient ses chiffres. Mesuré —
1,5 Gio de RAM sur 12 et 58 Gio de disque sur 65, quand la vraie tournait avec
3 Gio et 25. Les relevés locaux d'une VM qui vit ailleurs sont donc retirés
AVANT d'ajouter ceux de l'hôte : un hôte muet laisse la colonne VIDE, ce qui
est vrai. Une colonne vide se remarque ; une colonne juste et fausse, non.

L'alias enfin. Prendre le nom court quand il se trouvait libre donnait un parc
incohérent : sur ce même déploiement, deux VM ont reçu « 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 nom chaîné est systématique.

--- EN ---

Of three VMs on one Proxmox, only one had empty columns — and the other two
showed ANOTHER machine's figures. Two defects, one the mirror of an earlier
fix.

A remote pipeline's exit code is its LAST link's, the Odoo probe. While Odoo
is not listening — that is, during the WHOLE install, exactly when you are
watching — the loop ends in failure and the reading, perfectly good, was
thrown away. We had fixed the opposite error, a 0 taken for an answer;
demanding 0 was the same mistake reversed. Only a parsable resource list
proves an answer.

Meanwhile "virsh domstats" indexes by NAME, and a name is shared: the two VMs
with a LOCAL namesake displayed its figures. Measured — 1.5 GiB of RAM out of
12 and 58 GiB of disk out of 65, while the real one ran on 3 GiB and 25. Local
readings for a VM that lives elsewhere are therefore dropped BEFORE the host's
are added: a silent host leaves the column EMPTY, which is true. An empty
column gets noticed; a plausible wrong one does not.

The alias, finally. Taking the short name while it happened to be free gave an
inconsistent fleet: in that same deployment two VMs got "host+vm" — their
names were held by local domains — and the third its short name. A convention
that depends on what happens to sit in the file is not a convention. The
chained name is now systematic.

Assisted-by: Claude Opus 5

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=""):
+    def _pve_alias_names(self, nom, chaine, locaux=(), rebond=""):
-        """UN seul nom pour l'entrée ~/.ssh/config, et lequel.
+        """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é
+        Deux noms sur la même ligne « Host » — le chaîné et le court —
-        « hôte+vm » — étaient un doublon dès que le court était libre : ssh
+        étaient un doublon : ssh n'a besoin que d'un nom, et le second
-        n'a besoin que d'un nom, et le second n'ajoutait qu'une façon de plus
+        n'ajoutait qu'une façon de plus d'écrire la même adresse. Rapporté.
-        d'écrire la même adresse. Rapporté.
 
-        Le court quand il est LIBRE, c'est celui qu'on tape ; le chaîné
+        Reste à choisir lequel, et c'est le chaîné. Prendre le nom court
-        sinon, car un nom déjà pris désigne une AUTRE machine — une VM locale
+        quand il se trouvait libre donnait un parc INCOHÉRENT : sur un même
-        du même nom, ou la VM d'un autre hôte Proxmox. Vécu : « ssh » partait
+        déploiement, deux VM recevaient « hôte+vm » — leurs noms étaient pris
-        vers la machine locale qui partageait le nom.
+        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
+        Le chaîné est donc systématique. Il dit où la machine vit, il ne peut
-        présence : notre propre entrée, réécrite à chaque déploiement, se
+        rien voler à un domaine local, et deux VM du même nom sur deux hôtes
-        serait autrement prise pour une rivale — et le nom aurait basculé
+        Proxmox différents se distinguent d'elles-mêmes.
-        d'une fois sur l'autre.
 
-        Rend (noms, volé) — `volé` nomme ce qui a forcé le nom chaîné, pour
+        Rend (noms, volé) — la seconde valeur reste pour l'appelant, qui
-        que l'appelant le dise plutôt que de laisser la surprise."""
+        signale au passage un nom qu'une VM locale porte aussi."""
-        if nom in locaux:
+        return [chaine], (t("a local VM") if nom in locaux else "")
-            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")
 
     def _pve_set_timezone(self, cible, spec):
         """Pose le fuseau DANS la VM, par ssh.

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
+        # Le code de sortie ne prouve RIEN, dans AUCUN sens. La commande
-        # (pvesh ; echo ; du ; echo ; boucle), et son code est celui du DERNIER
+        # est une SUITE (pvesh ; echo ; du ; echo ; boucle) et son code est
-        # maillon. Un pvesh en panne rendait donc « l'hôte a répondu, la VM
+        # celui du DERNIER maillon — la sonde Odoo. Un pvesh en panne rendait
-        # n'y est plus » — et trois tours plus tard, la poubelle. Ce qui prouve
+        # donc 0, « l'hôte a répondu, la VM n'y est plus », et trois tours
-        # une réponse, c'est une LISTE de ressources analysable.
+        # plus tard la poubelle ; c'est ce qu'on avait corrigé. Mais
-        if code == 0 and _resources_parsable(sortie):
+        # 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.

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 "".

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é
+    """L'entrée portait DEUX noms sur sa ligne « Host », puis le mauvais.
-    « hôte+vm » et le nom court.
 
-    Rapporté : « Host erplibre-proxmox-9+erplibre-arch-latest
+    D'abord le doublon : « Host erplibre-proxmox-9+erplibre-arch-latest
-    erplibre-arch-latest ». Le second est un doublon dès que le premier
+    erplibre-arch-latest ». ssh n'a besoin que d'un nom, et le second
-    suffit — ssh n'a besoin que d'un nom, et le doubler n'ajoute qu'une façon
+    n'ajoutait qu'une façon de plus d'écrire la même adresse.
-    de plus d'écrire la même adresse.
 
-    Un seul, donc, et le bon : le court quand il est LIBRE, le chaîné quand
+    Puis le choix. Prendre le nom COURT quand il se trouvait libre donnait un
-    il désignerait une autre machine. « Pris » se juge sur le ProxyJump du
+    parc incohérent : sur un même déploiement de trois VM, deux recevaient
-    bloc, pas sur sa seule présence — sinon notre propre entrée, réécrite à
+    « hôte+vm » — leurs noms étaient pris par des domaines locaux — et la
-    chaque déploiement, se prendrait pour une rivale et le nom basculerait
+    troisième son nom court. Une convention qui dépend de ce qui traîne dans
-    d'une fois sur l'autre."""
+    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()
+        todo = TODO.__new__(TODO)
-        os.makedirs(os.path.join(self.maison, ".ssh"))
+        return todo._pve_alias_names(nom, f"pve9+{nom}", set(locaux), "pve9")
-        self._vrai_home = os.environ.get("HOME")
-        os.environ["HOME"] = self.maison
-        self.todo = TODO.__new__(TODO)
 
-    def tearDown(self):
+    def test_one_name_and_it_is_the_chained_one(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):
         noms, vole = self._choisit("erplibre-arch-latest")
-        self.assertEqual(noms, ["erplibre-arch-latest"])
+        self.assertEqual(noms, ["pve9+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.assertFalse(vole)
 
-    def test_a_local_vm_keeps_its_name(self):
+    def test_a_fleet_gets_one_single_convention(self):
-        # Vécu : « ssh » partait vers la machine locale du même nom.
+        # Le défaut rapporté : trois VM du même déploiement, deux nommées
-        noms, vole = self._choisit(
+        # 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 <nom court> » 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
+            # Hermétique : le choix du nom lit la liste des domaines
-            # domaines locaux. Sans ces deux bouchons, le test dépendrait de
+            # locaux. Sans ce bouchon, le test dépendrait de la machine qui
-            # la machine qui le lance.
+            # le lance.
-            todo._ssh_config_block = lambda nom: {}
             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é
+        # UN nom, et le chaîné : « hôte+vm » dit où la machine vit et ne
-        # « hôte+vm » ne sort que lorsqu'il faut départager (voir
+        # 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.

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.