Delete Backup files on error (#5880)

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."""

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(

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)