From 300f90f2a2722dec7712bfaa1fd537e1f7fc65a8 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Mon, 30 Oct 2023 16:54:52 -0700 Subject: [PATCH] backups: Don't leave services stopped if backup fails - We stop services before backup and restart them when backup is completed. However, if backup fails, we are not restarting the services. With this change, ensure that stopped services are restarted even if backup process fails. - Similarly for restore operation. Tests: - Backup and restore of an app work. - Functional tests for matrix-synapse work. - Run the following two tests without the patch to ensure that the reported bug is reproducible. - Make a backup operation fail by raising an exception in the privileged code that takes backup. Enable matrix-synapse app. Run backup including the matrix-synapse app. Backup fails and shows an error. The service is stopped before backup and restarted after backup failure. - Make a restore operation fail by raising an exception in the privileged code that does restore. Enable matrix-synapse app. Run backup including the matrix-synapse app and try to restore it. Restore fails and shows an error. The service is stopped before restore and restarted after restore failure. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/backups/api.py | 41 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/plinth/modules/backups/api.py b/plinth/modules/backups/api.py index 6763b396e..2213b3d4e 100644 --- a/plinth/modules/backups/api.py +++ b/plinth/modules/backups/api.py @@ -126,16 +126,17 @@ def backup_apps(backup_handler, path, app_ids=None, encryption_passphrase=None, backup_root = '/' snapshotted = False - packet = Packet('backup', 'apps', backup_root, components, path, - archive_comment) - _run_operation(backup_handler, packet, - encryption_passphrase=encryption_passphrase) - - if snapshotted: - _delete_snapshot(snapshot) - else: - _restore_services(original_state) - _lockdown_apps(components, lockdown=False) + try: + packet = Packet('backup', 'apps', backup_root, components, path, + archive_comment) + _run_operation(backup_handler, packet, + encryption_passphrase=encryption_passphrase) + finally: + if snapshotted: + _delete_snapshot(snapshot) + else: + _restore_services(original_state) + _lockdown_apps(components, lockdown=False) def restore_apps(restore_handler, app_ids=None, create_subvolume=True, @@ -157,15 +158,17 @@ def restore_apps(restore_handler, app_ids=None, create_subvolume=True, restore_root = '/' subvolume = False - packet = Packet('restore', 'apps', restore_root, components, backup_file) - _run_operation(restore_handler, packet, - encryption_passphrase=encryption_passphrase) - - if subvolume: - _switch_to_subvolume(subvolume) - else: - _restore_services(original_state) - _lockdown_apps(components, lockdown=False) + try: + packet = Packet('restore', 'apps', restore_root, components, + backup_file) + _run_operation(restore_handler, packet, + encryption_passphrase=encryption_passphrase) + finally: + if subvolume: + _switch_to_subvolume(subvolume) + else: + _restore_services(original_state) + _lockdown_apps(components, lockdown=False) def _install_apps_before_restore(components):