mathben/erplibre
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity

[FIX] LongTest : rapport écrit VM par VM, retrait ssh sans bloc nu

Browse source 
Descente de dix étages arrêtée pendant l'installation du quatrième : quatre
machines réelles restaient, et « --detruire » répondait « aucun rapport : rien
à défaire ». Le rapport ne s'écrivait qu'à la fin, donc le seul enregistrement
du couple (alias du parent, VMID) mourait avec le processus. Il fallait les
retrouver par leur NOM, ce que tout ce fichier s'applique à éviter.

En nettoyant à la main, second défaut : appelé sans nom à écrire — un retrait
pur, légitime, les machines n'existant plus — _write_ssh_config_entry écrivait
« Host » NU suivi d'un « HostName » vide dans le ~/.ssh/config réel, puis
mourait sur IndexError en annonçant l'ajout. Constaté, puis retiré du fichier.

Les deux correctifs meurent sous mutation : 3 tests et 2 tests.

--- EN ---

A ten-level descent stopped during the fourth level's install: four real
machines were left, and "--detruire" answered "no report: nothing to undo".
The report was only written at the end, so the sole record of the (parent
alias, VMID) pair died with the process. They had to be found by NAME, which
this whole file works to avoid.

Cleaning up by hand surfaced a second defect: called with no name to write — a
pure removal, legitimate since the machines are gone — _write_ssh_config_entry
wrote a BARE "Host" followed by an empty "HostName" into the real ~/.ssh/config,
then died on IndexError while announcing the addition. Observed, then removed.

Both fixes die under mutation: 3 tests and 2 tests.

Assisted-by: claude-opus-5
(cherry picked from commit e7c8b540c630b33c6eb1b1a2f51c01497e0b3ca0)
This commit is contained in:
Mathieu Benoit 2026-08-27 05:39:08 -04:00
parent 469a5fde9e
commit d0a06c03b7
5 changed files with 238 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

69
LongTest/deep_proxmox.py
View file

@ -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")

12
script/todo/todo.py
View file

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

4
script/todo/todo_i18n.py
View file

@ -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",

42
test/test_proxmox_form.py
View file

@ -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(

132
test/test_todo_longtest.py
View file

@ -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)