From 736ab3b676d69a7b426301a334a0da825d607c99 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Wed, 2 Sep 2026 07:35:28 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20qemu=20setup-host=20:=20demander=20avan?= =?UTF-8?q?t=20de=20red=C3=A9marrer=20l'h=C3=B4te?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accepter d'installer les paquets QEMU redémarrait la machine sans autre question : --assume-yes couvrait le gestionnaire de paquets, et la commande y ajoutait --reboot-if-needed, qui ne demandait rien. Une seule constante servait la VM qu'on vient de créer et le poste qui la crée. --reboot-if-needed PROPOSE désormais, sur /dev/tty pour rester visible quand la sortie est un tuyau, et vaut non par défaut ; un refus laisse les paquets posés et dit quoi faire. --assume-yes-reboot est le seul consentement muet, que seul le profil invité porte. Vérifié : 7 tests, rougis par deux mutations — assume_yes rouvrant la porte, le menu hôte reprenant le drapeau. --- EN --- Accepting the QEMU package install rebooted the machine with no further question: --assume-yes covered the package manager, and the command added --reboot-if-needed, which asked nothing. One constant served both the VM just created and the workstation creating it. --reboot-if-needed now OFFERS, on /dev/tty so it stays visible when output is a pipe, and defaults to no; a refusal leaves the packages in place and says what to do. --assume-yes-reboot is the only silent consent, carried by the guest profile alone. Checked: 7 tests, turned red by two mutations — assume_yes reopening the door, the host menu taking the flag back. Assisted-by: Claude Opus 5 --- script/qemu/deploy_qemu.py | 79 +++++++---- script/todo/qemu_install.py | 6 +- script/todo/qemu_menu.py | 9 +- test/test_qemu_setup_host_reboot.py | 196 ++++++++++++++++++++++++++++ 4 files changed, 263 insertions(+), 27 deletions(-) create mode 100644 test/test_qemu_setup_host_reboot.py diff --git a/script/qemu/deploy_qemu.py b/script/qemu/deploy_qemu.py index b874180..8955823 100755 --- a/script/qemu/deploy_qemu.py +++ b/script/qemu/deploy_qemu.py @@ -563,9 +563,7 @@ def default_image_name(distro: str, code: str, arch: str, version: str) -> str: # de cache fait qu'un déploiement Debian 13 et un Proxmox se # PARTAGENT le téléchargement (325 Mio) au lieu d'en faire deux. Sans # cette branche, le repli de fin nommait l'image « fedora-cloud-9 ». - return ( - f"debian-{PROXMOX_DEBIAN_BASE[version]}-genericcloud-{a}.qcow2" - ) + return f"debian-{PROXMOX_DEBIAN_BASE[version]}-genericcloud-{a}.qcow2" if distro == "arch": return f"arch-linux-{a}-cloudimg.qcow2" if distro == "opensuse": @@ -1072,6 +1070,7 @@ def setup_host( assume_yes: bool, no_install: bool, reboot_if_needed: bool = False, + assume_yes_reboot: bool = False, ) -> None: """Prépare l'hôte à faire tourner des VM : paquets, démon, groupe, réseau. @@ -1114,14 +1113,25 @@ def setup_host( # monte tout seul avec les modules du nouveau noyau. Un seul reboot # suffit donc à rendre l'hôte utilisable, sans repasser par ici. if reboot_if_needed: - print( - "\n↻ Redémarrage programmé (dans quelques secondes) : c'est la" - " SEULE façon de retrouver les modules du noyau.\n" - " Au retour, le réseau « default » démarrera seul" - " (autostart déjà actif)." + # Le consentement à installer des paquets ne vaut PAS consentement + # à redémarrer : assume_yes couvre pacman, jamais la machine de + # celui qui l'a tapé. Une provision sans personne devant l'écran + # passe --assume-yes-reboot, qui dit explicitement l'autre chose. + if assume_yes_reboot or prompt_yes_no( + "\n↻ Redémarrer MAINTENANT ? C'est la seule façon de" + " retrouver les modules du noyau. Au retour, le réseau" + " « default » démarrera seul (autostart déjà actif).", + default=False, + ): + print("\n↻ Redémarrage programmé (dans quelques secondes).") + schedule_reboot(runner) + return + sys.exit( + "Erreur : l'hôte n'est pas prêt, redémarrage refusé.\n" + f" {stale}\n" + " Redémarrez quand vous le voudrez, puis relancez" + " --setup-host." ) - schedule_reboot(runner) - return sys.exit(f"Erreur : l'hôte n'est pas prêt.\n {stale}") if not (ok and active): @@ -2468,18 +2478,23 @@ def _ip_taken(ip: str) -> bool: except (OSError, subprocess.SubprocessError): pass try: - if subprocess.run( - ["ping", "-c", "1", "-W", "1", ip], - capture_output=True, - timeout=5, - ).returncode == 0: + if ( + subprocess.run( + ["ping", "-c", "1", "-W", "1", ip], + capture_output=True, + timeout=5, + ).returncode + == 0 + ): return True except (OSError, subprocess.SubprocessError): pass return _ip_reachable(ip, port=22, timeout=1.5) -def static_net_plan(net: str | None, use_sudo: bool, name: str) -> dict[str, str] | None: +def static_net_plan( + net: str | None, use_sudo: bool, name: str +) -> dict[str, str] | None: """Adresse fixe libre pour une VM installée par debian-installer. L'initrd s390x ne contient QUE « netcfg-static » : le journal de d-i @@ -3019,9 +3034,11 @@ def virt_install( # VM que personne ne regarde, et il ne reste RIEN à lire ensuite — # exactement « l'installation a échoué, pas de sortie pertinente ». # Le fichier, lui, survit à l'arrêt du domaine. - f"pty,target_type={console_target},log.file={console_log}" - if installer - else f"pty,target_type={console_target}", + ( + f"pty,target_type={console_target},log.file={console_log}" + if installer + else f"pty,target_type={console_target}" + ), # Canal virtio de l'agent invité (org.qemu.guest_agent.0) : permet à # virsh de piloter la VM SANS réseau (ex. étendre le FS invité après # un redimensionnement de disque). Inoffensif si l'agent est absent. @@ -3495,8 +3512,15 @@ def build_parser() -> argparse.ArgumentParser: g_run.add_argument( "--reboot-if-needed", action="store_true", - help="Avec --setup-host : redémarre si le noyau a été mis à jour " - "depuis le démarrage (sinon libvirt ne peut pas créer virbr0).", + help="Avec --setup-host : PROPOSE un redémarrage si le noyau a été " + "mis à jour depuis le démarrage (sinon libvirt ne peut pas créer " + "virbr0). La question est posée sur /dev/tty et vaut non par défaut.", + ) + g_run.add_argument( + "--assume-yes-reboot", + action="store_true", + help="Redémarre sans poser la question. Réservé à une provision " + "sans personne devant l'écran ; --assume-yes ne l'implique pas.", ) g_run.add_argument( "--list-images", @@ -3555,6 +3579,7 @@ def main() -> None: args.assume_yes, args.no_install_deps, args.reboot_if_needed, + args.assume_yes_reboot, ) return @@ -3707,11 +3732,15 @@ def main() -> None: network_name(args.network), not args.dry_run, args.name ) if static: - print(f" Adresse fixe retenue : {static['ip']}" - f" (passerelle {static['gateway']})") + print( + f" Adresse fixe retenue : {static['ip']}" + f" (passerelle {static['gateway']})" + ) else: - print(" ⚠ Aucune adresse fixe déterminée : netcfg-static posera" - " la question à l'écran et l'installation s'arrêtera.") + print( + " ⚠ Aucune adresse fixe déterminée : netcfg-static posera" + " la question à l'écran et l'installation s'arrêtera." + ) build_installer_initrd( build_preseed(args, pw_hash, ssh_keys, static), initrd_src, diff --git a/script/todo/qemu_install.py b/script/todo/qemu_install.py index d65b00b..daafb07 100644 --- a/script/todo/qemu_install.py +++ b/script/todo/qemu_install.py @@ -19,9 +19,13 @@ class QemuInstallMixin: # Sans le groupe, virt-install retombe sur qemu:///session où « default » # n'existe pas : la VM échoue alors que tous les paquets sont installés. # L'ancien one-liner finissait par « || true » et masquait ses erreurs. + # Le redémarrage est consenti ICI et nulle part ailleurs : la VM vient + # d'être créée, personne ne la regarde, et le noyau fraîchement installé + # doit être chargé avant que libvirt puisse monter virbr0. Sur un poste de + # travail, la question se pose — voir _qemu_ensure_tools. _QEMU_QEMU_PKGS = ( "./script/qemu/deploy_qemu.py --setup-host --assume-yes" - " --reboot-if-needed" + " --reboot-if-needed --assume-yes-reboot" ) def _qemu_ask_prod(self): diff --git a/script/todo/qemu_menu.py b/script/todo/qemu_menu.py index 28db42f..a09e04f 100644 --- a/script/todo/qemu_menu.py +++ b/script/todo/qemu_menu.py @@ -253,7 +253,14 @@ class QemuMenuMixin: input(t("Install the QEMU/libvirt tools now? (Y/n): ")) ): return False - cmd = f"sudo {self._QEMU_QEMU_PKGS}" + # Sans --assume-yes-reboot : accepter d'installer des paquets n'est + # pas accepter de perdre ce qui tourne sur la machine. Quand le noyau + # a été remplacé depuis le démarrage, deploy_qemu.py pose la question + # sur /dev/tty, et un refus laisse l'hôte avec ses paquets posés. + cmd = ( + "sudo ./script/qemu/deploy_qemu.py --setup-host --assume-yes" + " --reboot-if-needed" + ) print(f"{t('Will execute:')} {cmd}") self.execute.exec_command_live(cmd, source_erplibre=False) if shutil.which("virsh"): diff --git a/test/test_qemu_setup_host_reboot.py b/test/test_qemu_setup_host_reboot.py new file mode 100644 index 0000000..f3727c0 --- /dev/null +++ b/test/test_qemu_setup_host_reboot.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +# © 2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) +"""Le redémarrage de l'hôte se demande, il ne se déduit pas. + +« --setup-host » installe des paquets, et sur une distribution à noyau +roulant il peut avoir besoin d'un redémarrage pour que libvirt monte virbr0. +Ces deux actes n'ont pas le même prix : le premier s'annule, le second emporte +tout ce qui tourne sur la machine. + +Ce que ces tests gardent : + +- « --assume-yes » couvre le gestionnaire de paquets, JAMAIS le redémarrage. +- « --reboot-if-needed » PROPOSE ; un refus laisse la machine debout et sort + en erreur, sans jamais programmer le redémarrage. +- « --assume-yes-reboot » est le seul consentement qui se passe de question, + et il est réservé à la provision d'une VM que personne ne regarde. +- La commande du menu hôte ne le porte pas ; celle du profil invité le porte. +""" + +import importlib.util +import io +import sys +import unittest +from contextlib import redirect_stdout +from pathlib import Path +from unittest import mock + +sys.argv = ["todo.py"] + +RACINE = Path(__file__).resolve().parents[1] + + +def _deploy_qemu(): + """deploy_qemu.py chargé comme module, comme le fait todo.py.""" + path = RACINE / "script/qemu/deploy_qemu.py" + spec = importlib.util.spec_from_file_location("deploy_qemu", path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +DQ = _deploy_qemu() + +# L'état exact qui déclenche la question : le noyau a été remplacé depuis le +# démarrage, donc le réseau « default » ne peut pas monter. +NOYAU_PERIME = "Le noyau en cours n'a plus ses modules." + + +class SetupHostReboot(unittest.TestCase): + def _lancer(self, reponse=False, **kwargs): + """setup_host sur un hôte au noyau périmé et au réseau inactif. + + Rend le triplet (redémarrages, questions, SystemExit ou None). Tout + ce qui touche au système est neutralisé : seul l'enchaînement des + décisions est sous test. « reponse » est ce que l'utilisateur répond + si la question lui est posée. + """ + runner = mock.MagicMock() + runner.dry_run = False + runner.use_sudo = False + reboots = [] + questions = [] + + def question(texte, default=True): + questions.append(texte) + return reponse + + with mock.patch.object(DQ, "ensure_tools"), mock.patch.object( + DQ, "ensure_libvirt_service" + ), mock.patch.object( + DQ, "ensure_libvirt_group", return_value=True + ), mock.patch.object( + DQ, "ensure_ssh_key" + ), mock.patch.object( + DQ, "ensure_network" + ), mock.patch.object( + DQ, "kernel_modules_stale", return_value=NOYAU_PERIME + ), mock.patch.object( + DQ, "libvirt_ready", return_value=True + ), mock.patch.object( + DQ, "network_state", return_value=(False, True) + ), mock.patch.object( + DQ, "schedule_reboot", side_effect=lambda r: reboots.append(r) + ), mock.patch.object( + DQ, "prompt_yes_no", side_effect=question + ): + with redirect_stdout(io.StringIO()): + try: + DQ.setup_host(runner, **kwargs) + sortie = None + except SystemExit as exc: + sortie = exc + return reboots, questions, sortie + + def test_assume_yes_alone_never_reboots(self): + """La régression même : accepter l'installation des paquets ne + redémarrait pas la machine, mais l'appelant, lui, ajoutait le drapeau + qui le faisait. Sans le drapeau, rien ne redémarre et rien n'est + demandé.""" + reboots, questions, sortie = self._lancer( + assume_yes=True, no_install=False, reboot_if_needed=False + ) + self.assertEqual(reboots, []) + self.assertEqual(questions, []) + self.assertIsInstance(sortie, SystemExit) + + def test_reboot_if_needed_asks_and_a_refusal_stops(self): + reboots, questions, sortie = self._lancer( + assume_yes=True, + no_install=False, + reboot_if_needed=True, + reponse=False, + ) + self.assertEqual(len(questions), 1, questions) + self.assertEqual(reboots, []) + self.assertIsInstance(sortie, SystemExit) + # Le refus doit se lire dans le message : un « pas prêt » sec laisse + # croire à une panne alors que la machine a obéi. + self.assertIn("refusé", str(sortie)) + + def test_reboot_if_needed_reboots_when_accepted(self): + reboots, questions, sortie = self._lancer( + assume_yes=True, + no_install=False, + reboot_if_needed=True, + reponse=True, + ) + self.assertEqual(len(questions), 1) + self.assertEqual(len(reboots), 1) + self.assertIsNone(sortie) + + def test_assume_yes_reboot_skips_the_question(self): + """La provision d'une VM neuve n'a personne pour répondre : sans ce + drapeau, la question tomberait sur un EOF et la VM resterait sur un + noyau sans modules.""" + reboots, questions, sortie = self._lancer( + assume_yes=True, + no_install=False, + reboot_if_needed=True, + assume_yes_reboot=True, + ) + self.assertEqual(questions, []) + self.assertEqual(len(reboots), 1) + self.assertIsNone(sortie) + + +class ConsentInTheCallers(unittest.TestCase): + """Le drapeau se lit dans les commandes que TODO fabrique.""" + + @staticmethod + def _commande_hote(): + """La chaîne assignée à « cmd » dans _qemu_ensure_tools. + + Lue par l'arbre syntaxique et non par le texte : un commentaire qui + NOMME le drapeau pour expliquer son absence est légitime, et une + recherche textuelle le prendrait pour la commande. + """ + import ast + + source = (RACINE / "script/todo/qemu_menu.py").read_text( + encoding="utf-8" + ) + for node in ast.walk(ast.parse(source)): + if ( + isinstance(node, ast.FunctionDef) + and node.name == "_qemu_ensure_tools" + ): + for stmt in ast.walk(node): + if ( + isinstance(stmt, ast.Assign) + and getattr(stmt.targets[0], "id", "") == "cmd" + ): + return ast.literal_eval(stmt.value) + raise AssertionError("cmd introuvable dans _qemu_ensure_tools") + + def test_the_host_menu_never_assumes_the_reboot(self): + cmd = self._commande_hote() + self.assertIn("--setup-host", cmd) + self.assertIn("--reboot-if-needed", cmd) + self.assertNotIn("--assume-yes-reboot", cmd) + + def test_the_guest_profile_carries_the_explicit_consent(self): + source = (RACINE / "script/todo/qemu_install.py").read_text( + encoding="utf-8" + ) + self.assertIn("--assume-yes-reboot", source) + + def test_the_flag_exists_in_the_parser(self): + parser = DQ.build_parser() + rendu = parser.format_help() + self.assertIn("--assume-yes-reboot", rendu) + + +if __name__ == "__main__": + unittest.main()