Merge pull request #2133 from ogayot/erase-install-exclude-renumbering

storage: exclude erase-install scenarios that renumber partitions
canonical · Jan 20, 2025 · 782dbb6 · 782dbb6
2 parents aaa1738 + 74a781c
commit 782dbb6
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 2 deletions.
diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py
@@ -181,7 +181,7 @@ def make_partition(
     model = device._m
     if size is None or size == -1 or offset is None:
         if offset is None:
-            gap = gaps.largest_gap(device)
+            gap = gaps.largest_gap(device, in_extended=flag == "logical")
             offset = gap.offset
         else:
             gap = gaps.includes(device, offset)

diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py
@@ -1233,6 +1233,17 @@ def available_erase_install_scenarios(
                 if partition.os is None:
                     continue
 
+                if (
+                    partition.is_logical
+                    and disk.partitions_by_number()[-1] != partition
+                ):
+                    # FIXME If we remove this partition, the subsequent logical
+                    # partitions will be renumbered. This will cause various
+                    # mismatches in the info returned by /storage/v2. For now,
+                    # we exclude this scenario.
+                    # See LP: #2091172
+                    continue
+
                 # Make an ephemeral copy of the disk object with the relevant
                 # partition removed. Then it's as if we're installing in the
                 # resulting gap (which will include free space that was

diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py
@@ -1789,6 +1789,55 @@ async def test_available_erase_install_scenarios__full_primaries(self):
 
         self.assertFalse(self.fsc.available_erase_install_scenarios(install_min))
 
+    async def test_available_erase_install_scenarios__with_logical_partitions(self):
+        await self._setup(Bootloader.UEFI, "dos", fix_bios=True)
+        install_min = self.fsc.calculate_suggested_install_min()
+
+        model, disk = self.model, self.disk
+        # Sizes are irrelevant
+        size = 4 << 20
+
+        # This is inspired from threebuntu-on-msdos.json
+        p1 = make_partition(model, disk, preserve=True, size=size)
+        make_partition(model, disk, preserve=True, size=size * 4, flag="extended")
+        make_partition(model, disk, preserve=True, size=size)  # This is the ESP
+        p5 = make_partition(model, disk, preserve=True, size=size, flag="logical")
+        p6 = make_partition(model, disk, preserve=True, size=size, flag="logical")
+
+        self.model._probe_data["os"] = {
+            p1._path(): {
+                "label": "Ubuntu",
+                "long": "Ubuntu 20.04.4 LTS",
+                "type": "linux",
+                "version": "20.04.4",
+            },
+            p5._path(): {
+                "label": "Ubuntu1",
+                "long": "Ubuntu 21.10",
+                "type": "linux",
+                "version": "21.10",
+            },
+            p6._path(): {
+                "label": "Ubuntu2",
+                "long": "Ubuntu 22.04 LTS",
+                "type": "linux",
+                "version": "22.04",
+            },
+        }
+
+        indexed_scenarios = self.fsc.available_erase_install_scenarios(install_min)
+
+        scenarios = [indexed_scenario[1] for indexed_scenario in indexed_scenarios]
+        sorted_scenarios = sorted(
+            scenarios, key=lambda sc: (sc.disk_id, sc.partition_number)
+        )
+        # Currently we expect only two scenarios because of the workaround for
+        # LP: #2091172. If we drop the workaround, we will have a third
+        # scenario for partition p5.
+        self.assertEqual(1, sorted_scenarios[0].partition_number)
+        self.assertEqual(6, sorted_scenarios[1].partition_number)
+        self.assertEqual(2, len(sorted_scenarios))
+
     async def test_resize_has_enough_room_for_partitions__one_primary(self):
         await self._setup(Bootloader.NONE, "gpt", fix_bios=True)
 
@@ -1842,7 +1891,7 @@ async def test_resize_has_enough_room_for_partitions__logical(self, p_boot_plan)
         # Sizes are irrelevant
         size = 4 << 20
         p1 = make_partition(model, disk, preserve=True, size=size)
-        p2 = make_partition(model, disk, preserve=True, size=size, flag="extended")
+        p2 = make_partition(model, disk, preserve=True, size=size * 2, flag="extended")
         p5 = make_partition(model, disk, preserve=True, size=size, flag="logical")
         p6 = make_partition(model, disk, preserve=True, size=size, flag="logical")
         p3 = make_partition(model, disk, preserve=True, size=size)