Merge pull request #1323 from dbungert/autoinstall-use-gap

filesystem: autoinstall support use_gap mode

subiquity/common/filesystem/manipulator.py

@@ -152,20 +152,22 @@ class FilesystemManipulator:
             return
         getattr(self, 'delete_' + obj.type)(obj)
 
-    def clear(self, obj):
+    def clear(self, obj, wipe=None):
         if obj.type == "disk":
             obj.preserve = False
-        obj.wipe = 'superblock'
+        if wipe is None:
+            wipe = 'superblock'
+        obj.wipe = wipe
         for subobj in obj.fs(), obj.constructed_device():
             self.delete(subobj)
 
-    def reformat(self, disk, ptable=None):
+    def reformat(self, disk, ptable=None, wipe=None):
         disk.grub_device = False
         if ptable is not None:
             disk.ptable = ptable
         for p in list(disk.partitions()):
             self.delete_partition(p, True)
-        self.clear(disk)
+        self.clear(disk, wipe)
 
     def can_resize_partition(self, partition):
         if not partition.preserve:

subiquity/server/controllers/filesystem.py

@@ -133,28 +133,27 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
                 "autoinstall config did not create needed bootloader "
                 "partition")
 
-    def guided_direct(self, disk):
+    def setup_disk_for_guided(self, disk, mode):
-        self.reformat(disk)
+        if mode is None or mode == 'reformat_disk':
-        gap = gaps.largest_gap(disk)
+            self.reformat(disk, wipe='superblock-recursive')
-        spec = {
-            "size": gap.size,
-            "fstype": "ext4",
-            "mount": "/",
-            }
-        self.partition_disk_handler(disk, spec, gap=gap)
-
-    def guided_lvm(self, disk, lvm_options=None):
-        self.reformat(disk)
         if DeviceAction.TOGGLE_BOOT in DeviceAction.supported(disk):
             self.add_boot_disk(disk)
-        gap = gaps.largest_gap(disk)
+        return gaps.largest_gap(disk)
-        size = sizes.get_bootfs_size(gap.size)
+
-        gap_boot, gap_rest = gap.split(size)
+    def guided_direct(self, disk, mode=None):
-        spec = dict(size=size, fstype="ext4", mount='/boot')
+        gap = self.setup_disk_for_guided(disk, mode)
+        spec = dict(fstype="ext4", mount="/")
+        self.create_partition(device=disk, gap=gap, spec=spec)
+
+    def guided_lvm(self, disk, mode=None, lvm_options=None):
+        gap = self.setup_disk_for_guided(disk, mode)
+        gap_boot, gap_rest = gap.split(sizes.get_bootfs_size(gap.size))
+        spec = dict(fstype="ext4", mount='/boot')
         self.create_partition(device=disk, gap=gap_boot, spec=spec)
 
-        spec = dict(size=gap_rest.size, fstype=None)
+        part = self.create_partition(
-        part = self.create_partition(device=disk, gap=gap_rest, spec=spec)
+                device=disk, gap=gap_rest, spec=dict(fstype=None))
+
         vg_name = 'ubuntu-vg'
         i = 0
         while self.model._one(type='lvm_volgroup', name=vg_name) is not None:
@@ -200,7 +199,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
                         'password': choice.password,
                         },
                     }
-            self.guided_lvm(disk, lvm_options)
+            self.guided_lvm(disk, lvm_options=lvm_options)
         else:
             self.guided_direct(disk)
 
@@ -461,19 +460,38 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
                 continue
             break
 
+    def run_guided(self, layout):
+        guided_method = getattr(self, "guided_" + layout['name'])
+        mode = layout.get('mode', 'reformat_disk')
+        self.validate_layout_mode(mode)
+
+        if mode == 'reformat_disk':
+            match = layout.get("match", {'size': 'largest'})
+            disk = self.model.disk_for_match(self.model.all_disks(), match)
+            if not disk:
+                raise Exception("autoinstall cannot configure storage "
+                                "- no disk found large enough for install")
+        elif mode == 'use_gap':
+            bootable = [d for d in self.model.all_disks()
+                        if boot.can_be_boot_device(d, with_reformatting=False)]
+            gap = gaps.largest_gap(bootable)
+            if not gap:
+                raise Exception("autoinstall cannot configure storage "
+                                "- no gap found large enough for install")
+            # This is not necessarily the exact gap to be used, as the gap size
+            # may change once add_boot_disk has sorted things out.
+            disk = gap.device
+        guided_method(disk=disk, mode=mode)
+
+    def validate_layout_mode(self, mode):
+        if mode not in ('reformat_disk', 'use_gap'):
+            raise ValueError(f'Unknown layout mode {mode}')
+
     @with_context()
     def convert_autoinstall_config(self, context=None):
         log.debug("self.ai_data = %s", self.ai_data)
         if 'layout' in self.ai_data:
-            layout = self.ai_data['layout']
+            self.run_guided(self.ai_data['layout'])
-            meth = getattr(self, "guided_" + layout['name'])
-            disk = self.model.disk_for_match(
-                self.model.all_disks(),
-                layout.get("match", {'size': 'largest'}))
-            if not disk:
-                raise Exception("autoinstall cannot configure storage "
-                                "- no disk found large enough for install")
-            meth(disk)
         elif 'config' in self.ai_data:
             self.model.apply_autoinstall_config(self.ai_data['config'])
             self.model.grub = self.ai_data.get('grub')

subiquity/server/controllers/tests/test_filesystem.py

@@ -15,6 +15,8 @@
 
 from unittest import mock, TestCase
 
+from parameterized import parameterized
+
 from subiquity.server.controllers.filesystem import FilesystemController
 
 from subiquitycore.tests.util import run_coro
@@ -103,3 +105,21 @@ class TestGuided(TestCase):
         self.assertEqual(None, d1p1.mount)
         self.assertEqual('/', d1p2.mount)
         self.assertEqual(d1p1.size + d1p1.offset, d1p2.offset)
+
+
+class TestLayout(TestCase):
+    def setUp(self):
+        self.app = make_app()
+        self.app.opts.bootloader = 'UEFI'
+        self.app.report_start_event = mock.Mock()
+        self.app.report_finish_event = mock.Mock()
+        self.fsc = FilesystemController(app=self.app)
+
+    @parameterized.expand([('reformat_disk',), ('use_gap',)])
+    def test_good_modes(self, mode):
+        self.fsc.validate_layout_mode(mode)
+
+    @parameterized.expand([('resize_biggest',), ('use_free',)])
+    def test_bad_modes(self, mode):
+        with self.assertRaises(ValueError):
+            self.fsc.validate_layout_mode(mode)