From c855eaab52f629284e7e45b0ae4009af37449c53 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 13 May 2025 20:51:16 +0200 Subject: [PATCH] Delete Backup files on error (#5880) --- supervisor/backups/backup.py | 5 --- supervisor/backups/manager.py | 6 +++- tests/backups/test_manager.py | 65 +++++++++++++++++++++++++++++++++-- 3 files changed, 67 insertions(+), 9 deletions(-) diff --git a/supervisor/backups/backup.py b/supervisor/backups/backup.py index 43842945a..1f98e865c 100644 --- a/supervisor/backups/backup.py +++ b/supervisor/backups/backup.py @@ -244,11 +244,6 @@ class Backup(JobGroup): """Return backup size in bytes.""" return self._locations[self.location].size_bytes - @property - def is_new(self) -> bool: - """Return True if there is new.""" - return not self.tarfile.exists() - @property def tarfile(self) -> Path: """Return path to backup tarfile.""" diff --git a/supervisor/backups/manager.py b/supervisor/backups/manager.py index 99885704f..6e5cf1818 100644 --- a/supervisor/backups/manager.py +++ b/supervisor/backups/manager.py @@ -518,7 +518,8 @@ class BackupManager(FileConfiguration, JobGroup): ) -> Backup | None: """Create a backup. - Must be called from an existing backup job. + Must be called from an existing backup job. If the backup failed, the + backup file is being deleted and None is returned. """ addon_start_tasks: list[Awaitable[None]] | None = None @@ -548,9 +549,12 @@ class BackupManager(FileConfiguration, JobGroup): self._change_stage(BackupJobStage.FINISHING_FILE, backup) except BackupError as err: + await self.sys_run_in_executor(backup.tarfile.unlink) + _LOGGER.error("Backup %s error: %s", backup.slug, err) self.sys_jobs.current.capture_error(err) return None except Exception as err: # pylint: disable=broad-except + await self.sys_run_in_executor(backup.tarfile.unlink) _LOGGER.exception("Backup %s error", backup.slug) await async_capture_exception(err) self.sys_jobs.current.capture_error( diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index 4461511e1..1d33608d8 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -16,7 +16,7 @@ from supervisor.addons.addon import Addon from supervisor.addons.const import AddonBackupMode from supervisor.addons.model import AddonModel from supervisor.backups.backup import Backup, BackupLocation -from supervisor.backups.const import LOCATION_TYPE, BackupType +from supervisor.backups.const import LOCATION_TYPE, BackupJobStage, BackupType from supervisor.backups.manager import BackupManager from supervisor.const import FOLDER_HOMEASSISTANT, FOLDER_SHARE, AddonState, CoreState from supervisor.coresys import CoreSys @@ -36,6 +36,7 @@ from supervisor.homeassistant.api import HomeAssistantAPI from supervisor.homeassistant.const import WSType from supervisor.homeassistant.core import HomeAssistantCore from supervisor.homeassistant.module import HomeAssistant +from supervisor.jobs import JobSchedulerOptions from supervisor.jobs.const import JobCondition from supervisor.mounts.mount import Mount from supervisor.resolution.const import UnhealthyReason @@ -405,7 +406,63 @@ async def test_fail_invalid_partial_backup( await manager.do_restore_partial(backup_instance) -async def test_backup_error( +async def test_backup_error_homeassistant( + coresys: CoreSys, + backup_mock: MagicMock, + install_addon_ssh: Addon, + capture_exception: Mock, +): + """Test error collected and file deleted when Home Assistant Core backup fails.""" + await coresys.core.set_state(CoreState.RUNNING) + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup_instance = backup_mock.return_value + + backup_instance.store_homeassistant.side_effect = ( + err := BackupError("Error while storing homeassistant") + ) + + job, backup_task = coresys.jobs.schedule_job( + coresys.backups.do_backup_full, JobSchedulerOptions() + ) + assert await backup_task is None + + assert job.errors[0].type_ is type(err) + assert job.errors[0].message == str(err) + assert job.errors[0].stage == BackupJobStage.HOME_ASSISTANT + + backup_instance.tarfile.unlink.assert_called_once() + + +async def test_backup_error_folder( + coresys: CoreSys, + backup_mock: MagicMock, + install_addon_ssh: Addon, + capture_exception: Mock, +): + """Test error collected and file deleted when folder backup fails.""" + await coresys.core.set_state(CoreState.RUNNING) + coresys.hardware.disk.get_disk_free_space = lambda x: 5000 + + backup_instance = backup_mock.return_value + + backup_instance.store_folders.side_effect = ( + err := BackupError("Error while storing folders") + ) + + job, backup_task = coresys.jobs.schedule_job( + coresys.backups.do_backup_full, JobSchedulerOptions() + ) + assert await backup_task is None + + assert job.errors[0].type_ is type(err) + assert job.errors[0].message == str(err) + assert job.errors[0].stage == BackupJobStage.FOLDERS + + backup_instance.tarfile.unlink.assert_called_once() + + +async def test_backup_error_capture( coresys: CoreSys, backup_mock: MagicMock, install_addon_ssh: Addon, @@ -416,7 +473,9 @@ async def test_backup_error( coresys.hardware.disk.get_disk_free_space = lambda x: 5000 backup_mock.return_value.store_folders.side_effect = (err := OSError()) - await coresys.backups.do_backup_full() + backup = await coresys.backups.do_backup_full() + + assert backup is None capture_exception.assert_called_once_with(err)