Prefer to create backups in local storage if selected (#145331)

homeassistant/components/hassio/backup.py

@@ -297,10 +297,17 @@ class SupervisorBackupReaderWriter(BackupReaderWriter):
         # It's inefficient to let core do all the copying so we want to let
         # supervisor handle as much as possible.
         # Therefore, we split the locations into two lists: encrypted and decrypted.
-        # The longest list will be sent to supervisor, and the remaining locations
+        # The backup will be created in the first location in the list sent to
-        # will be handled by async_upload_backup.
+        # supervisor, and if that location is not available, the backup will
-        # If the lists are the same length, it does not matter which one we send,
+        # fail.
-        # we send the encrypted list to have a well defined behavior.
+        # To make it less likely that the backup fails, we prefer to create the
+        # backup in the local storage location if included in the list of
+        # locations.
+        # Hence, we send the list of locations to supervisor in this priority order:
+        # 1. The list which has local storage
+        # 2. The longest list of locations
+        # 3. The list of encrypted locations
+        # In any case the remaining locations will be handled by async_upload_backup.
         encrypted_locations: list[str] = []
         decrypted_locations: list[str] = []
         agents_settings = manager.config.data.agents
@@ -315,16 +322,26 @@ class SupervisorBackupReaderWriter(BackupReaderWriter):
                     encrypted_locations.append(hassio_agent.location)
             else:
                 decrypted_locations.append(hassio_agent.location)
+        locations = []
+        if LOCATION_LOCAL_STORAGE in decrypted_locations:
+            locations = decrypted_locations
+            password = None
+            # Move local storage to the front of the list
+            decrypted_locations.remove(LOCATION_LOCAL_STORAGE)
+            decrypted_locations.insert(0, LOCATION_LOCAL_STORAGE)
+        elif LOCATION_LOCAL_STORAGE in encrypted_locations:
+            locations = encrypted_locations
+            # Move local storage to the front of the list
+            encrypted_locations.remove(LOCATION_LOCAL_STORAGE)
+            encrypted_locations.insert(0, LOCATION_LOCAL_STORAGE)
         _LOGGER.debug("Encrypted locations: %s", encrypted_locations)
         _LOGGER.debug("Decrypted locations: %s", decrypted_locations)
-        if hassio_agents:
+        if not locations and hassio_agents:
             if len(encrypted_locations) >= len(decrypted_locations):
                 locations = encrypted_locations
             else:
                 locations = decrypted_locations
                 password = None
-        else:
-            locations = []
         locations = locations or [LOCATION_CLOUD_BACKUP]
 
         date = dt_util.now().isoformat()

tests/components/hassio/test_backup.py

@@ -1300,6 +1300,16 @@ async def test_reader_writer_create_job_done(
             False,
             [],
         ),
+        # LOCATION_LOCAL_STORAGE should be moved to the front of the list
+        (
+            [],
+            None,
+            ["hassio.share1", "hassio.local", "hassio.share2", "hassio.share3"],
+            None,
+            [LOCATION_LOCAL_STORAGE, "share1", "share2", "share3"],
+            False,
+            [],
+        ),
         (
             [],
             "hunter2",
@@ -1309,54 +1319,86 @@ async def test_reader_writer_create_job_done(
             True,
             [],
         ),
+        # LOCATION_LOCAL_STORAGE should be moved to the front of the list
         (
-            [
+            [],
-                {
-                    "type": "backup/config/update",
-                    "agents": {
-                        "hassio.local": {"protected": False},
-                    },
-                }
-            ],
             "hunter2",
-            ["hassio.local", "hassio.share1", "hassio.share2", "hassio.share3"],
+            ["hassio.share1", "hassio.local", "hassio.share2", "hassio.share3"],
             "hunter2",
-            ["share1", "share2", "share3"],
+            [LOCATION_LOCAL_STORAGE, "share1", "share2", "share3"],
             True,
-            [LOCATION_LOCAL_STORAGE],
+            [],
         ),
+        # Prefer the list of locations which has LOCATION_LOCAL_STORAGE
         (
             [
                 {
                     "type": "backup/config/update",
                     "agents": {
                         "hassio.local": {"protected": False},
-                        "hassio.share1": {"protected": False},
-                    },
-                }
-            ],
-            "hunter2",
-            ["hassio.local", "hassio.share1", "hassio.share2", "hassio.share3"],
-            "hunter2",
-            ["share2", "share3"],
-            True,
-            [LOCATION_LOCAL_STORAGE, "share1"],
-        ),
-        (
-            [
-                {
-                    "type": "backup/config/update",
-                    "agents": {
-                        "hassio.local": {"protected": False},
-                        "hassio.share1": {"protected": False},
-                        "hassio.share2": {"protected": False},
                     },
                 }
             ],
             "hunter2",
             ["hassio.local", "hassio.share1", "hassio.share2", "hassio.share3"],
             None,
-            [LOCATION_LOCAL_STORAGE, "share1", "share2"],
+            [LOCATION_LOCAL_STORAGE],
+            True,
+            ["share1", "share2", "share3"],
+        ),
+        # If the list of locations does not have LOCATION_LOCAL_STORAGE, send the
+        # longest list
+        (
+            [
+                {
+                    "type": "backup/config/update",
+                    "agents": {
+                        "hassio.share0": {"protected": False},
+                    },
+                }
+            ],
+            "hunter2",
+            ["hassio.share0", "hassio.share1", "hassio.share2", "hassio.share3"],
+            "hunter2",
+            ["share1", "share2", "share3"],
+            True,
+            ["share0"],
+        ),
+        # Prefer the list of encrypted locations if the lists are the same length
+        (
+            [
+                {
+                    "type": "backup/config/update",
+                    "agents": {
+                        "hassio.share0": {"protected": False},
+                        "hassio.share1": {"protected": False},
+                    },
+                }
+            ],
+            "hunter2",
+            ["hassio.share0", "hassio.share1", "hassio.share2", "hassio.share3"],
+            "hunter2",
+            ["share2", "share3"],
+            True,
+            ["share0", "share1"],
+        ),
+        # If the list of locations does not have LOCATION_LOCAL_STORAGE, send the
+        # longest list
+        (
+            [
+                {
+                    "type": "backup/config/update",
+                    "agents": {
+                        "hassio.share0": {"protected": False},
+                        "hassio.share1": {"protected": False},
+                        "hassio.share2": {"protected": False},
+                    },
+                }
+            ],
+            "hunter2",
+            ["hassio.share0", "hassio.share1", "hassio.share2", "hassio.share3"],
+            None,
+            ["share0", "share1", "share2"],
             True,
             ["share3"],
         ),
@@ -1407,7 +1449,7 @@ async def test_reader_writer_create_per_agent_encryption(
                 server=f"share{i}",
                 type=supervisor_mounts.MountType.CIFS,
             )
-            for i in range(1, 4)
+            for i in range(4)
         ],
     )
     supervisor_client.backups.partial_backup.return_value.job_id = UUID(TEST_JOB_ID)