From 32e7dc54c5e942d3d2e5e5c182ea83ea2fe0a296 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Fri, 1 Mar 2024 15:12:03 -0800 Subject: [PATCH] API: Add non-reportable errors to /meta/status API response Adds a field to the ApplicationStatus struct, nonreportable_error, to be filled when the server enters an error state due to a non-reportable error/exception type. --- subiquity/common/types.py | 17 ++++++ subiquity/server/server.py | 5 +- subiquity/server/tests/test_server.py | 10 +++- subiquity/tests/api/test_api.py | 55 ++++++++++++++++++-- test_data/autoinstall/bad-early-command.yaml | 12 +++++ test_data/autoinstall/invalid-early.yaml | 6 +++ 6 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 test_data/autoinstall/bad-early-command.yaml create mode 100644 test_data/autoinstall/invalid-early.yaml diff --git a/subiquity/common/types.py b/subiquity/common/types.py index b7e3bf0c..4c5ae938 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -24,6 +24,7 @@ from typing import Any, Dict, List, Optional, Union import attr +from subiquity.server.nonreportable import NonReportableException from subiquitycore.models.network import NetDevInfo @@ -55,6 +56,21 @@ class ErrorReportRef: oops_id: Optional[str] +@attr.s(auto_attribs=True) +class NonReportableError: + cause: str + message: str + details: Optional[str] + + @classmethod + def from_exception(cls, exc: NonReportableException): + return cls( + cause=type(exc).__name__, + message=str(exc), + details=exc.details, + ) + + class ApplicationState(enum.Enum): """Represents the state of the application at a given time.""" @@ -87,6 +103,7 @@ class ApplicationStatus: state: ApplicationState confirming_tty: str error: Optional[ErrorReportRef] + nonreportable_error: Optional[NonReportableError] cloud_init_ok: Optional[bool] interactive: Optional[bool] echo_syslog_id: str diff --git a/subiquity/server/server.py b/subiquity/server/server.py index f76a6b01..1cc75c70 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -38,6 +38,7 @@ from subiquity.common.types import ( ErrorReportRef, KeyFingerprint, LiveSessionSSHInfo, + NonReportableError, PasswordKind, ) from subiquity.models.subiquity import ModelNames, SubiquityModel @@ -84,6 +85,7 @@ class MetaController: state=self.app.state, confirming_tty=self.app.confirming_tty, error=self.app.fatal_error, + nonreportable_error=self.app.nonreportable_error, cloud_init_ok=self.app.cloud_init_ok, interactive=self.app.interactive, echo_syslog_id=self.app.echo_syslog_id, @@ -292,6 +294,7 @@ class SubiquityServer(Application): self.interactive = None self.confirming_tty = "" self.fatal_error: Optional[ErrorReport] = None + self.nonreportable_error: Optional[NonReportableError] = None self.running_error_commands = False self.installer_user_name = None self.installer_user_passwd_kind = PasswordKind.NONE @@ -431,7 +434,7 @@ class SubiquityServer(Application): report: Optional[ErrorReport] = None if isinstance(exc, NonReportableException): - pass + self.nonreportable_error = NonReportableError.from_exception(exc) else: report = self.error_reporter.report_for_exc(exc) if report is None: diff --git a/subiquity/server/tests/test_server.py b/subiquity/server/tests/test_server.py index 3eecd091..65252a5f 100644 --- a/subiquity/server/tests/test_server.py +++ b/subiquity/server/tests/test_server.py @@ -20,7 +20,7 @@ from unittest.mock import AsyncMock, Mock, patch import jsonschema from jsonschema.validators import validator_for -from subiquity.common.types import PasswordKind +from subiquity.common.types import NonReportableError, PasswordKind from subiquity.server.autoinstall import AutoinstallValidationError from subiquity.server.nonreportable import NonReportableException from subiquity.server.server import ( @@ -194,6 +194,9 @@ class TestAutoinstallValidation(SubiTestCase): self.server._exception_handler(loop, context) self.server.make_apport_report.assert_not_called() + self.assertIsNone(self.server.fatal_error) + error = NonReportableError.from_exception(exception) + self.assertEqual(error, self.server.nonreportable_error) class TestMetaController(SubiTestCase): @@ -264,6 +267,8 @@ class TestExceptionHandling(SubiTestCase): self.server.make_apport_report.assert_not_called() self.assertEqual(self.server.fatal_error, None) + error = NonReportableError.from_exception(exception) + self.assertEqual(error, self.server.nonreportable_error) async def test_not_suppressed_apport_reporting(self): """Test apport reporting not suppressed""" @@ -275,4 +280,5 @@ class TestExceptionHandling(SubiTestCase): self.server._exception_handler(loop, context) self.server.make_apport_report.assert_called() - self.assertNotEqual(self.server.fatal_error, None) + self.assertIsNotNone(self.server.fatal_error) + self.assertIsNone(self.server.nonreportable_error) diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index 8bce77fb..2d9f33c1 100644 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -118,7 +118,7 @@ class Client: return (self.loads(content), resp) return self.loads(content) - async def poll_startup(self): + async def poll_startup(self, allow_error: bool = False): for _ in range(default_timeout * 10): try: resp = await self.get("/meta/status") @@ -129,7 +129,7 @@ class Client: ): await asyncio.sleep(0.5) continue - if resp["state"] == "ERROR": + if resp["state"] == "ERROR" and not allow_error: raise Exception("server in error state") return except aiohttp.client_exceptions.ClientConnectorError: @@ -270,7 +270,7 @@ def tempdirs(*args, **kwargs): @contextlib.asynccontextmanager -async def start_server_factory(factory, *args, **kwargs): +async def start_server_factory(factory, *args, allow_error: bool = False, **kwargs): with tempfile.TemporaryDirectory() as tempdir: socket_path = f"{tempdir}/socket" conn = aiohttp.UnixConnector(path=socket_path) @@ -279,7 +279,7 @@ async def start_server_factory(factory, *args, **kwargs): try: await server.spawn(tempdir, socket_path, *args, **kwargs) await poll_for_socket_exist(socket_path) - await server.poll_startup() + await server.poll_startup(allow_error=allow_error) yield server finally: await server.close() @@ -2032,6 +2032,53 @@ class TestAutoinstallServer(TestAPI): ) self.assertTrue(expected.issubset(resp)) + async def test_autoinstall_validation_error(self): + cfg = "examples/machines/simple.json" + extra = [ + "--autoinstall", + "test_data/autoinstall/invalid-early.yaml", + ] + # bare server factory for early fail + async with start_server_factory( + Server, cfg, extra_args=extra, allow_error=True + ) as inst: + resp = await inst.get("/meta/status") + + error = resp["nonreportable_error"] + self.assertIsNone(resp["error"]) + + self.assertIsNotNone(error) + self.assertIn("cause", error) + self.assertIn("message", error) + self.assertIn("details", error) + self.assertEqual(error["cause"], "AutoinstallValidationError") + + # This test isn't perfect, because in the future we should + # really throw an AutoinstallError when a user provided + # command fails, but this is the simplest way to test + # the non-reportable errors are still reported correctly. + # This has the added bonus of failing in the future when + # we want to implement this behavior in the command + # controllers + async def test_autoinstall_not_autoinstall_error(self): + cfg = "examples/machines/simple.json" + extra = [ + "--autoinstall", + "test_data/autoinstall/bad-early-command.yaml", + ] + # bare server factory for early fail + async with start_server_factory( + Server, cfg, extra_args=extra, allow_error=True + ) as inst: + resp = await inst.get("/meta/status") + + error = resp["error"] + self.assertIsNone(resp["nonreportable_error"]) + + self.assertIsNotNone(error) + self.assertNotEqual(error, None) + self.assertEqual(error["kind"], "UNKNOWN") + class TestWSLSetupOptions(TestAPI): @timeout() diff --git a/test_data/autoinstall/bad-early-command.yaml b/test_data/autoinstall/bad-early-command.yaml new file mode 100644 index 00000000..14ed6214 --- /dev/null +++ b/test_data/autoinstall/bad-early-command.yaml @@ -0,0 +1,12 @@ +version: 1 + +identity: + realname: '' + hostname: ubuntu + username: ubuntu + password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1' + + +early-commands: + - $((1/0)) + diff --git a/test_data/autoinstall/invalid-early.yaml b/test_data/autoinstall/invalid-early.yaml new file mode 100644 index 00000000..3a24d9df --- /dev/null +++ b/test_data/autoinstall/invalid-early.yaml @@ -0,0 +1,6 @@ +version: 0 +identity: + realname: '' + username: ubuntu + password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1' + hostname: ubuntu