[FIX] LongTest : l'étage 1 s'identifie par son UUID, pas par son nom
Dernières trouvailles de la relecture, et la famille la plus tenace de ce travail : une machine liée à ce qu'elle s'appelle plutôt qu'à ce qui l'identifie. « virsh undefine --remove-all-storage » partait sur le nom fixe deep-pve-1, quel que soit le domaine qui le porte — la VM d'une descente précédente qu'on voulait garder, ou une machine sans rapport. L'UUID est noté à la création et vérifié avant de détruire ; un rapport ancien n'en a pas, on procède alors par le nom faute de mieux, mais on le dit. Le nom des étages imbriqués était de même déduit du numéro d'étage à la RELECTURE. Un rapport écrit avant un changement de nom_etage aurait désigné des machines qui ne sont pas les siennes. Il est écrit à la création. Les deux meurent sous mutation. 52 tests dans ce fichier. --- EN --- Last findings from the review, and the most persistent family in this work: a machine bound to what it is called rather than to what identifies it. "virsh undefine --remove-all-storage" went by the fixed name deep-pve-1, whatever domain carries it — a previous descent's VM one meant to keep, or an unrelated machine. The UUID is recorded at creation and checked before destroying; an older report has none, so we fall back to the name, and say so. The nested levels' names were likewise derived from the level number at READ time. A report written before a change to nom_etage would have named machines that are not its own. It is now written at creation. Both die under mutation. 52 tests in this file. Assisted-by: claude-opus-5 (cherry picked from commit 93292653afb91351b0efa5700fcf66f6bb82604f)
This commit is contained in:
parent
178785b90a
commit
006f4d271b
2 changed files with 180 additions and 6 deletions
|
|
@ -481,6 +481,25 @@ class Descente:
|
|||
)
|
||||
return nom
|
||||
|
||||
@staticmethod
|
||||
def uuid_libvirt(nom):
|
||||
"""L'UUID du domaine `nom`, ou "". C'est lui qui l'identifie.
|
||||
|
||||
Un nom se réutilise ; un UUID non. Sans lui, « --detruire » effaçait
|
||||
« deep-pve-1 » quel qu'il soit — la VM d'une descente précédente qu'on
|
||||
voulait garder, ou une machine sans rapport qui porte ce nom.
|
||||
"""
|
||||
try:
|
||||
res = subprocess.run(
|
||||
["sudo", "-n", "virsh", "domuuid", nom],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=60,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return ""
|
||||
return "" if res.returncode else res.stdout.strip()
|
||||
|
||||
def creer_enfant(self, parent, niveau, res, prepare, noter=None):
|
||||
"""« qm create » sur le parent. Rend (vmid, adresse) ou (None, None).
|
||||
|
||||
|
|
@ -589,6 +608,10 @@ class Descente:
|
|||
self.interrompu = True
|
||||
break
|
||||
alias = nom
|
||||
etage["nom"] = nom
|
||||
# L'UUID, et non le nom : c'est de lui que « --detruire » se
|
||||
# servira. Un nom se réutilise, un UUID non.
|
||||
etage["uuid"] = self.uuid_libvirt(nom)
|
||||
# Le domaine libvirt existe : le rapport doit exister aussi.
|
||||
self._sauver(etage)
|
||||
else:
|
||||
|
|
@ -599,9 +622,19 @@ class Descente:
|
|||
self.interrompu = True
|
||||
break
|
||||
|
||||
def noter(numero, etage=etage, parent_alias=parent_alias):
|
||||
def noter(
|
||||
numero,
|
||||
etage=etage,
|
||||
parent_alias=parent_alias,
|
||||
niveau=niveau,
|
||||
):
|
||||
etage["vmid"] = numero
|
||||
etage["parent_alias"] = parent_alias
|
||||
# Le nom est ÉCRIT, non déduit du numéro d'étage à la
|
||||
# relecture : si nom_etage change un jour, un rapport
|
||||
# ancien désignerait des machines qui ne sont pas les
|
||||
# siennes.
|
||||
etage["nom"] = nom_etage(niveau)
|
||||
self._sauver(etage)
|
||||
|
||||
vmid, adresse = self.creer_enfant(
|
||||
|
|
@ -880,7 +913,10 @@ def a_defaire(rapport):
|
|||
int(e["niveau"]),
|
||||
e["parent_alias"],
|
||||
int(e["vmid"]),
|
||||
nom_etage(int(e["niveau"])),
|
||||
# Le nom ÉCRIT par la descente. Le déduire du numéro d'étage
|
||||
# supposait que nom_etage ne changera jamais — un rapport ancien
|
||||
# aurait alors nommé des machines qui ne sont pas les siennes.
|
||||
e.get("nom") or nom_etage(int(e["niveau"])),
|
||||
)
|
||||
for e in etages
|
||||
]
|
||||
|
|
@ -932,7 +968,7 @@ def detruire_une(parent_alias, vmid, nom, journal):
|
|||
return True
|
||||
|
||||
|
||||
def detruire_etage1(journal, dry_run=False):
|
||||
def detruire_etage1(journal, dry_run=False, attendu=None, nom=None):
|
||||
"""Le domaine libvirt du premier étage — le SEUL qui en soit un.
|
||||
|
||||
La boucle d'avant tournait sur trente niveaux avec une condition morte, et
|
||||
|
|
@ -940,8 +976,15 @@ def detruire_etage1(journal, dry_run=False):
|
|||
jamais rien créé : « virsh undefine --remove-all-storage » partait alors
|
||||
sur un domaine qui pouvait être n'importe quoi, sortie capturée, sans un
|
||||
mot.
|
||||
|
||||
`attendu` : l'UUID que le rapport a noté à la création. C'est LUI qui
|
||||
identifie la machine, pas son nom. Un nom se réutilise — la VM d'une
|
||||
descente précédente qu'on voulait garder, ou une machine sans rapport qui
|
||||
porte celui-là — et « --remove-all-storage » efface un disque pour de bon.
|
||||
Un rapport ancien n'a pas d'UUID : on procède alors comme avant, par le
|
||||
nom, faute de mieux, mais en le disant.
|
||||
"""
|
||||
nom = nom_etage(1)
|
||||
nom = nom or nom_etage(1)
|
||||
existe = subprocess.run(
|
||||
["sudo", "-n", "virsh", "dominfo", nom],
|
||||
capture_output=True,
|
||||
|
|
@ -950,6 +993,19 @@ def detruire_etage1(journal, dry_run=False):
|
|||
if existe.returncode:
|
||||
dire(f" — {nom} : aucun domaine libvirt", journal)
|
||||
return True
|
||||
if attendu:
|
||||
vu = Descente.uuid_libvirt(nom)
|
||||
if vu != attendu:
|
||||
dire(
|
||||
f" ✗ {nom} : UUID {vu or '—'} au lieu de {attendu} —"
|
||||
" ce n'est PAS notre machine, rien touché",
|
||||
journal,
|
||||
)
|
||||
return False
|
||||
else:
|
||||
dire(
|
||||
f" ⚠ {nom} : rapport sans UUID, identifié par son NOM", journal
|
||||
)
|
||||
if dry_run:
|
||||
dire(
|
||||
f" [à blanc] virsh undefine {nom} --remove-all-storage", journal
|
||||
|
|
@ -1015,7 +1071,15 @@ def detruire(journal=None, dry_run=False):
|
|||
f" étage {niveau:2d} {nom} ({vmid}) sur {parent_alias}",
|
||||
journal,
|
||||
)
|
||||
dire(f" étage 1 {nom_etage(1)} (libvirt)", journal)
|
||||
etage1_nom = next(
|
||||
(
|
||||
e.get("nom")
|
||||
for e in (rapport.get("etages") or [])
|
||||
if int(e.get("niveau", 0)) == 1
|
||||
),
|
||||
None,
|
||||
)
|
||||
dire(f" étage 1 {etage1_nom or nom_etage(1)} (libvirt)", journal)
|
||||
if dry_run:
|
||||
dire("\n --dry-run : rien ne sera détruit.", journal)
|
||||
return 0
|
||||
|
|
@ -1036,7 +1100,17 @@ def detruire(journal=None, dry_run=False):
|
|||
# machines » et sortait 1, si bien que le seul avertissement censé
|
||||
# prévenir qu'un disque de plusieurs dizaines de Go reste alloué
|
||||
# s'affichait toujours, et qu'on apprenait à ne plus le lire.
|
||||
racine = detruire_etage1(journal)
|
||||
etage1 = next(
|
||||
(
|
||||
e
|
||||
for e in (rapport.get("etages") or [])
|
||||
if int(e.get("niveau", 0)) == 1
|
||||
),
|
||||
{},
|
||||
)
|
||||
racine = detruire_etage1(
|
||||
journal, attendu=etage1.get("uuid"), nom=etage1.get("nom")
|
||||
)
|
||||
if racine:
|
||||
faits += 1
|
||||
retirer_alias(rapport, journal)
|
||||
|
|
|
|||
|
|
@ -561,6 +561,106 @@ class TestNeJamaisDetruireSousUneDescenteVivante(unittest.TestCase):
|
|||
self.assertIn("descente tourne", sortie.getvalue())
|
||||
|
||||
|
||||
class TestLEtage1SIdentifiePasParSonNom(unittest.TestCase):
|
||||
"""« virsh undefine --remove-all-storage » efface un disque pour de bon.
|
||||
|
||||
Il partait sur le NOM fixe deep-pve-1, quel que soit le domaine qui le
|
||||
porte : la VM d'une descente précédente qu'on voulait garder, ou une
|
||||
machine sans rapport. C'est la famille de défauts la plus tenace de ce
|
||||
travail — une ressource liée à une machine par son nom au lieu de ce qui
|
||||
l'identifie vraiment."""
|
||||
|
||||
def setUp(self):
|
||||
sys.path.insert(0, os.path.join(RACINE, "LongTest"))
|
||||
import deep_proxmox
|
||||
|
||||
self.dp = deep_proxmox
|
||||
self.vrai_run = deep_proxmox.subprocess.run
|
||||
# LE staticmethod, pas la fonction qu'il enveloppe : le rendre nu en
|
||||
# ferait une méthode d'instance, et « self.uuid_libvirt(nom) »
|
||||
# passerait deux arguments à une fonction qui en prend un. La fuite
|
||||
# tombait sur les tests SUIVANTS.
|
||||
self.vrai_uuid = deep_proxmox.Descente.__dict__["uuid_libvirt"]
|
||||
self.addCleanup(setattr, deep_proxmox.subprocess, "run", self.vrai_run)
|
||||
self.addCleanup(
|
||||
setattr, deep_proxmox.Descente, "uuid_libvirt", self.vrai_uuid
|
||||
)
|
||||
self.lances = []
|
||||
|
||||
def _virsh(self, dominfo=0):
|
||||
import types
|
||||
|
||||
def faux(argv, **kw):
|
||||
self.lances.append(" ".join(argv[2:]))
|
||||
code = dominfo if "dominfo" in argv else 0
|
||||
return types.SimpleNamespace(returncode=code, stdout="", stderr="")
|
||||
|
||||
self.dp.subprocess.run = faux
|
||||
|
||||
def test_a_homonym_with_another_uuid_is_left_alone(self):
|
||||
self._virsh()
|
||||
self.dp.Descente.uuid_libvirt = staticmethod(lambda nom: "AUTRE-UUID")
|
||||
with contextlib.redirect_stdout(io.StringIO()) as sortie:
|
||||
res = self.dp.detruire_etage1(None, attendu="LE-NOTRE")
|
||||
self.assertFalse(res)
|
||||
self.assertIn("PAS notre machine", sortie.getvalue())
|
||||
# Aucun undefine, aucun destroy : seule la lecture a eu lieu.
|
||||
self.assertTrue(all("dominfo" in c for c in self.lances), self.lances)
|
||||
|
||||
def test_our_own_machine_is_destroyed(self):
|
||||
self._virsh()
|
||||
self.dp.Descente.uuid_libvirt = staticmethod(lambda nom: "LE-NOTRE")
|
||||
with contextlib.redirect_stdout(io.StringIO()):
|
||||
res = self.dp.detruire_etage1(None, attendu="LE-NOTRE")
|
||||
self.assertTrue(res)
|
||||
self.assertTrue(
|
||||
any(
|
||||
"undefine" in c and "remove-all-storage" in c
|
||||
for c in self.lances
|
||||
),
|
||||
self.lances,
|
||||
)
|
||||
|
||||
def test_an_old_report_without_a_uuid_says_so(self):
|
||||
"""On procède alors par le nom, faute de mieux — mais on le DIT,
|
||||
plutôt que de laisser croire qu'on a vérifié."""
|
||||
self._virsh()
|
||||
with contextlib.redirect_stdout(io.StringIO()) as sortie:
|
||||
res = self.dp.detruire_etage1(None)
|
||||
self.assertTrue(res)
|
||||
self.assertIn("identifié par son NOM", sortie.getvalue())
|
||||
|
||||
def test_an_absent_domain_is_not_an_error(self):
|
||||
self._virsh(dominfo=1)
|
||||
with contextlib.redirect_stdout(io.StringIO()):
|
||||
self.assertTrue(self.dp.detruire_etage1(None, attendu="X"))
|
||||
|
||||
def test_the_name_comes_from_the_report_not_from_the_level(self):
|
||||
"""Le déduire du numéro d'étage supposait que nom_etage ne changera
|
||||
jamais : un rapport ancien nommerait alors d'autres machines."""
|
||||
rapport = {
|
||||
"etages": [
|
||||
{
|
||||
"niveau": 2,
|
||||
"vmid": 102,
|
||||
"parent_alias": "a",
|
||||
"nom": "nom-ecrit-a-la-creation",
|
||||
}
|
||||
]
|
||||
}
|
||||
self.assertEqual(
|
||||
self.dp.a_defaire(rapport),
|
||||
[(2, "a", 102, "nom-ecrit-a-la-creation")],
|
||||
)
|
||||
|
||||
def test_a_report_without_a_name_falls_back_on_the_level(self):
|
||||
rapport = {"etages": [{"niveau": 3, "vmid": 103, "parent_alias": "b"}]}
|
||||
self.assertEqual(
|
||||
self.dp.a_defaire(rapport),
|
||||
[(3, "b", 103, self.dp.nom_etage(3))],
|
||||
)
|
||||
|
||||
|
||||
class TestLaCauseDUnMontageAbsent(unittest.TestCase):
|
||||
"""« pve_unit_cmd » joint le journal de l'unité à un échec — « la seule
|
||||
façon de dire la cause à quelqu'un dont le seul accès à l'hôte est cet
|
||||
|
|
|
|||
Loading…
Reference in a new issue