Merge pull request #1781 from dbungert/min-size-revisited

Min size revisited

examples/sources/desktop.yaml

@@ -1,31 +1,11 @@
-- description:
-    en: A minimal but usable Ubuntu Desktop.
-  id: ubuntu-desktop-minimal
-  locale_support: langpack
-  name:
-    en: Ubuntu Desktop (minimized)
-  path: minimal.squashfs
-  preinstalled_langs:
-  - de
-  - en
-  - es
-  - fr
-  - it
-  - pt
-  - ru
-  - zh
-  - ''
-  size: 6135083008
-  type: fsimage-layered
-  variant: desktop
 - default: true
   description:
     en: A full featured Ubuntu Desktop.
   id: ubuntu-desktop
   locale_support: langpack
   name:
     en: Ubuntu Desktop
-  path: minimal.standard.squashfs
+  path: standard.squashfs
   preinstalled_langs:
   - de
   - en
@@ -36,6 +16,14 @@
   - ru
   - zh
   - ''
-  size: 7604871168
+  size: 4033826816
   type: fsimage-layered
   variant: desktop
+  variations:
+    enhanced-secureboot:
+      path: standard.enhanced-secureboot.squashfs
+      size: 4336062464
+      snapd_system_label: enhanced-secureboot-desktop
+    standard:
+      path: standard.squashfs
+      size: 4033826816

subiquity/common/filesystem/sizes.py

@@ -16,7 +16,6 @@
 import math
 
 import attr
-from curtin import swap
 
 from subiquity.common.types import GuidedResizeValues
 from subiquity.models.filesystem import GiB, MiB, align_down, align_up
@@ -119,24 +118,17 @@ def calculate_guided_resize(
 # 2) Room for boot - we employ a scaling system to help select the recommended
 #    size of a dedicated /boot and/or efi system partition (see above).  If
 #    /boot is not actually a separate partition, this space needs to be
-#    accounted for as part of the planned rootfs size.  The files that would
-#    otherwise be stored in a dedicated UEFI partition are presumed to be
-#    negligible on non-UEFI systems.  As /boot itself uses a scaling sizing
-#    system, use the bootfs_scale.maximum value for the purposes of finding
-#    minimum suggested size.  The maximum value is to be used instead of the
-#    minimum as the boot scaling system will also make adjustments, and we
-#    don’t want to short change the other calculations.
-# 3) Room to grow - while meaningful work can sometimes be possible on a full
+#    accounted for as part of the planned rootfs size.
+# 3) room for esp - similar to boot.  Included in all calculations, even if
+#    we're not UEFI boot.
+# 4) Room to grow - while meaningful work can sometimes be possible on a full
 #    disk, it’s not the sort of thing to suggest in a guided install.
-#    Suggest for room to grow max(2GiB, 80% of source minimum).
-# 4) Swap - Ubuntu policy recommends swap, either in the form of a partition or
-#    a swapfile.  Curtin has an existing swap size recommendation at
-#    curtin.swap.suggested_swapsize().
+#    Suggest for room to grow max(2GiB, 50% of source minimum).
 def calculate_suggested_install_min(source_min: int, part_align: int = MiB) -> int:
-    room_for_boot = bootfs_scale.maximum
-    room_to_grow = max(2 * GiB, math.ceil(0.8 * source_min))
-    room_for_swap = swap.suggested_swapsize()
-    total = source_min + room_for_boot + room_to_grow + room_for_swap
+    room_for_boot = bootfs_scale.minimum
+    room_for_esp = uefi_scale.minimum
+    room_to_grow = max(2 * GiB, math.ceil(0.5 * source_min))
+    total = source_min + room_for_boot + room_for_esp + room_to_grow
     return align_up(total, part_align)
 
 

subiquity/common/filesystem/tests/test_sizes.py

@@ -140,37 +140,29 @@ def assertAligned(val):
 
 
 class TestCalculateInstallMin(unittest.TestCase):
-    @mock.patch("subiquity.common.filesystem.sizes.swap.suggested_swapsize")
+    @mock.patch("subiquity.common.filesystem.sizes.uefi_scale")
     @mock.patch("subiquity.common.filesystem.sizes.bootfs_scale")
-    def test_small_setups(self, bootfs_scale, swapsize):
-        swapsize.return_value = 1 << 30
-        bootfs_scale.maximum = 1 << 30
+    def test_small_setups(self, bootfs_scale, uefi_scale):
+        uefi_scale.minimum = 1 << 30
+        bootfs_scale.minimum = 1 << 30
         source_min = 1 << 30
         # with a small source, we hit the default 2GiB padding
         self.assertEqual(5 << 30, calculate_suggested_install_min(source_min))
 
-    @mock.patch("subiquity.common.filesystem.sizes.swap.suggested_swapsize")
+    @mock.patch("subiquity.common.filesystem.sizes.uefi_scale")
     @mock.patch("subiquity.common.filesystem.sizes.bootfs_scale")
-    def test_small_setups_big_swap(self, bootfs_scale, swapsize):
-        swapsize.return_value = 10 << 30
-        bootfs_scale.maximum = 1 << 30
+    def test_small_setups_big_boot(self, bootfs_scale, uefi_scale):
+        uefi_scale.minimum = 1 << 30
+        bootfs_scale.minimum = 10 << 30
         source_min = 1 << 30
         self.assertEqual(14 << 30, calculate_suggested_install_min(source_min))
 
-    @mock.patch("subiquity.common.filesystem.sizes.swap.suggested_swapsize")
+    @mock.patch("subiquity.common.filesystem.sizes.uefi_scale")
     @mock.patch("subiquity.common.filesystem.sizes.bootfs_scale")
-    def test_small_setups_big_boot(self, bootfs_scale, swapsize):
-        swapsize.return_value = 1 << 30
-        bootfs_scale.maximum = 10 << 30
-        source_min = 1 << 30
-        self.assertEqual(14 << 30, calculate_suggested_install_min(source_min))
-
-    @mock.patch("subiquity.common.filesystem.sizes.swap.suggested_swapsize")
-    @mock.patch("subiquity.common.filesystem.sizes.bootfs_scale")
-    def test_big_source(self, bootfs_scale, swapsize):
-        swapsize.return_value = 1 << 30
-        bootfs_scale.maximum = 2 << 30
+    def test_big_source(self, bootfs_scale, uefi_scale):
+        uefi_scale.minimum = 1 << 30
+        bootfs_scale.minimum = 2 << 30
         source_min = 10 << 30
-        # a bigger source should hit 80% padding
-        expected = (10 + 8 + 1 + 2) << 30
+        # a bigger source should hit 50% padding
+        expected = (10 + 5 + 1 + 2) << 30
         self.assertEqual(expected, calculate_suggested_install_min(source_min))

subiquity/server/controllers/filesystem.py

@@ -70,6 +70,7 @@
     _Device,
     align_down,
     align_up,
+    humanize_size,
 )
 from subiquity.server import snapdapi
 from subiquity.server.controller import SubiquityController
@@ -157,13 +158,8 @@ def is_valid(self) -> bool:
     def capability_info_for_gap(
         self,
         gap: gaps.Gap,
+        install_min: int,
     ) -> CapabilityInfo:
-        if self.label is None:
-            install_min = sizes.calculate_suggested_install_min(
-                self.min_size, gap.device.alignment_data().part_align
-            )
-        else:
-            install_min = self.min_size
         r = CapabilityInfo()
         r.disallowed = list(self.capability_info.disallowed)
         if self.capability_info.allowed and gap.size < install_min:
@@ -931,7 +927,9 @@ def calculate_suggested_install_min(self):
         align = max(
             (pa.part_align for pa in self.model._partition_alignment_data.values())
         )
-        return sizes.calculate_suggested_install_min(source_min, align)
+        install_min = sizes.calculate_suggested_install_min(source_min, align)
+        log.debug(f"suggested install minimum size: {humanize_size(install_min)}")
+        return install_min
 
     async def get_v2_storage_response(self, model, wait, include_raid):
         probe_resp = await self._probe_response(wait, StorageResponseV2)
@@ -1015,7 +1013,9 @@ async def v2_guided_GET(self, wait: bool = False) -> GuidedStorageResponseV2:
             capability_info = CapabilityInfo()
             for variation in self._variation_info.values():
                 gap = gaps.largest_gap(disk._reformatted())
-                capability_info.combine(variation.capability_info_for_gap(gap))
+                capability_info.combine(
+                    variation.capability_info_for_gap(gap, install_min)
+                )
             reformat = GuidedStorageTargetReformat(
                 disk_id=disk.id,
                 allowed=capability_info.allowed,
@@ -1042,7 +1042,9 @@ async def v2_guided_GET(self, wait: bool = False) -> GuidedStorageResponseV2:
             for variation in self._variation_info.values():
                 if variation.is_core_boot_classic():
                     continue
-                capability_info.combine(variation.capability_info_for_gap(gap))
+                capability_info.combine(
+                    variation.capability_info_for_gap(gap, install_min)
+                )
             api_gap = labels.for_client(gap)
             use_gap = GuidedStorageTargetUseGap(
                 disk_id=disk.id,

subiquity/tests/api/test_api.py

@@ -386,7 +386,7 @@ async def test_v2_flow(self):
         cfg = self.machineConfig("examples/machines/simple.json")
         with cfg.edit() as data:
             attrs = data["storage"]["blockdev"]["/dev/sda"]["attrs"]
-            attrs["size"] = str(25 << 30)
+            attrs["size"] = str(10 << 30)
         extra_args = ["--source-catalog", "examples/sources/desktop.yaml"]
         async with start_server(cfg, extra_args=extra_args) as inst:
             disk_id = "disk-sda"