Merge pull request #1888 from ogayot/apport-title-show-curtin-step

apport: show name of curtin stage when an error occurs

subiquity/server/controllers/install.py

@@ -55,6 +55,12 @@ from subiquitycore.utils import arun_command, log_process_streams
 log = logging.getLogger("subiquity.server.controllers.install")
 
 
+class CurtinInstallError(Exception):
+    def __init__(self, *, stages: List[str]) -> None:
+        super().__init__()
+        self.stages = stages
+
+
 class TracebackExtractor:
     start_marker = re.compile(r"^Traceback \(most recent call last\):")
     end_marker = re.compile(r"\S")
@@ -195,6 +201,17 @@ class InstallController(SubiquityController):
         keyboard = self.app.controllers.Keyboard
         await keyboard.setup_target(context=context)
 
+    @staticmethod
+    def error_in_curtin_invocation(exc: Exception) -> Optional[str]:
+        """If the exception passed as an argument corresponds to an error
+        during the invocation of a single curtin stage, return the name of the
+        stage. Otherwise, return None."""
+        if not isinstance(exc, CurtinInstallError):
+            return None
+        if len(exc.stages) != 1:
+            return None
+        return exc.stages[0]
+
     @with_context(description="executing curtin install {name} step")
     async def run_curtin_step(
         self,
@@ -226,16 +243,19 @@ class InstallController(SubiquityController):
         else:
             source_args = ()
 
-        await run_curtin_command(
-            self.app,
-            context,
-            "install",
-            "--set",
-            f"json:stages={json.dumps(stages)}",
-            *source_args,
-            config=str(config_file),
-            private_mounts=False,
-        )
+        try:
+            await run_curtin_command(
+                self.app,
+                context,
+                "install",
+                "--set",
+                f"json:stages={json.dumps(stages)}",
+                *source_args,
+                config=str(config_file),
+                private_mounts=False,
+            )
+        except subprocess.CalledProcessError:
+            raise CurtinInstallError(stages=stages)
 
         device_map_path = config.get("storage", {}).get("device_map_path")
         if device_map_path is not None:
@@ -673,13 +693,13 @@ class InstallController(SubiquityController):
             await self.postinstall(context=context)
 
             self.app.update_state(ApplicationState.DONE)
-        except Exception:
+        except Exception as exc:
             kw = {}
             if self.tb_extractor.traceback:
                 kw["Traceback"] = "\n".join(self.tb_extractor.traceback)
-            self.app.make_apport_report(
-                ErrorReportKind.INSTALL_FAIL, "install failed", **kw
-            )
+            text = self.error_in_curtin_invocation(exc) or "install failed"
+
+            self.app.make_apport_report(ErrorReportKind.INSTALL_FAIL, text, **kw)
             raise
 
     async def platform_postinstall(self):

subiquity/server/controllers/tests/test_install.py

@@ -25,7 +25,7 @@ from curtin.util import EFIBootEntry, EFIBootState
 
 from subiquity.common.types import PackageInstallState
 from subiquity.models.tests.test_filesystem import make_model_and_partition
-from subiquity.server.controllers.install import InstallController
+from subiquity.server.controllers.install import CurtinInstallError, InstallController
 from subiquitycore.tests.mocks import make_app
 
 
@@ -88,6 +88,31 @@ class TestWriteConfig(unittest.IsolatedAsyncioTestCase):
             private_mounts=False,
         )
 
+    @patch("subiquity.server.controllers.install.open", mock_open())
+    async def test_run_curtin_install_step_failed(self):
+        cmd = ["curtin", "install", "--set", 'json:stages=["partitioning"]']
+        stages = ["partitioning"]
+
+        async def fake_run_curtin_command(*args, **kwargs):
+            raise subprocess.CalledProcessError(returncode=1, cmd=cmd)
+
+        with patch(
+            "subiquity.server.controllers.install.run_curtin_command",
+            fake_run_curtin_command,
+        ):
+            with self.assertRaises(CurtinInstallError) as exc_cm:
+                await self.controller.run_curtin_step(
+                    name="MyStep",
+                    stages=stages,
+                    config_file=Path("/config.yaml"),
+                    source=None,
+                    config=self.controller.base_config(
+                        logs_dir=Path("/"), resume_data_file=Path("resume-data")
+                    ),
+                )
+        self.assertEqual(stages, exc_cm.exception.stages)
+        self.assertEqual(cmd, exc_cm.exception.__context__.cmd)
+
     def test_base_config(self):
         config = self.controller.base_config(
             logs_dir=Path("/logs"), resume_data_file=Path("resume-data")
@@ -397,3 +422,19 @@ class TestInstallController(unittest.IsolatedAsyncioTestCase):
                 "https://canonical-subiquity.readthedocs-hosted.com/en/latest/reference/autoinstall-reference.html",  # noqa: E501
                 data,
             )
+
+    def test_error_in_curtin_invocation(self):
+        method = self.controller.error_in_curtin_invocation
+
+        self.assertIsNone(method(Exception()))
+        self.assertIsNone(method(RuntimeError()))
+
+        self.assertIsNone(method(CurtinInstallError(stages=[])))
+        # Running multiple stages in one "curtin step" is not something that
+        # currently happens in practice.
+        self.assertIsNone(
+            method(CurtinInstallError(stages=["extract", "partitioning"]))
+        )
+
+        self.assertEqual("extract", method(CurtinInstallError(stages=["extract"])))
+        self.assertEqual("curthooks", method(CurtinInstallError(stages=["curthooks"])))