From d87685b95a2794a636144ceaaed3100ac7ceb12f Mon Sep 17 00:00:00 2001 From: James Valleroy Date: Mon, 20 May 2024 19:44:11 -0400 Subject: [PATCH] diagnostics: Add option for automatic repair - Not enabled by default currently. This can be changed after further testing. - Re-use existing operation from diagnostics run. However, this requires changing the app_id of the operation for each app. Tests: - Enable automatic repair, and run diagnostics. See that repairs are run. - Enable automatic repair, and wait for daily diagnostics run. See that repairs are run. Closes: #2399. Signed-off-by: James Valleroy Reviewed-by: Sunil Mohan Adapa --- plinth/modules/diagnostics/__init__.py | 29 +++++++++++++++++++++-- plinth/modules/diagnostics/forms.py | 4 ++++ plinth/modules/diagnostics/views.py | 11 +++++++++ plinth/privileged/packages.py | 3 ++- plinth/setup.py | 32 +++++++++++++++++++------- 5 files changed, 68 insertions(+), 11 deletions(-) diff --git a/plinth/modules/diagnostics/__init__.py b/plinth/modules/diagnostics/__init__.py index f763b239c..8fd453db3 100644 --- a/plinth/modules/diagnostics/__init__.py +++ b/plinth/modules/diagnostics/__init__.py @@ -21,6 +21,7 @@ from plinth.diagnostic_check import (CheckJSONDecoder, CheckJSONEncoder, DiagnosticCheck, Result) from plinth.modules.apache.components import diagnose_url_on_all from plinth.modules.backups.components import BackupRestore +from plinth.setup import run_repair_on_app from . import manifest @@ -256,7 +257,7 @@ def _daily_diagnostics_run(data: None = None): logger.info('Skipping daily diagnostics run (disabled)') -def start_diagnostics(data: None = None): +def start_diagnostics(): """Start full diagnostics as a background operation.""" logger.info('Running full diagnostics') try: @@ -275,6 +276,7 @@ def _run_diagnostics(): from plinth.notification import Notification _run_on_all_enabled_modules() + apps_with_issues = set() with results_lock: results = current_results['results'] # Store the most recent results in the database. @@ -283,13 +285,14 @@ def _run_diagnostics(): issue_count = 0 severity = 'warning' - for _app_id, app_data in results.items(): + for app_id, app_data in results.items(): if app_data['exception']: issue_count += 1 severity = 'error' else: for check in app_data['diagnosis']: if check.result != Result.PASSED: + apps_with_issues.add(app_id) issue_count += 1 if check.result != Result.WARNING: severity = 'error' @@ -323,6 +326,14 @@ def _run_diagnostics(): data=data, group='admin') note.dismiss(False) + # If enabled, run automatic repair for apps with failed diagnostics. + if is_automatic_repair_enabled(): + logger.info('Starting automatic repair...') + for app_id in apps_with_issues: + run_repair_on_app(app_id, False) + else: + logger.info('Skipping automatic repair, disabled.') + def are_results_available(): """Return whether diagnostic results are available.""" @@ -374,3 +385,17 @@ def is_daily_run_enabled() -> bool: def set_daily_run_enabled(enabled: bool): """Enable or disable daily run.""" kvstore.set('diagnostics_daily_run_enabled', enabled) + + +def is_automatic_repair_enabled() -> bool: + """Return whether automatic repair is enabled. + + In case it is not set, assume it is not enabled. This default could be + changed later. + """ + return kvstore.get_default('diagnostics_automatic_repair_enabled', False) + + +def set_automatic_repair_enabled(enabled: bool): + """Enable or disable automatic repair.""" + kvstore.set('diagnostics_automatic_repair_enabled', enabled) diff --git a/plinth/modules/diagnostics/forms.py b/plinth/modules/diagnostics/forms.py index 21f2ba25a..5f62b5044 100644 --- a/plinth/modules/diagnostics/forms.py +++ b/plinth/modules/diagnostics/forms.py @@ -10,3 +10,7 @@ class ConfigureForm(forms.Form): daily_run_enabled = forms.BooleanField( label=_('Enable daily run'), required=False, help_text=_('When enabled, diagnostic checks will run once a day.')) + + automatic_repair = forms.BooleanField( + label=_('Enable automatic repair'), required=False, + help_text=_('If issues are found, try to repair them automatically.')) diff --git a/plinth/modules/diagnostics/views.py b/plinth/modules/diagnostics/views.py index 8c8648ac2..f3a73f2be 100644 --- a/plinth/modules/diagnostics/views.py +++ b/plinth/modules/diagnostics/views.py @@ -44,14 +44,25 @@ class DiagnosticsView(AppView): """Return the initial values for the form.""" status = super().get_initial() status['daily_run_enabled'] = diagnostics.is_daily_run_enabled() + status['automatic_repair'] = diagnostics.is_automatic_repair_enabled() return status def form_valid(self, form): """Apply the form changes.""" old_status = form.initial new_status = form.cleaned_data + updated = False + if old_status['daily_run_enabled'] != new_status['daily_run_enabled']: diagnostics.set_daily_run_enabled(new_status['daily_run_enabled']) + updated = True + + if old_status['automatic_repair'] != new_status['automatic_repair']: + diagnostics.set_automatic_repair_enabled( + new_status['automatic_repair']) + updated = True + + if updated: messages.success(self.request, _('Configuration updated.')) return super().form_valid(form) diff --git a/plinth/privileged/packages.py b/plinth/privileged/packages.py index 9045d2fad..90d40ccd1 100644 --- a/plinth/privileged/packages.py +++ b/plinth/privileged/packages.py @@ -40,7 +40,8 @@ def install(app_id: str, packages: list[str], skip_recommends: bool = False, try: _assert_managed_packages(app_id, packages) except Exception: - raise PermissionError(f'Packages are not managed: {packages}') + raise PermissionError( + f'Packages are not managed by {app_id}: {packages}') extra_arguments = [] if skip_recommends: diff --git a/plinth/setup.py b/plinth/setup.py index 6c8bbf4c8..af19995fa 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -97,20 +97,36 @@ def _run_setup_on_app(app, current_version, repair: bool = False): operation.on_update(message, exception_to_update) -def run_repair_on_app(app_id): - """Execute the repair process in a thread.""" +def run_repair_on_app(app_id, create_operation=True): + """Execute the repair process in a thread. + + In case this is called from within another operation, creating a new + operation can be skipped. + """ app = app_module.App.get(app_id) current_version = app.get_setup_version() if not current_version: logger.warning('App %s is not installed, cannot repair', app_id) return - logger.debug('Creating operation to repair app: %s', app_id) - return operation_module.manager.new(f'{app_id}-repair', app_id, - gettext_noop('Repairing app'), - _run_repair_on_app, [app], - show_message=True, - show_notification=True) + if create_operation: + logger.debug('Creating operation to repair app: %s', app_id) + return operation_module.manager.new(f'{app_id}-repair', app_id, + gettext_noop('Repairing app'), + _run_repair_on_app, [app], + show_message=True, + show_notification=True) + + # Re-use existing operation. + try: + operation = operation_module.Operation.get_operation() + except AttributeError: + raise RuntimeError( + 'run_repair_on_app: Expected an existing operation.') + + # XXX: Ugly hack to re-use operation from another app. + operation.app_id = app_id + _run_repair_on_app(app) def _run_repair_on_app(app: app_module.App):