Skip to content

Commit

Permalink
AutoinstallError: Disable apport reporting
Browse files Browse the repository at this point in the history
Autoinstall related failures are more likely than not going to be
user caused, so we shouldn't immediately generate a crash report
for these types of failures. This should hopefully allow the user
to debug their autoinstall data much faster and reduce the number
of autoinstall-related bugs reported.
  • Loading branch information
Chris-Peterson444 committed Feb 5, 2024
1 parent f1944dd commit 24de248
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 6 deletions.
31 changes: 31 additions & 0 deletions subiquity/server/autoinstall.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Copyright 2024 Canonical, Ltd.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU Affero General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import logging

log = logging.getLogger("subiquity.server.autoinstall")


class AutoinstallError(Exception):
pass


class AutoinstallValidationError(AutoinstallError):
def __init__(
self,
owner: str,
):
self.message = f"Malformed autoinstall in {owner!r} section"
self.owner = owner
super().__init__(self.message)
11 changes: 6 additions & 5 deletions subiquity/server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from subiquity.cloudinit import get_host_combined_cloud_config
from subiquity.common.api.server import bind, controller_for_request
from subiquity.common.apidef import API
from subiquity.common.errorreport import ErrorReporter, ErrorReportKind
from subiquity.common.errorreport import ErrorReport, ErrorReporter, ErrorReportKind
from subiquity.common.serialize import to_json
from subiquity.common.types import (
ApplicationState,
Expand All @@ -41,7 +41,7 @@
PasswordKind,
)
from subiquity.models.subiquity import ModelNames, SubiquityModel
from subiquity.server.autoinstall import AutoinstallValidationError
from subiquity.server.autoinstall import AutoinstallError, AutoinstallValidationError
from subiquity.server.controller import SubiquityController
from subiquity.server.dryrun import DRConfig
from subiquity.server.errors import ErrorController
Expand Down Expand Up @@ -403,8 +403,9 @@ def note_data_for_apport(self, key, value):
def make_apport_report(self, kind, thing, *, wait=False, **kw):
return self.error_reporter.make_apport_report(kind, thing, wait=wait, **kw)

async def _run_error_cmds(self, report):
await report._info_task
async def _run_error_cmds(self, report: Optional[ErrorReport] = None) -> None:
if report is not None and report._info_task is not None:
await report._info_task
Error = getattr(self.controllers, "Error", None)
if Error is not None and Error.cmds:
try:
Expand All @@ -421,7 +422,7 @@ def _exception_handler(self, loop, context):
return
report = self.error_reporter.report_for_exc(exc)
log.error("top level error", exc_info=exc)
if not report:
if not isinstance(exc, AutoinstallError) and not report:
report = self.make_apport_report(
ErrorReportKind.UNKNOWN, "unknown error", exc=exc
)
Expand Down
7 changes: 6 additions & 1 deletion subiquity/server/tests/test_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def test_setup_autoinstall(self, mock_load):
mock_load.assert_called_once_with("default-data")

def test_autoinstall_validation(self):
"""Test validation error type"""
"""Test validation error type and no apport reporting"""

self.controller.autoinstall_schema = {
"type": "object",
Expand All @@ -84,3 +84,8 @@ def test_autoinstall_validation(self):

# Assert error section is based on autoinstall_key
self.assertEquals(exception.owner, "some-key")

# Assert apport report is not created
# This only checks that controllers do not manually create an apport
# report on validation. Should also be tested in Server
self.controller.app.make_apport_report.assert_not_called()
15 changes: 15 additions & 0 deletions subiquity/server/tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ async def asyncSetUp(self):
},
},
}
self.server.make_apport_report = Mock()

def test_valid_schema(self):
"""Test that the expected autoinstall JSON schema is valid"""
Expand All @@ -179,6 +180,20 @@ def test_autoinstall_validation__error_type(self):
with self.assertRaises(AutoinstallValidationError):
self.server.validate_autoinstall()

async def test_autoinstall_validation__no_error_report(self):
"""Test no apport reporting"""

exception = AutoinstallValidationError("Mock")

loop = Mock()
context = {"exception": exception}

with patch("subiquity.server.server.log"):
with patch.object(self.server, "_run_error_cmds"):
self.server._exception_handler(loop, context)

self.server.make_apport_report.assert_not_called()


class TestMetaController(SubiTestCase):
async def test_interactive_sections_not_present(self):
Expand Down
1 change: 1 addition & 0 deletions subiquitycore/tests/mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ def make_app(model=None):
app.log_syslog_id = None
app.report_start_event = mock.Mock()
app.report_finish_event = mock.Mock()
app.make_apport_report = mock.Mock()

return app

0 comments on commit 24de248

Please sign in to comment.