offer reformats for the install media

examples/machines/lp-1986676-missing-osprober.json

@@ -623,7 +623,7 @@
                     "range": "16",
                     "removable": "1",
                     "ro": "0",
-                    "size": "15728640000",
+                    "size": "157286400000",
                     "stat": "   71504      922  8712293   373538     2643     1512   442264   197181        0   367044   570719        0        0        0        0        0        0",
                     "subsystem": "block",
                     "uevent": "MAJOR=8\nMINOR=48\nDEVNAME=sdd\nDEVTYPE=disk\nDISKSEQ=15"

subiquity/common/filesystem/boot.py

@@ -15,6 +15,7 @@
 
 import abc
 import functools
+import logging
 from typing import Any, Optional
 
 import attr
@@ -29,6 +30,9 @@
     )
 
 
+log = logging.getLogger('subiquity.common.filesystem.boot')
+
+
 @functools.singledispatch
 def is_boot_device(device):
     """Is `device` a boot device?"""
@@ -331,7 +335,9 @@ def can_be_boot_device(device, *,
 def _can_be_boot_device_disk(disk, *,
                              resize_partition=None, with_reformatting=False):
     if with_reformatting:
-        return True
+        new_disk = attr.evolve(disk)
+        new_disk._partitions = [p for p in disk.partitions() if p._is_in_use]
+        disk = new_disk
     plan = get_boot_device_plan(disk, resize_partition=resize_partition)
     return plan is not None
 
@@ -358,7 +364,9 @@ def is_esp(device):
 
 @is_esp.register(Partition)
 def _is_esp_partition(partition):
-    if not can_be_boot_device(partition.device, with_reformatting=True):
+    new_disk = attr.evolve(partition.device)
+    new_disk._partitions = []
+    if not can_be_boot_device(new_disk, with_reformatting=True):
         return False
     if partition.device.ptable == "gpt":
         return partition.flag == "boot"

subiquity/server/controllers/filesystem.py

@@ -496,7 +496,13 @@ def start_guided(self, target: GuidedStorageTarget,
     def start_guided_reformat(self, target: GuidedStorageTargetReformat,
                               disk: ModelDisk) -> gaps.Gap:
         """Perform the reformat, and return the resulting gap."""
-        self.reformat(disk, wipe='superblock-recursive')
+        in_use_parts = [p for p in disk.partitions() if p._is_in_use]
+        if in_use_parts:
+            for p in disk.partitions():
+                if not p._is_in_use:
+                    self.delete_partition(p)
+        else:
+            self.reformat(disk, wipe='superblock-recursive')
         return gaps.largest_gap(disk)
 
     @start_guided.register
@@ -680,7 +686,7 @@ def potential_boot_disks(self, check_boot=True, with_reformatting=False):
                 if can_be_boot:
                     continue
             disks.append(disk)
-        return [d for d in disks if not d._has_in_use_partition]
+        return [d for d in disks]
 
     def _offsets_and_sizes_for_volume(self, volume):
         offset = self.model._partition_alignment_data['gpt'].min_start_offset
@@ -943,7 +949,11 @@ async def v2_guided_GET(self, wait: bool = False) \
             scenarios.append((disk.size, reformat))
 
         for disk in self.potential_boot_disks(with_reformatting=False):
-            parts = [p for p in disk.partitions() if p.flag != 'bios_grub']
+            parts = [
+                p
+                for p in disk.partitions()
+                if p.flag != 'bios_grub' and not p._is_in_use
+                ]
             if len(parts) < 1:
                 # On an (essentially) empty disk, don't bother to offer it
                 # with UseGap, as it's basically the same as the Reformat

subiquity/server/controllers/tests/test_filesystem.py

@@ -26,14 +26,15 @@
 from subiquitycore.utils import matching_dicts
 from subiquitycore.tests.util import random_string
 
-from subiquity.common.filesystem import gaps
+from subiquity.common.filesystem import gaps, labels
 from subiquity.common.filesystem.actions import DeviceAction
 from subiquity.common.types import (
     AddPartitionV2,
     Bootloader,
     Gap,
     GapUsable,
     GuidedCapability,
+    GuidedDisallowedCapability,
     GuidedDisallowedCapabilityReason,
     GuidedChoiceV2,
     GuidedStorageTargetManual,
@@ -76,6 +77,13 @@
     ]
 
 
+default_capabilities_disallowed_too_small = [
+    GuidedDisallowedCapability(
+        capability=cap,
+        reason=GuidedDisallowedCapabilityReason.TOO_SMALL)
+    for cap in default_capabilities]
+
+
 class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase):
     MOCK_PREFIX = 'subiquity.server.controllers.filesystem.'
 
@@ -646,24 +654,13 @@ async def test_manual(self):
     async def test_small_blank_disk(self, bootloader, ptable):
         await self._setup(bootloader, ptable, size=1 << 30)
         resp = await self.fsc.v2_guided_GET()
-        self.assertEqual(2, len(resp.targets))
-        self.assertEqual(GuidedStorageTargetManual(), resp.targets[1])
-        self.assertEqual(0, len(resp.targets[0].allowed))
-        self.assertEqual(
-            {
-                disabled_cap.capability
-                for disabled_cap in resp.targets[0].disallowed
-            },
-            set(default_capabilities)
-            )
-        self.assertEqual(
-            {
-                disabled_cap.reason
-                for disabled_cap in resp.targets[0].disallowed
-            },
-            {
-                GuidedDisallowedCapabilityReason.TOO_SMALL,
-            })
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=[],
+                disallowed=default_capabilities_disallowed_too_small),
+            GuidedStorageTargetManual(),
+            ]
+        self.assertEqual(expected, resp.targets)
 
     @parameterized.expand(bootloaders_and_ptables)
     async def test_used_half_disk(self, bootloader, ptable):
@@ -904,6 +901,93 @@ async def test_unscaled_disk(self, bootloader, ptable):
         # but we should using most of the disk, minus boot partition(s)
         self.assertLess(45 << 30, size)
 
+    @parameterized.expand(bootloaders_and_ptables)
+    async def test_in_use(self, bootloader, ptable):
+        # Disks with "in use" partitions allow a reformat if there is
+        # enough space on the rest of the disk.
+        await self._setup(bootloader, ptable, fix_bios=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=4 << 30, is_in_use=True)
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=default_capabilities),
+            GuidedStorageTargetManual(),
+            ]
+        resp = await self.fsc.v2_guided_GET()
+        self.assertEqual(expected, resp.targets)
+        self.assertEqual(ProbeStatus.DONE, resp.status)
+
+    @parameterized.expand(bootloaders_and_ptables)
+    async def test_in_use_full(self, bootloader, ptable):
+        # Disks with "in use" partitions allow a reformat (but not
+        # usegap) if there is enough space on the rest of the disk,
+        # even if the disk is full of other partitions.
+        await self._setup(bootloader, ptable, fix_bios=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=4 << 30, is_in_use=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=gaps.largest_gap_size(self.disk))
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=default_capabilities),
+            GuidedStorageTargetManual(),
+            ]
+        resp = await self.fsc.v2_guided_GET()
+        self.assertEqual(expected, resp.targets)
+        self.assertEqual(ProbeStatus.DONE, resp.status)
+
+    @parameterized.expand(bootloaders_and_ptables)
+    async def test_in_use_too_small(self, bootloader, ptable):
+        # Disks with "in use" partitions do not allow a reformat if
+        # there is not enough space on the rest of the disk.
+        await self._setup(bootloader, ptable, fix_bios=True, size=5 << 30)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=4 << 30, is_in_use=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=gaps.largest_gap_size(self.disk))
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=default_capabilities),
+            GuidedStorageTargetManual(),
+            ]
+        resp = await self.fsc.v2_guided_GET()
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=[],
+                disallowed=default_capabilities_disallowed_too_small),
+            GuidedStorageTargetManual(),
+            ]
+        self.assertEqual(expected, resp.targets)
+
+    @parameterized.expand(bootloaders_and_ptables)
+    async def test_in_use_reformat_and_gap(self, bootloader, ptable):
+        # Disks with "in use" partitions allow both a reformat and a
+        # usegap if there is an in use partition, another partition
+        # and a big enough gap.
+        await self._setup(bootloader, ptable, fix_bios=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=4 << 30, is_in_use=True)
+        make_partition(
+            self.model, self.disk, preserve=True,
+            size=gaps.largest_gap_size(self.disk)//2)
+        expected = [
+            GuidedStorageTargetReformat(
+                disk_id=self.disk.id, allowed=default_capabilities),
+            GuidedStorageTargetUseGap(
+                disk_id=self.disk.id, allowed=default_capabilities,
+                gap=labels.for_client(gaps.largest_gap(self.disk))),
+            GuidedStorageTargetManual(),
+            ]
+        resp = await self.fsc.v2_guided_GET()
+        self.assertEqual(expected, resp.targets)
+        self.assertEqual(ProbeStatus.DONE, resp.status)
+
 
 class TestManualBoot(IsolatedAsyncioTestCase):
     def _setup(self, bootloader, ptable, **kw):