diff --git a/LongTest/deep_proxmox.py b/LongTest/deep_proxmox.py index 9bd0d26..4f5fc94 100755 --- a/LongTest/deep_proxmox.py +++ b/LongTest/deep_proxmox.py @@ -130,9 +130,10 @@ def alias_etage(niveau, parent_alias): class Descente: """Un étage après l'autre, et ce qu'on en sait.""" - def __init__(self, plan, journal, dry_run=False): + def __init__(self, plan, journal, dry_run=False, chemin_json=None): self.plan = plan self.journal = journal + self.chemin_json = chemin_json self.dry_run = dry_run self.etages = [] self.interrompu = False @@ -520,6 +521,8 @@ class Descente: self.interrompu = True break alias = nom + # Le domaine libvirt existe : le rapport doit exister aussi. + self._sauver(etage) else: prepare = self.preparer_parent(parent) if not prepare: @@ -538,6 +541,7 @@ class Descente: # sert. Sans lui, une VM abandonnée juste après « qm create » # n'était nommée nulle part. etage["parent_alias"] = parent_alias + self._sauver(etage) alias = alias_etage(niveau, parent_alias) if not self.dry_run: self.ecrire_alias(alias, adresse, parent_alias) @@ -560,6 +564,7 @@ class Descente: ("pmxcfs", lambda: self.reparer_pmxcfs(cible)), ): etage["etape"] = etape + self._sauver(etage) if not action(): self.etages.append(etage) return self.rapport(interrompu=True) @@ -571,6 +576,7 @@ class Descente: etage["ok"] = not self.dry_run etage["secondes"] = int(time.time() - debut) self.etages.append(etage) + self._sauver() self.dire(f" ✓ étage {niveau} en {etage['secondes']} s") parent, parent_alias = cible, alias return self.rapport(interrompu=self.interrompu) @@ -589,8 +595,52 @@ class Descente: identity_file=prive, ) + def _etat(self, interrompu, en_cours=None): + """Le rapport, à cet instant. `en_cours` : l'étage pas encore rangé.""" + etages = list(self.etages) + if en_cours is not None and en_cours not in etages: + etages.append(en_cours) + return { + "demandee": self.plan["demandee"], + "atteignable": self.plan["atteignable"], + "atteinte": sum(1 for e in etages if e.get("ok")), + "interrompu": interrompu, + # Sans ce champ, un rapport d'essai à blanc se lisait comme une + # descente réussie — et « --detruire » s'en servait. + "dry_run": self.dry_run, + "etages": etages, + } + + def _sauver(self, en_cours=None): + """Écrit le rapport PARTIEL, dès qu'une VM existe. + + Il ne s'écrivait qu'à la fin. Une descente tuée au quatrième étage — + c'est arrivé — laissait quatre machines réelles et « --detruire » + répondait « aucun rapport : rien à défaire » : le seul enregistrement + du couple (alias du parent, VMID) mourait avec le processus. Il fallait + alors les retrouver et les détruire à la main, c'est-à-dire par leur + nom, ce que tout le reste de ce fichier s'applique à ne pas faire. + + Marqué « interrompu » jusqu'au bout : un rapport partiel ne doit jamais + se lire comme une descente terminée. + """ + if self.dry_run or not self.chemin_json: + return + temporaire = self.chemin_json + ".tmp" + try: + with open(temporaire, "w", encoding="utf-8") as fh: + json.dump( + self._etat(interrompu=True, en_cours=en_cours), + fh, + indent=2, + ) + os.replace(temporaire, self.chemin_json) + except OSError as err: + self.dire(f" ⚠ rapport non écrit : {err}") + def rapport(self, interrompu=False): - atteint = sum(1 for e in self.etages if e["ok"]) + etat = self._etat(interrompu) + atteint = etat["atteinte"] print("") if self.dry_run: self.dire( @@ -611,16 +661,7 @@ class Descente: f"{e.get('secondes', '—')} s" if e["ok"] else e["etape"] ) self.dire(f" {marque} étage {e['niveau']:2d} {detail}") - return { - "demandee": self.plan["demandee"], - "atteignable": self.plan["atteignable"], - "atteinte": atteint, - "interrompu": interrompu, - # Sans ce champ, un rapport d'essai à blanc se lisait comme une - # descente réussie — et « --detruire » s'en servait. - "dry_run": self.dry_run, - "etages": self.etages, - } + return etat def dernier_rapport(): @@ -868,9 +909,9 @@ def principal(argv=None): print(f"\n journal : {journal}") if args.dry_run: print(" --dry-run : rien ne sera créé.\n") - descente = Descente(plan, journal, args.dry_run) - rapport = descente.parcourir() chemin = journal[:-4] + ("-dryrun.json" if args.dry_run else ".json") + descente = Descente(plan, journal, args.dry_run, chemin) + rapport = descente.parcourir() with open(chemin, "w", encoding="utf-8") as fh: json.dump(rapport, fh, indent=2) print(f"\n rapport : {chemin}\n") diff --git a/script/todo/todo.py b/script/todo/todo.py index cd9acf1..2a2dd43 100755 --- a/script/todo/todo.py +++ b/script/todo/todo.py @@ -1468,6 +1468,18 @@ class TODO( existing = self._ssh_config_drop_hosts( existing, names + [n for n in also_drop if n not in names] ).rstrip("\n") + if not names: + # Retirer sans réécrire est un appel légitime : les machines + # n'existent plus. Sans ce retour, un « Host » NU était écrit dans + # le ~/.ssh/config de l'utilisateur — un bloc sans nom, suivi d'un + # « HostName » vide, qui s'applique alors à rien et brouille la + # lecture du fichier. + with open(cfg, "w", encoding="utf-8") as fh: + fh.write(existing + "\n" if existing else "") + os.chmod(cfg, 0o600) + retires = ", ".join(also_drop) + print(f"🗑 {t('Removed from ~/.ssh/config:')} {retires}") + return block = ( f"Host {' '.join(names)}\n" f" HostName {ip}\n" diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index b292f80..d94bf33 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -278,6 +278,10 @@ TRANSLATIONS = { "fr": "Ajouté à ~/.ssh/config :", "en": "Added to ~/.ssh/config:", }, + "Removed from ~/.ssh/config:": { + "fr": "Retiré de ~/.ssh/config :", + "en": "Removed from ~/.ssh/config:", + }, "SSH address input method": { "fr": "Méthode de saisie de l'adresse SSH", "en": "SSH address input method", diff --git a/test/test_proxmox_form.py b/test/test_proxmox_form.py index adbc7fc..7276470 100644 --- a/test/test_proxmox_form.py +++ b/test/test_proxmox_form.py @@ -1057,6 +1057,48 @@ class TestLAncienNomSEnVa(unittest.TestCase): ligne.rstrip() for ligne in fh if ligne.startswith("Host ") ] + def test_dropping_the_last_entry_writes_no_nameless_block(self): + """Retirer sans réécrire est un appel légitime : les machines + n'existent plus. + + Constaté dans le vrai ~/.ssh/config de l'utilisateur : l'appel écrivait + « Host » NU, suivi d'un « HostName » vide, puis mourait sur un + IndexError en annonçant l'ajout. Le bloc sans nom s'applique à rien et + brouille la lecture du fichier.""" + import os + + self.todo._write_ssh_config_entry( + ["deep-1"], "erplibre", "10.10.10.150" + ) + self.todo._write_ssh_config_entry( + ["deep-2"], "erplibre", "10.10.10.151", proxy_jump="deep-1" + ) + self.todo._write_ssh_config_entry( + [], "erplibre", "", also_drop=("deep-1", "deep-2") + ) + self.assertEqual(self._hosts(), []) + with open( + os.path.join(self.maison, ".ssh/config"), encoding="utf-8" + ) as fh: + reste = fh.read() + self.assertNotIn("Host", reste) + self.assertNotIn("HostName", reste) + # Et le fichier garde ses droits : ssh refuse un config trop ouvert. + self.assertEqual( + oct(os.stat(os.path.join(self.maison, ".ssh/config")).st_mode)[ + -3: + ], + "600", + ) + + def test_dropping_one_entry_leaves_the_others_untouched(self): + for nom, ip in (("garde-a", "10.0.0.1"), ("part", "10.0.0.2")): + self.todo._write_ssh_config_entry([nom], "erplibre", ip) + self.todo._write_ssh_config_entry( + [], "erplibre", "", also_drop=("part",) + ) + self.assertEqual(self._hosts(), ["Host garde-a"]) + 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( diff --git a/test/test_todo_longtest.py b/test/test_todo_longtest.py index 7ad9c73..5be410b 100644 --- a/test/test_todo_longtest.py +++ b/test/test_todo_longtest.py @@ -10,9 +10,14 @@ sans virtualisation. Ce fichier-ci vérifie la frontière, et que l'essai à blanc du test long dit quelque chose sans rien créer. """ +import contextlib +import io +import json import os +import shutil import subprocess import sys +import tempfile import unittest sys.argv = ["todo.py"] @@ -180,9 +185,14 @@ class TestLEssaiABlanc(unittest.TestCase): self.assertEqual(rapport["atteinte"], 0) self.assertTrue(all(not e["ok"] for e in rapport["etages"])) - def test_the_first_level_is_wide_and_the_others_are_not(self): - # 12 vCPU au quatrième étage ont gelé un noyau invité ; deux - # avançaient. + def test_the_plan_shrinks_towards_the_bottom(self): + """Chaque étage annoncé est plus étroit que son parent, sur les trois + ressources. + + Deux vCPU à chaque étage imbriqué donnaient un parent aussi étroit que + son enfant : cent pour cent de surengagement, et l'hyperviseur à servir + par-dessus. Mesuré : l'installation de l'étage 4 dépassait 2 h 50 + contre 793 s pour l'étage 3.""" # Par expression exacte : la ligne « machine : … Mo … Go » du haut # contient les mêmes unités et décalait l'index d'un cran. import re @@ -193,10 +203,17 @@ class TestLEssaiABlanc(unittest.TestCase): re.M, ) self.assertEqual(len(plan), 4, plan) - niveaux = {int(n): int(v) for n, v, _r, _d in plan} - self.assertGreater(niveaux[1], 1, "le premier étage peut être large") - for niveau in (2, 3, 4): - self.assertEqual(niveaux[niveau], 2, f"étage {niveau}") + etages = sorted( + (int(n), int(v), int(r), int(d)) for n, v, r, d in plan + ) + for parent, enfant in zip(etages, etages[1:]): + for i, quoi in ((1, "vCPU"), (2, "RAM"), (3, "disque")): + self.assertGreater( + parent[i], enfant[i], f"étage {parent[0]} : {quoi}" + ) + # Et le plus profond reçoit ce qu'un Proxmox de test demande, pas ce + # qui reste. + self.assertEqual(etages[-1][1], 2, "vCPU du plus profond") class TestDefaireSansEffacerAutreChose(unittest.TestCase): @@ -272,6 +289,107 @@ class TestDefaireSansEffacerAutreChose(unittest.TestCase): self.assertIn("dry_run=args.dry_run", principal) +class TestUnRapportQuiSurvitAuProcessus(unittest.TestCase): + """Le rapport ne s'écrivait qu'à la FIN de la descente. + + Constaté : une descente de dix étages arrêtée pendant l'installation du + quatrième laissait quatre machines réelles, et « --detruire » répondait + « aucun rapport de descente : rien à défaire ». Le seul enregistrement du + couple (alias du parent, VMID) mourait avec le processus — il fallait + retrouver ces VM à la main, c'est-à-dire par leur nom, ce que tout le + reste de ce fichier s'applique à ne pas faire. + """ + + def setUp(self): + sys.path.insert(0, os.path.join(RACINE, "LongTest")) + import deep_proxmox + + self.dp = deep_proxmox + self.dossier = tempfile.mkdtemp(prefix="longtest-rapport-") + self.addCleanup(shutil.rmtree, self.dossier, ignore_errors=True) + + def _descente_tuee(self, a_l_etage): + """Une descente dont l'installation MEURT à l'étage donné. + + Rien de réel n'est touché : aucune des méthodes qui créent une machine + ou écrivent dans ~/.ssh/config n'est appelée pour de vrai. + """ + niveaux = [ + {"niveau": n, "vcpu": 2, "ram": 4096, "disque": 25} + for n in (1, 2, 3) + ] + plan = {"demandee": 3, "atteignable": 3, "niveaux": niveaux} + chemin = os.path.join(self.dossier, "rapport.json") + d = self.dp.Descente(plan, None, False, chemin) + + appels = [] + d.creer_etage1 = lambda res: "deep-pve-1" + d.preparer_parent = lambda parent: {"stockage": "local-lvm"} + d.creer_enfant = lambda parent, niveau, res, prep: ( + 100 + niveau, + f"10.10.10.{niveau}", + ) + d.ecrire_alias = lambda *a, **k: None + d.attendre_ssh = lambda cible, delai: 1 + d.redemarrer_et_verifier = lambda cible: True + d.reparer_pmxcfs = lambda cible: True + + def installer(cible): + appels.append(cible) + if len(appels) >= a_l_etage: + raise KeyboardInterrupt("descente tuée") + return True + + d.installer_proxmox = installer + with contextlib.redirect_stdout(io.StringIO()): + with self.assertRaises(KeyboardInterrupt): + d.parcourir() + with open(chemin, encoding="utf-8") as fh: + return json.load(fh) + + def test_a_killed_descent_still_names_what_it_created(self): + rapport = self._descente_tuee(a_l_etage=3) + # Le couple (parent, VMID) des étages imbriqués créés : c'est de lui + # seul que « --detruire » se sert. + self.assertEqual( + self.dp.a_defaire(rapport), + [ + (3, "deep-pve-1+deep-pve-2", 103, self.dp.nom_etage(3)), + (2, "deep-pve-1", 102, self.dp.nom_etage(2)), + ], + ) + + def test_the_report_exists_as_soon_as_the_first_vm_does(self): + """Tuée pendant l'installation de l'étage 1, il n'y a aucun VMID à + noter — mais le domaine libvirt existe, et sans rapport « --detruire » + ne le regardait même pas.""" + rapport = self._descente_tuee(a_l_etage=1) + self.assertTrue(rapport["etages"]) + self.assertEqual(self.dp.a_defaire(rapport), []) + + def test_a_partial_report_never_reads_as_a_finished_descent(self): + rapport = self._descente_tuee(a_l_etage=3) + self.assertTrue(rapport["interrompu"]) + self.assertLess(rapport["atteinte"], rapport["demandee"]) + # Et il n'est pas écarté comme un essai à blanc : c'est bien de VRAIES + # machines qu'il parle. + self.assertFalse(rapport["dry_run"]) + + def test_a_dry_run_writes_no_partial_report(self): + """Un plan n'a rien créé : lui laisser écrire un rapport ferait + détruire d'après un plan.""" + plan = { + "demandee": 1, + "atteignable": 1, + "niveaux": [{"niveau": 1, "vcpu": 2, "ram": 4096, "disque": 25}], + } + chemin = os.path.join(self.dossier, "blanc.json") + d = self.dp.Descente(plan, None, True, chemin) + with contextlib.redirect_stdout(io.StringIO()): + d._sauver({"niveau": 1, "vmid": 101, "parent_alias": "x"}) + self.assertFalse(os.path.exists(chemin)) + + class TestLeMenu(unittest.TestCase): def test_the_mixin_is_wired_into_TODO(self): todo = TODO.__new__(TODO)