From 5ecaba6e91ecdca69e106d905b0e9b3b1aa41e09 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 19 Jun 2024 11:13:50 +0200 Subject: [PATCH] storage: make storage manipulator async ready Going forward, the client storage/filesystem controller should perform all of the storage actions by making async calls to the API. These calls will be implemented using coroutines functions that will call aiohttp. At the start, these new coroutine functions will override the methods from the manipulator. However, the functions defined by the manipulator are not coroutines functions so there is a mismatch. Move the needed functions to coroutine functions so that we can properly override them with coroutine functions. Signed-off-by: Olivier Gayot --- subiquity/common/filesystem/boot.py | 22 +-- subiquity/common/filesystem/manipulator.py | 120 ++++++++-------- .../filesystem/tests/test_manipulator.py | 134 +++++++++--------- subiquity/server/controllers/filesystem.py | 70 ++++----- .../controllers/tests/test_filesystem.py | 2 +- subiquity/ui/views/filesystem/delete.py | 19 ++- subiquity/ui/views/filesystem/filesystem.py | 23 +-- subiquity/ui/views/filesystem/lvm.py | 7 +- subiquity/ui/views/filesystem/partition.py | 23 ++- subiquity/ui/views/filesystem/raid.py | 7 +- .../ui/views/filesystem/tests/test_lvm.py | 2 + .../views/filesystem/tests/test_partition.py | 5 + .../ui/views/filesystem/tests/test_raid.py | 2 + 13 files changed, 240 insertions(+), 196 deletions(-) diff --git a/subiquity/common/filesystem/boot.py b/subiquity/common/filesystem/boot.py index d86e03dbd..833e2f0a9 100644 --- a/subiquity/common/filesystem/boot.py +++ b/subiquity/common/filesystem/boot.py @@ -64,7 +64,7 @@ class MakeBootDevicePlan(abc.ABC): """ @abc.abstractmethod - def apply(self, manipulator): + async def apply(self, manipulator): pass @@ -77,8 +77,10 @@ class CreatePartPlan(MakeBootDevicePlan): spec: dict = attr.ib(factory=dict) args: dict = attr.ib(factory=dict) - def apply(self, manipulator): - manipulator.create_partition(self.gap.device, self.gap, self.spec, **self.args) + async def apply(self, manipulator): + await manipulator.create_partition( + self.gap.device, self.gap, self.spec, **self.args + ) def _can_resize_part(inst, field, part): @@ -93,7 +95,7 @@ class ResizePlan(MakeBootDevicePlan): size_delta: int = 0 allow_resize_preserved: bool = False - def apply(self, manipulator): + async def apply(self, manipulator): self.part.size += self.size_delta if self.part.preserve: self.part.resize = True @@ -111,7 +113,7 @@ class SlidePlan(MakeBootDevicePlan): parts: list = attr.ib(validator=_no_preserve_parts) offset_delta: int = 0 - def apply(self, manipulator): + async def apply(self, manipulator): for part in self.parts: part.offset += self.offset_delta @@ -124,7 +126,7 @@ class SetAttrPlan(MakeBootDevicePlan): attr: str val: Any - def apply(self, manipulator): + async def apply(self, manipulator): setattr(self.device, self.attr, self.val) @@ -134,7 +136,7 @@ class MountBootEfiPlan(MakeBootDevicePlan): part: object - def apply(self, manipulator): + async def apply(self, manipulator): manipulator._mount_esp(self.part) @@ -142,7 +144,7 @@ def apply(self, manipulator): class NoOpBootPlan(MakeBootDevicePlan): """Do nothing, successfully""" - def apply(self, manipulator): + async def apply(self, manipulator): pass @@ -152,9 +154,9 @@ class MultiStepPlan(MakeBootDevicePlan): plans: list - def apply(self, manipulator): + async def apply(self, manipulator): for plan in self.plans: - plan.apply(manipulator) + await plan.apply(manipulator) def get_boot_device_plan_bios(device) -> Optional[MakeBootDevicePlan]: diff --git a/subiquity/common/filesystem/manipulator.py b/subiquity/common/filesystem/manipulator.py index e0226118f..7c92659f5 100644 --- a/subiquity/common/filesystem/manipulator.py +++ b/subiquity/common/filesystem/manipulator.py @@ -40,7 +40,7 @@ class FilesystemManipulator: - def create_mount(self, fs, spec): + async def create_mount(self, fs, spec): if spec.get("mount") is None: return mount = self.model.add_mount( @@ -49,15 +49,15 @@ def create_mount(self, fs, spec): if self.model.needs_bootloader_partition(): vol = fs.volume if vol.type == "partition" and boot.can_be_boot_device(vol.device): - self.add_boot_disk(vol.device) + await self.add_boot_disk(vol.device) return mount - def delete_mount(self, mount): + async def delete_mount(self, mount): if mount is None: return self.model.remove_mount(mount) - def create_filesystem(self, volume, spec): + async def create_filesystem(self, volume, spec): if spec.get("fstype") is None: # prep partitions are always wiped (and never have a filesystem) if getattr(volume, "flag", None) != "prep": @@ -84,18 +84,18 @@ def create_filesystem(self, volume, spec): elif spec.get("fstype") is None and spec.get("use_swap"): self.model.add_mount(fs, "") else: - self.create_mount(fs, spec) + await self.create_mount(fs, spec) return fs - def delete_filesystem(self, fs): + async def delete_filesystem(self, fs): if fs is None: return - self.delete_mount(fs.mount()) + await self.delete_mount(fs.mount()) self.model.remove_filesystem(fs) delete_format = delete_filesystem - def create_partition(self, device, gap, spec, **kw): + async def create_partition(self, device, gap, spec, **kw): flag = kw.pop("flag", None) if gap.in_extended: if flag not in (None, "logical"): @@ -106,45 +106,45 @@ def create_partition(self, device, gap, spec, **kw): part = self.model.add_partition( device, size=gap.size, offset=gap.offset, flag=flag, **kw ) - self.create_filesystem(part, spec) + await self.create_filesystem(part, spec) return part - def delete_partition(self, part, override_preserve=False): + async def delete_partition(self, part, override_preserve=False): if ( not override_preserve and part.device.preserve and self.model.storage_version < 2 ): raise Exception("cannot delete partitions from preserved disks") - self.clear(part) + await self.clear(part) self.model.remove_partition(part) - def create_raid(self, spec): + async def create_raid(self, spec): for d in spec["devices"] | spec["spare_devices"]: - self.clear(d) + await self.clear(d) raid = self.model.add_raid( spec["name"], spec["level"].value, spec["devices"], spec["spare_devices"] ) return raid - def delete_raid(self, raid): + async def delete_raid(self, raid): if raid is None: return - self.clear(raid) + await self.clear(raid) for v in raid._subvolumes: - self.delete_raid(v) + await self.delete_raid(v) for p in list(raid.partitions()): - self.delete_partition(p, True) + await self.delete_partition(p, True) for d in set(raid.devices) | set(raid.spare_devices): d.wipe = "superblock" self.model.remove_raid(raid) - def create_volgroup(self, spec): + async def create_volgroup(self, spec): devices = set() key = spec.get("passphrase") for device in spec["devices"]: - self.clear(device) + await self.clear(device) if key: device = self.model.add_dm_crypt( device, @@ -156,9 +156,9 @@ def create_volgroup(self, spec): create_lvm_volgroup = create_volgroup - def delete_volgroup(self, vg): + async def delete_volgroup(self, vg): for lv in list(vg.partitions()): - self.delete_logical_volume(lv) + await self.delete_logical_volume(lv) for d in vg.devices: d.wipe = "superblock" if d.type == "dm_crypt": @@ -167,15 +167,15 @@ def delete_volgroup(self, vg): delete_lvm_volgroup = delete_volgroup - def create_logical_volume(self, vg, spec): + async def create_logical_volume(self, vg, spec): lv = self.model.add_logical_volume(vg=vg, name=spec["name"], size=spec["size"]) - self.create_filesystem(lv, spec) + await self.create_filesystem(lv, spec) return lv create_lvm_partition = create_logical_volume - def delete_logical_volume(self, lv): - self.clear(lv) + async def delete_logical_volume(self, lv): + await self.clear(lv) self.model.remove_logical_volume(lv) delete_lvm_partition = delete_logical_volume @@ -188,13 +188,13 @@ def delete_logical_volume(self, lv): "swap", ] - def create_cryptoswap(self, device): + async def create_cryptoswap(self, device): dmc = self.model.add_dm_crypt( device, keyfile="/dev/urandom", options=self.cryptoswap_options, ) - self.create_filesystem(dmc, dict(fstype="swap")) + await self.create_filesystem(dmc, dict(fstype="swap")) return dmc def create_zpool( @@ -243,27 +243,27 @@ def create_zpool( keyfile=keyfile, ) - def delete(self, obj): + async def delete(self, obj): if obj is None: return - getattr(self, "delete_" + obj.type)(obj) + await getattr(self, "delete_" + obj.type)(obj) - def clear(self, obj, wipe=None): + async def clear(self, obj, wipe=None): if obj.type == "disk": obj.preserve = False if wipe is None: wipe = "superblock" obj.wipe = wipe for subobj in obj.fs(), obj.constructed_device(): - self.delete(subobj) + await self.delete(subobj) - def reformat(self, disk, ptable=None, wipe=None): + async 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, wipe) + await self.delete_partition(p, True) + await self.clear(disk, wipe) def can_resize_partition(self, partition): if not partition.preserve: @@ -272,7 +272,7 @@ def can_resize_partition(self, partition): return False return True - def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): + async def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): log.debug("partition_disk_handler: %s %s %s %s", disk, spec, partition, gap) if disk.on_remote_storage(): @@ -293,8 +293,8 @@ def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): partition.resize = True for part in trailing: part.offset += size_change - self.delete_filesystem(partition.fs()) - self.create_filesystem(partition, spec) + await self.delete_filesystem(partition.fs()) + await self.create_filesystem(partition, spec) return if len(disk.partitions()) == 0: @@ -308,7 +308,7 @@ def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): log.debug("model needs a bootloader partition? {}".format(needs_boot)) can_be_boot = boot.can_be_boot_device(disk) if needs_boot and len(disk.partitions()) == 0 and can_be_boot: - self.add_boot_disk(disk) + await self.add_boot_disk(disk) # adjust downward the partition size (if necessary) to accommodate # bios/grub partition. It's OK and useful to assign a new gap: @@ -322,11 +322,11 @@ def partition_disk_handler(self, disk, spec, *, partition=None, gap=None): spec["size"] = gap.size gap = gap.split(spec["size"])[0] - self.create_partition(disk, gap, spec) + await self.create_partition(disk, gap, spec) log.debug("Successfully added partition") - def logical_volume_handler(self, vg, spec, *, partition, gap): + async def logical_volume_handler(self, vg, spec, *, partition, gap): # keep the partition name for compat with PartitionStretchy.handler lv = partition @@ -342,33 +342,33 @@ def logical_volume_handler(self, vg, spec, *, partition, gap): lv.size = align_up(spec["size"]) if gaps.largest_gap_size(vg) < 0: raise Exception("lv size too large") - self.delete_filesystem(lv.fs()) - self.create_filesystem(lv, spec) + await self.delete_filesystem(lv.fs()) + await self.create_filesystem(lv, spec) return - self.create_logical_volume(vg, spec) + await self.create_logical_volume(vg, spec) - def add_format_handler(self, volume, spec): + async def add_format_handler(self, volume, spec): log.debug("add_format_handler %s %s", volume, spec) - self.clear(volume) - self.create_filesystem(volume, spec) + await self.clear(volume) + await self.create_filesystem(volume, spec) - def raid_handler(self, existing, spec): + async def raid_handler(self, existing, spec): log.debug("raid_handler %s %s", existing, spec) if existing is not None: for d in existing.devices | existing.spare_devices: d._constructed_device = None for d in spec["devices"] | spec["spare_devices"]: - self.clear(d) + await self.clear(d) d._constructed_device = existing existing.name = spec["name"] existing.raidlevel = spec["level"].value existing.devices = spec["devices"] existing.spare_devices = spec["spare_devices"] else: - self.create_raid(spec) + await self.create_raid(spec) - def volgroup_handler(self, existing, spec): + async def volgroup_handler(self, existing, spec): if existing is not None: key = spec.get("passphrase") for d in existing.devices: @@ -378,7 +378,7 @@ def volgroup_handler(self, existing, spec): d._constructed_device = None devices = set() for d in spec["devices"]: - self.clear(d) + await self.clear(d) if key: d = self.model.add_dm_crypt( d, @@ -390,14 +390,14 @@ def volgroup_handler(self, existing, spec): existing.name = spec["name"] existing.devices = devices else: - self.create_volgroup(spec) + await self.create_volgroup(spec) def _mount_esp(self, part): if part.fs() is None: self.model.add_filesystem(part, "fat32") self.model.add_mount(part.fs(), "/boot/efi") - def remove_boot_disk(self, boot_disk): + async def remove_boot_disk(self, boot_disk): if self.model.bootloader == Bootloader.BIOS: boot_disk.grub_device = False partitions = [ @@ -414,10 +414,10 @@ def remove_boot_disk(self, boot_disk): elif self.model.bootloader == Bootloader.UEFI: if p.fs(): if p.fs().mount(): - self.delete_mount(p.fs().mount()) + await self.delete_mount(p.fs().mount()) remount = True if not p.fs().preserve and p.original_fstype(): - self.delete_filesystem(p.fs()) + await self.delete_filesystem(p.fs()) self.model.add_filesystem( p, p.original_fstype(), preserve=True ) @@ -428,7 +428,7 @@ def remove_boot_disk(self, boot_disk): tot_size += p.size if p.fs() and p.fs().mount(): remount = True - self.delete_partition(p) + await self.delete_partition(p) if full: largest_part = max(boot_disk.partitions(), key=lambda p: p.size) largest_part.size += tot_size @@ -437,14 +437,16 @@ def remove_boot_disk(self, boot_disk): if part: self._mount_esp(part) - def add_boot_disk(self, new_boot_disk): + async def add_boot_disk(self, new_boot_disk): + # NOTE this is async because FilesystemController.add_boot_disk needs + # to be async. if not self.supports_resilient_boot: for disk in boot.all_boot_devices(self.model): - self.remove_boot_disk(disk) + await self.remove_boot_disk(disk) plan = boot.get_boot_device_plan(new_boot_disk) if plan is None: raise ValueError(f"No known plan to make {new_boot_disk} bootable") - plan.apply(self) + await plan.apply(self) if not new_boot_disk._has_preexisting_partition(): if new_boot_disk.type == "disk": new_boot_disk.preserve = False diff --git a/subiquity/common/filesystem/tests/test_manipulator.py b/subiquity/common/filesystem/tests/test_manipulator.py index 64622dceb..0dcc0219d 100644 --- a/subiquity/common/filesystem/tests/test_manipulator.py +++ b/subiquity/common/filesystem/tests/test_manipulator.py @@ -70,20 +70,20 @@ class Create: size: int -class TestFilesystemManipulator(unittest.TestCase): - def test_delete_encrypted_vg(self): +class TestFilesystemManipulator(unittest.IsolatedAsyncioTestCase): + async def test_delete_encrypted_vg(self): manipulator, disk = make_manipulator_and_disk() spec = { "passphrase": "passw0rd", "devices": {disk}, "name": "vg0", } - vg = manipulator.create_volgroup(spec) - manipulator.delete_volgroup(vg) + vg = await manipulator.create_volgroup(spec) + await manipulator.delete_volgroup(vg) dm_crypts = [a for a in manipulator.model._actions if a.type == "dm_crypt"] self.assertEqual(dm_crypts, []) - def test_wipe_existing_fs(self): + async def test_wipe_existing_fs(self): # LP: #1983036 - edit a partition to wipe it and mount it, but not # actually change the fs type already there # example: it's ext2, wipe and make it ext2 again @@ -93,24 +93,24 @@ def test_wipe_existing_fs(self): manipulator.model, partition=p, fstype="ext2", preserve=True ) manipulator.model._actions.append(fs) - manipulator.delete_filesystem(fs) + await manipulator.delete_filesystem(fs) spec = {"wipe": "random"} with mock.patch.object(p, "original_fstype") as original_fstype: original_fstype.return_value = "ext2" - fs = manipulator.create_filesystem(p, spec) + fs = await manipulator.create_filesystem(p, spec) self.assertTrue(p.preserve) self.assertEqual("random", p.wipe) self.assertFalse(fs.preserve) self.assertEqual("ext2", fs.fstype) - def test_can_only_add_boot_once(self): + async def test_can_only_add_boot_once(self): # This is really testing model code but it's much easier to test with a # manipulator around. for bl in Bootloader: manipulator, disk = make_manipulator_and_disk(bl) if DeviceAction.TOGGLE_BOOT not in DeviceAction.supported(disk): continue - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertFalse( DeviceAction.TOGGLE_BOOT.can(disk)[0], "add_boot_disk(disk) did not make _can_TOGGLE_BOOT false " @@ -171,7 +171,7 @@ def assertIsNotBootDisk(self, manipulator, disk): if part.flag == "prep" and part.grub_device: self.fail("{} is a boot disk".format(disk)) - def test_boot_disk_resilient(self): + async def test_boot_disk_resilient(self): for bl in Bootloader: if bl == Bootloader.NONE: continue @@ -185,13 +185,13 @@ def test_boot_disk_resilient(self): disk2, size=gap.size, offset=gap.offset ) - manipulator.add_boot_disk(disk1) + await manipulator.add_boot_disk(disk1) self.assertIsBootDisk(manipulator, disk1) if bl == Bootloader.UEFI: self.assertIsMountedAtBootEFI(disk1.partitions()[0]) size_before = disk2p1.size - manipulator.add_boot_disk(disk2) + await manipulator.add_boot_disk(disk2) self.assertIsBootDisk(manipulator, disk1) self.assertIsBootDisk(manipulator, disk2) if bl == Bootloader.UEFI: @@ -201,19 +201,19 @@ def test_boot_disk_resilient(self): self.assertEqual(disk2.partitions()[1], disk2p1) self.assertEqual(disk2.partitions()[0].size + disk2p1.size, size_before) - manipulator.remove_boot_disk(disk1) + await manipulator.remove_boot_disk(disk1) self.assertIsNotBootDisk(manipulator, disk1) self.assertIsBootDisk(manipulator, disk2) if bl == Bootloader.UEFI: self.assertIsMountedAtBootEFI(disk2.partitions()[0]) self.assertEqual(len(disk1.partitions()), 0) - manipulator.remove_boot_disk(disk2) + await manipulator.remove_boot_disk(disk2) self.assertIsNotBootDisk(manipulator, disk2) self.assertEqual(len(disk2.partitions()), 1) self.assertEqual(disk2p1.size, size_before) - def test_boot_disk_no_resilient(self): + async def test_boot_disk_no_resilient(self): for bl in Bootloader: if bl == Bootloader.NONE: continue @@ -227,13 +227,13 @@ def test_boot_disk_no_resilient(self): disk2, size=gap.size, offset=gap.offset ) - manipulator.add_boot_disk(disk1) + await manipulator.add_boot_disk(disk1) self.assertIsBootDisk(manipulator, disk1) if bl == Bootloader.UEFI: self.assertIsMountedAtBootEFI(disk1.partitions()[0]) size_before = disk2p1.size - manipulator.add_boot_disk(disk2) + await manipulator.add_boot_disk(disk2) self.assertIsNotBootDisk(manipulator, disk1) self.assertIsBootDisk(manipulator, disk2) if bl == Bootloader.UEFI: @@ -242,7 +242,7 @@ def test_boot_disk_no_resilient(self): self.assertEqual(disk2.partitions()[1], disk2p1) self.assertEqual(disk2.partitions()[0].size + disk2p1.size, size_before) - def test_boot_disk_existing(self): + async def test_boot_disk_existing(self): for bl in Bootloader: if bl == Bootloader.NONE: continue @@ -252,19 +252,19 @@ def test_boot_disk_existing(self): part = self.add_existing_boot_partition(manipulator, disk1) wipe_before = part.wipe - manipulator.add_boot_disk(disk1) + await manipulator.add_boot_disk(disk1) self.assertIsBootDisk(manipulator, disk1) if bl == Bootloader.UEFI: self.assertIsMountedAtBootEFI(part) - manipulator.remove_boot_disk(disk1) + await manipulator.remove_boot_disk(disk1) self.assertIsNotBootDisk(manipulator, disk1) self.assertEqual(len(disk1.partitions()), 1) self.assertEqual(part.wipe, wipe_before) if bl == Bootloader.UEFI: self.assertNotMounted(part) - def test_mounting_partition_makes_boot_disk(self): + async def test_mounting_partition_makes_boot_disk(self): manipulator = make_manipulator(Bootloader.UEFI) disk1 = make_disk(manipulator.model, preserve=True) disk1p1 = manipulator.model.add_partition( @@ -275,20 +275,24 @@ def test_mounting_partition_makes_boot_disk(self): disk1, size=8192 << 20, offset=513 << 20 ) disk1p2.preserve = True - manipulator.partition_disk_handler( - disk1, {"fstype": "ext4", "mount": "/"}, partition=disk1p2 - ) + with mock.patch.object( + manipulator, "add_boot_disk", side_effect=manipulator.add_boot_disk + ) as m_add_boot_disk: + await manipulator.partition_disk_handler( + disk1, {"fstype": "ext4", "mount": "/"}, partition=disk1p2 + ) efi_mnt = manipulator.model._mount_for_path("/boot/efi") self.assertEqual(efi_mnt.device.volume, disk1p1) + m_add_boot_disk.assert_called_once() - def test_add_boot_has_valid_offset(self): + async def test_add_boot_has_valid_offset(self): for bl in Bootloader: if bl == Bootloader.NONE: continue manipulator = make_manipulator(bl) disk1 = make_disk(manipulator.model, preserve=True) - manipulator.add_boot_disk(disk1) + await manipulator.add_boot_disk(disk1) part = gaps.parts_and_gaps(disk1)[0] self.assertEqual(1024 * 1024, part.offset) @@ -332,7 +336,7 @@ def assertPartitionOperations(self, disk, *ops): self.fail("no assertion about {}".format(new_parts)) @parameterized.expand([(1,), (2,)]) - def test_add_boot_BIOS_empty(self, version): + async def test_add_boot_BIOS_empty(self, version): manipulator = make_manipulator(Bootloader.BIOS, version) disk = make_disk(manipulator.model, preserve=True) with self.assertPartitionOperations( @@ -342,12 +346,12 @@ def test_add_boot_BIOS_empty(self, version): size=sizes.BIOS_GRUB_SIZE_BYTES, ), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(1,), (2,)]) - def test_add_boot_BIOS_full(self, version): + async def test_add_boot_BIOS_full(self, version): manipulator = make_manipulator(Bootloader.BIOS, version) disk = make_disk(manipulator.model, preserve=True) part = make_partition(manipulator.model, disk, size=gaps.largest_gap_size(disk)) @@ -364,12 +368,12 @@ def test_add_boot_BIOS_full(self, version): size=-sizes.BIOS_GRUB_SIZE_BYTES, ), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(1,), (2,)]) - def test_add_boot_BIOS_half_full(self, version): + async def test_add_boot_BIOS_half_full(self, version): manipulator = make_manipulator(Bootloader.BIOS, version) disk = make_disk(manipulator.model, preserve=True) part = make_partition( @@ -383,11 +387,11 @@ def test_add_boot_BIOS_half_full(self, version): ), Move(part=part, offset=sizes.BIOS_GRUB_SIZE_BYTES), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(1,), (2,)]) - def test_add_boot_BIOS_full_resizes_larger(self, version): + async def test_add_boot_BIOS_full_resizes_larger(self, version): manipulator = make_manipulator(Bootloader.BIOS) # 2002MiB so that the space available for partitioning (2000MiB) # divided by 4 is an whole number of megabytes. @@ -411,7 +415,7 @@ def test_add_boot_BIOS_full_resizes_larger(self, version): size=-sizes.BIOS_GRUB_SIZE_BYTES, ), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(1,), (2,)]) @@ -431,7 +435,7 @@ def test_no_add_boot_BIOS_preserved_v1(self): ) self.assertFalse(boot.can_be_boot_device(disk)) - def test_add_boot_BIOS_preserved_v2(self): + async def test_add_boot_BIOS_preserved_v2(self): manipulator = make_manipulator(Bootloader.BIOS, 2) disk = make_disk(manipulator.model, preserve=True) half_size = gaps.largest_gap_size(disk) // 2 @@ -446,10 +450,10 @@ def test_add_boot_BIOS_preserved_v2(self): ), Unchanged(part=part), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) - def test_add_boot_BIOS_new_and_preserved_v2(self): + async def test_add_boot_BIOS_new_and_preserved_v2(self): manipulator = make_manipulator(Bootloader.BIOS, 2) # 2002MiB so that the space available for partitioning (2000MiB) # divided by 4 is an whole number of megabytes. @@ -472,7 +476,7 @@ def test_add_boot_BIOS_new_and_preserved_v2(self): ), Unchanged(part=p2_preserved), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(1,), (2,)]) @@ -492,7 +496,7 @@ def test_no_add_boot_BIOS_gpt_teeny_disk(self, version): self.assertFalse(boot.can_be_boot_device(disk)) @parameterized.expand([(1,), (2,)]) - def test_add_boot_BIOS_msdos(self, version): + async def test_add_boot_BIOS_msdos(self, version): manipulator = make_manipulator(Bootloader.BIOS, version) disk = make_disk(manipulator.model, preserve=True, ptable="msdos") part = make_partition( @@ -502,7 +506,7 @@ def test_add_boot_BIOS_msdos(self, version): disk, Unchanged(part=part), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) def boot_size_for_disk(self, disk): @@ -522,7 +526,7 @@ def boot_size_for_disk(self, disk): ] @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_empty(self, bl, version): + async def test_add_boot_empty(self, bl, version): manipulator = make_manipulator(bl, version) disk = make_disk(manipulator.model, preserve=True) with self.assertPartitionOperations( @@ -532,11 +536,11 @@ def test_add_boot_empty(self, bl, version): size=self.boot_size_for_disk(disk), ), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_full(self, bl, version): + async def test_add_boot_full(self, bl, version): manipulator = make_manipulator(bl, version) disk = make_disk(manipulator.model, preserve=True) part = make_partition(manipulator.model, disk, size=gaps.largest_gap_size(disk)) @@ -546,11 +550,11 @@ def test_add_boot_full(self, bl, version): Create(offset=disk.alignment_data().min_start_offset, size=size), MoveResize(part=part, offset=size, size=-size), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_half_full(self, bl, version): + async def test_add_boot_half_full(self, bl, version): manipulator = make_manipulator(bl, version) disk = make_disk(manipulator.model, preserve=True) part = make_partition( @@ -562,11 +566,11 @@ def test_add_boot_half_full(self, bl, version): Unchanged(part=part), Create(offset=part.offset + part.size, size=size), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_full_resizes_larger(self, bl, version): + async def test_add_boot_full_resizes_larger(self, bl, version): manipulator = make_manipulator(bl, version) # 2002MiB so that the space available for partitioning (2000MiB) # divided by 4 is an whole number of megabytes. @@ -584,7 +588,7 @@ def test_add_boot_full_resizes_larger(self, bl, version): Create(offset=part_smaller.offset + part_smaller.size, size=size), MoveResize(part=part_larger, offset=size, size=-size), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand(ADD_BOOT_PARAMS) @@ -596,7 +600,7 @@ def test_no_add_boot_full_preserved_partition(self, bl, version): self.assertFalse(boot.can_be_boot_device(disk)) @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_half_preserved_partition(self, bl, version): + async def test_add_boot_half_preserved_partition(self, bl, version): manipulator = make_manipulator(bl, version) disk = make_disk(manipulator.model, preserve=True) half_size = gaps.largest_gap_size(disk) // 2 @@ -610,11 +614,11 @@ def test_add_boot_half_preserved_partition(self, bl, version): Unchanged(part), Create(offset=part.offset + part.size, size=size), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand([(Bootloader.UEFI,), (Bootloader.PREP,)]) - def test_add_boot_half_preserved_half_new_partition(self, bl): + async def test_add_boot_half_preserved_half_new_partition(self, bl): # This test is v2 only because you can only get into this # situation in a v2 world. manipulator = make_manipulator(bl, 2) @@ -630,11 +634,11 @@ def test_add_boot_half_preserved_half_new_partition(self, bl): Create(offset=new_part.offset, size=size), MoveResize(part=new_part, offset=size, size=-size), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) @parameterized.expand(ADD_BOOT_PARAMS) - def test_add_boot_existing_boot_partition(self, bl, version): + async def test_add_boot_existing_boot_partition(self, bl, version): manipulator = make_manipulator(bl, version) disk = make_disk(manipulator.model, preserve=True, size=2002 * MiB) if bl == Bootloader.UEFI: @@ -651,7 +655,7 @@ def test_add_boot_existing_boot_partition(self, bl, version): Unchanged(boot_part), Unchanged(rest_part), ): - manipulator.add_boot_disk(disk) + await manipulator.add_boot_disk(disk) self.assertIsBootDisk(manipulator, disk) def test_logical_no_esp_in_gap(self): @@ -698,39 +702,39 @@ def test_too_many_primaries(self, bl): self.assertFalse(gap.is_usable) self.assertFalse(boot.can_be_boot_device(d1)) - def test_logical_when_in_extended(self): + async def test_logical_when_in_extended(self): manipulator = make_manipulator(storage_version=2) m = manipulator.model d1 = make_disk(m, preserve=True, ptable="msdos") size = gaps.largest_gap_size(d1) make_partition(m, d1, flag="extended", size=size) gap = gaps.largest_gap(d1) - p = manipulator.create_partition(d1, gap, spec={}, flag="primary") + p = await manipulator.create_partition(d1, gap, spec={}, flag="primary") self.assertEqual(p.flag, "logical") -class TestReformat(unittest.TestCase): +class TestReformat(unittest.IsolatedAsyncioTestCase): def setUp(self): self.manipulator = make_manipulator() - def test_reformat_default(self): + async def test_reformat_default(self): disk = make_disk(self.manipulator.model, ptable=None) - self.manipulator.reformat(disk) + await self.manipulator.reformat(disk) self.assertEqual(None, disk.ptable) - def test_reformat_keep_current(self): + async def test_reformat_keep_current(self): disk = make_disk(self.manipulator.model, ptable="msdos") - self.manipulator.reformat(disk) + await self.manipulator.reformat(disk) self.assertEqual("msdos", disk.ptable) - def test_reformat_to_gpt(self): + async def test_reformat_to_gpt(self): disk = make_disk(self.manipulator.model, ptable=None) - self.manipulator.reformat(disk, "gpt") + await self.manipulator.reformat(disk, "gpt") self.assertEqual("gpt", disk.ptable) - def test_reformat_to_msdos(self): + async def test_reformat_to_msdos(self): disk = make_disk(self.manipulator.model, ptable=None) - self.manipulator.reformat(disk, "msdos") + await self.manipulator.reformat(disk, "msdos") self.assertEqual("msdos", disk.ptable) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 0802e1600..3b4c31b52 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -558,21 +558,21 @@ def update_devices(self, device_map): if action in self._device_to_structure: self._device_to_structure[action].device = path - def guided_direct(self, gap): + async def guided_direct(self, gap): spec = dict(fstype="ext4", mount="/") - self.create_partition(device=gap.device, gap=gap, spec=spec) + await self.create_partition(device=gap.device, gap=gap, spec=spec) def guided_dd(self, disk: ModelDisk): self.model.dd_target = disk - def guided_lvm(self, gap, choice: GuidedChoiceV2): + async def guided_lvm(self, gap, choice: GuidedChoiceV2): device = gap.device part_align = device.alignment_data().part_align bootfs_size = align_up(sizes.get_bootfs_size(gap.size), part_align) gap_boot, gap_rest = gap.split(bootfs_size) spec = dict(fstype="ext4", mount="/boot") - self.create_partition(device, gap_boot, spec) - part = self.create_partition(device, gap_rest, dict(fstype=None)) + await self.create_partition(device, gap_boot, spec) + part = await self.create_partition(device, gap_rest, dict(fstype=None)) vg_name = "ubuntu-vg" i = 0 @@ -588,7 +588,7 @@ def guided_lvm(self, gap, choice: GuidedChoiceV2): choice.recovery_key, default_suffix=f"recovery-key-{vg_name}.txt" ) - vg = self.create_volgroup(spec) + vg = await self.create_volgroup(spec) if choice.sizing_policy == SizingPolicy.SCALED: lv_size = sizes.scaled_rootfs_size(vg.size) lv_size = align_down(lv_size, LVM_CHUNK_SIZE) @@ -597,7 +597,7 @@ def guided_lvm(self, gap, choice: GuidedChoiceV2): else: raise Exception(f"Unhandled size policy {choice.sizing_policy}") log.debug(f"lv_size {lv_size} for {choice.sizing_policy}") - self.create_logical_volume( + await self.create_logical_volume( vg=vg, spec=dict( size=lv_size, @@ -609,12 +609,12 @@ def guided_lvm(self, gap, choice: GuidedChoiceV2): self.model.load_or_generate_recovery_keys() self.model.expose_recovery_keys() - def guided_zfs(self, gap, choice: GuidedChoiceV2): + async def guided_zfs(self, gap, choice: GuidedChoiceV2): device = gap.device part_align = device.alignment_data().part_align bootfs_size = align_up(sizes.get_bootfs_size(gap.size), part_align) gap_boot, gap_rest = gap.split(bootfs_size) - bpart = self.create_partition(device, gap_boot, dict(fstype=None)) + bpart = await self.create_partition(device, gap_boot, dict(fstype=None)) encryption_style = None if encrypted := choice.password is not None: encryption_style = "luks_keystore" @@ -624,13 +624,13 @@ def guided_zfs(self, gap, choice: GuidedChoiceV2): if swap_size > 0: gap_swap, gap = gap_rest.split(swap_size) if encrypted: - part = self.create_partition(device, gap_swap, {}) - self.create_cryptoswap(part) + part = await self.create_partition(device, gap_swap, {}) + await self.create_cryptoswap(part) else: - self.create_partition(device, gap_swap, dict(fstype="swap")) + await self.create_partition(device, gap_swap, dict(fstype="swap")) else: gap = gap_rest - rpart = self.create_partition(device, gap, dict(fstype=None)) + rpart = await self.create_partition(device, gap, dict(fstype=None)) uuid = gen_zsys_uuid() @@ -671,13 +671,15 @@ def guided_zfs(self, gap, choice: GuidedChoiceV2): rpool.create_zfs(f"USERDATA/home_{userdata_uuid}", mountpoint="/home") @functools.singledispatchmethod - def start_guided(self, target: GuidedStorageTarget, disk: ModelDisk) -> gaps.Gap: + async def start_guided( + self, target: GuidedStorageTarget, disk: ModelDisk + ) -> gaps.Gap: """Setup changes to the disk to prepare the gap that we will be doing a guided install into.""" raise NotImplementedError(target) @start_guided.register - def start_guided_reformat( + async def start_guided_reformat( self, target: GuidedStorageTargetReformat, disk: ModelDisk ) -> gaps.Gap: """Perform the reformat, and return the resulting gap.""" @@ -685,20 +687,20 @@ def start_guided_reformat( if in_use_parts: for p in disk.partitions(): if not p._is_in_use: - self.delete_partition(p) + await self.delete_partition(p) else: - self.reformat(disk, wipe="superblock-recursive") + await self.reformat(disk, wipe="superblock-recursive") return gaps.largest_gap(disk) @start_guided.register - def start_guided_use_gap( + async def start_guided_use_gap( self, target: GuidedStorageTargetUseGap, disk: ModelDisk ) -> gaps.Gap: """Lookup the matching model gap.""" return gaps.at_offset(disk, target.gap.offset) @start_guided.register - def start_guided_resize( + async def start_guided_resize( self, target: GuidedStorageTargetResize, disk: ModelDisk ) -> gaps.Gap: """Perform the resize of the target partition, @@ -768,9 +770,9 @@ async def guided( await self.guided_core_boot(disk) return - gap = self.start_guided(choice.target, disk) + gap = await self.start_guided(choice.target, disk) if DeviceAction.TOGGLE_BOOT in DeviceAction.supported(disk): - self.add_boot_disk(disk) + await self.add_boot_disk(disk) # find what's left of the gap after adding boot gap = gap.within() if gap is None: @@ -787,7 +789,7 @@ async def guided( reset_size = int(cp.stdout.strip().split()[0]) reset_size = align_up(int(reset_size * 1.10), 256 * MiB) reset_gap, gap = gap.split(reset_size) - self.model.reset_partition = self.create_partition( + self.model.reset_partition = await self.create_partition( device=reset_gap.device, gap=reset_gap, spec={"fstype": "fat32"}, @@ -796,16 +798,16 @@ async def guided( self.reset_partition_only = reset_partition_only if reset_partition_only: for mount in self.model._all(type="mount"): - self.delete_mount(mount) + await self.delete_mount(mount) self.model.target = self.app.base_model.target = None return if choice.capability.is_lvm(): - self.guided_lvm(gap, choice) + await self.guided_lvm(gap, choice) elif choice.capability.is_zfs(): - self.guided_zfs(gap, choice) + await self.guided_zfs(gap, choice) elif choice.capability == GuidedCapability.DIRECT: - self.guided_direct(gap) + await self.guided_direct(gap) elif choice.capability == GuidedCapability.DD: self.guided_dd(disk) else: @@ -927,11 +929,11 @@ async def guided_core_boot(self, disk: Disk): for part in list(disk.partitions()): if part not in preserved_parts: - self.delete_partition(part) + await self.delete_partition(part) del parts_by_offset_size[(part.offset, part.size)] if not preserved_parts: - self.reformat(disk, self._on_volume.schema) + await self.reformat(disk, self._on_volume.schema) for structure, offset, size in self._offsets_and_sizes_for_volume( self._on_volume @@ -957,7 +959,7 @@ async def guided_core_boot(self, disk: Disk): part.partition_name = structure.name if structure.filesystem: part.wipe = "superblock" - self.delete_filesystem(part.fs()) + await self.delete_filesystem(part.fs()) fs = self.model.add_filesystem( part, structure.filesystem, label=structure.label ) @@ -1226,7 +1228,7 @@ async def v2_guided_POST(self, data: GuidedChoiceV2) -> GuidedStorageResponseV2: async def v2_reformat_disk_POST(self, data: ReformatDisk) -> StorageResponseV2: self.locked_probe_data = True - self.reformat(self.model._one(id=data.disk_id), data.ptable) + await self.reformat(self.model._one(id=data.disk_id), data.ptable) return await self.v2_GET() async def v2_add_boot_partition_POST(self, disk_id: str) -> StorageResponseV2: @@ -1236,7 +1238,7 @@ async def v2_add_boot_partition_POST(self, disk_id: str) -> StorageResponseV2: raise StorageRecoverableError("device already has bootloader partition") if DeviceAction.TOGGLE_BOOT not in DeviceAction.supported(disk): raise StorageRecoverableError("disk does not support boot partiton") - self.add_boot_disk(disk) + await self.add_boot_disk(disk) return await self.v2_GET() async def v2_add_partition_POST(self, data: AddPartitionV2) -> StorageResponseV2: @@ -1259,7 +1261,7 @@ async def v2_add_partition_POST(self, data: AddPartitionV2) -> StorageResponseV2 } gap = gaps.at_offset(disk, data.gap.offset).split(requested_size)[0] - self.create_partition(disk, gap, spec, wipe="superblock") + await self.create_partition(disk, gap, spec, wipe="superblock") return await self.v2_GET() async def v2_delete_partition_POST( @@ -1269,7 +1271,7 @@ async def v2_delete_partition_POST( self.locked_probe_data = True disk = self.model._one(id=data.disk_id) partition = self.get_partition(disk, data.partition.number) - self.delete_partition(partition) + await self.delete_partition(partition) return await self.v2_GET() async def v2_edit_partition_POST( @@ -1295,7 +1297,7 @@ async def v2_edit_partition_POST( if data.partition.size is not None: spec["size"] = data.partition.size spec["wipe"] = data.partition.wipe - self.partition_disk_handler(disk, spec, partition=partition) + await self.partition_disk_handler(disk, spec, partition=partition) return await self.v2_GET() async def dry_run_wait_probe_POST(self) -> None: diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 39eb4215b..ec9cbc243 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -753,7 +753,7 @@ async def test_guided_zfs_swap_size_lp_2034939(self, suggested): async def _guided_side_by_side(self, bl, ptable): await self._guided_setup(bl, ptable, storage_version=2) - self.controller.add_boot_disk(self.d1) + await self.controller.add_boot_disk(self.d1) for p in self.d1._partitions: p.preserve = True if bl == Bootloader.UEFI: diff --git a/subiquity/ui/views/filesystem/delete.py b/subiquity/ui/views/filesystem/delete.py index e8e22c79c..c084dc324 100644 --- a/subiquity/ui/views/filesystem/delete.py +++ b/subiquity/ui/views/filesystem/delete.py @@ -19,6 +19,7 @@ from urwid import Text from subiquity.common.filesystem import labels +from subiquitycore.async_helpers import run_bg_task from subiquitycore.ui.buttons import danger_btn, other_btn from subiquitycore.ui.stretchy import Stretchy from subiquitycore.ui.table import TablePile, TableRow @@ -101,9 +102,12 @@ def __init__(self, parent, obj): super().__init__("", widgets, stretchy_index, len(lines) + 1) def confirm(self, sender=None): - self.parent.controller.delete(self.obj) - self.parent.refresh_model_inputs() - self.parent.remove_overlay() + async def async_confirm() -> None: + await self.parent.controller.delete(self.obj) + self.parent.refresh_model_inputs() + self.parent.remove_overlay() + + run_bg_task(async_confirm()) def cancel(self, sender=None): self.parent.remove_overlay() @@ -168,9 +172,12 @@ def __init__(self, parent, obj): super().__init__(title, widgets, 0, 2) def confirm(self, sender=None): - self.parent.controller.reformat(self.obj) - self.parent.refresh_model_inputs() - self.parent.remove_overlay() + async def async_confirm() -> None: + await self.parent.controller.reformat(self.obj) + self.parent.refresh_model_inputs() + self.parent.remove_overlay() + + run_bg_task(async_confirm()) def cancel(self, sender=None): self.parent.remove_overlay() diff --git a/subiquity/ui/views/filesystem/filesystem.py b/subiquity/ui/views/filesystem/filesystem.py index 341463996..2a34be5a1 100644 --- a/subiquity/ui/views/filesystem/filesystem.py +++ b/subiquity/ui/views/filesystem/filesystem.py @@ -19,6 +19,7 @@ configuration. """ +import asyncio import logging import attr @@ -27,6 +28,7 @@ from subiquity.common.filesystem import boot, gaps, labels from subiquity.common.filesystem.actions import DeviceAction from subiquity.models.filesystem import humanize_size +from subiquitycore.async_helpers import connect_async_signal, run_bg_task from subiquitycore.ui.actionmenu import Action, ActionMenu, ActionMenuOpenButton from subiquitycore.ui.buttons import back_btn, done_btn, menu_btn, other_btn, reset_btn from subiquitycore.ui.container import ListBox, WidgetWrap @@ -113,10 +115,10 @@ def __init__(self, parent): ) super().__init__(self.table) - def _mount_action(self, sender, action, mount): + async def _mount_action(self, mount, sender, action): log.debug("_mount_action %s %s", action, mount) if action == "unmount": - self.parent.controller.delete_mount(mount) + await self.parent.controller.delete_mount(mount) self.parent.refresh_model_inputs() def refresh_model_inputs(self): @@ -168,7 +170,9 @@ def refresh_model_inputs(self): ] actions = [(_("Unmount"), mi.mount.can_delete(), "unmount")] menu = ActionMenu(actions) - connect_signal(menu, "action", self._mount_action, mi.mount) + connect_async_signal( + menu, "action", self._mount_action, user_args=[mi.mount] + ) cells = [ Text("["), Text(path_markup), @@ -274,12 +278,13 @@ def _disk_REMOVE(self, disk): disk._constructed_device = None self.parent.refresh_model_inputs() - def _disk_TOGGLE_BOOT(self, disk): + async def _disk_TOGGLE_BOOT(self, disk): if boot.is_boot_device(disk): - self.parent.controller.remove_boot_disk(disk) + await self.parent.controller.remove_boot_disk(disk) + self.parent.refresh_model_inputs() else: - self.parent.controller.add_boot_disk(disk) - self.parent.refresh_model_inputs() + await self.parent.controller.add_boot_disk(disk) + self.parent.refresh_model_inputs() _partition_EDIT = _stretchy_shower( lambda parent, part: PartitionStretchy(parent, part.device, partition=part) @@ -309,7 +314,9 @@ def _disk_TOGGLE_BOOT(self, disk): def _action(self, sender, value, device): action, meth = value log.debug("_action %s %s", action, device.id) - meth(device) + maybe_coro = meth(device) + if asyncio.iscoroutine(maybe_coro): + run_bg_task(maybe_coro) def _label_REMOVE(self, action, device): cd = device.constructed_device() diff --git a/subiquity/ui/views/filesystem/lvm.py b/subiquity/ui/views/filesystem/lvm.py index d59949cb3..71ade8b09 100644 --- a/subiquity/ui/views/filesystem/lvm.py +++ b/subiquity/ui/views/filesystem/lvm.py @@ -26,6 +26,7 @@ MultiDeviceField, get_possible_components, ) +from subiquitycore.async_helpers import connect_async_signal from subiquitycore.ui.container import Pile from subiquitycore.ui.form import ( BooleanField, @@ -203,7 +204,7 @@ def __init__(self, parent, existing=None): self.form.devices.widget.set_supports_spares(False) connect_signal(form.devices.widget, "change", self._change_devices) - connect_signal(form, "submit", self.done) + connect_async_signal(form, "submit", self.done) connect_signal(form, "cancel", self.cancel) rows = form.as_rows() @@ -216,7 +217,7 @@ def _change_devices(self, sender, new_devices): else: self.form.size.value = "-" - def done(self, sender): + async def done(self, sender): result = self.form.as_data() del result["size"] mdc = self.form.devices.widget @@ -236,7 +237,7 @@ def done(self, sender): if "passphrase" in safe_result: safe_result["passphrase"] = "" log.debug("vg_done: {}".format(safe_result)) - self.parent.controller.volgroup_handler(self.existing, result) + await self.parent.controller.volgroup_handler(self.existing, result) self.parent.refresh_model_inputs() self.parent.remove_overlay() diff --git a/subiquity/ui/views/filesystem/partition.py b/subiquity/ui/views/filesystem/partition.py index 495c78a9b..7385dc176 100644 --- a/subiquity/ui/views/filesystem/partition.py +++ b/subiquity/ui/views/filesystem/partition.py @@ -40,6 +40,7 @@ common_mountpoints, suitable_mountpoints_for_existing_fs, ) +from subiquitycore.async_helpers import connect_async_signal, run_bg_task from subiquitycore.ui.container import Pile from subiquitycore.ui.form import ( BooleanField, @@ -539,7 +540,7 @@ def __init__(self, parent, disk, *, partition=None, gap=None): self.form.name.enabled = False self.form.size.enabled = False - connect_signal(self.form, "submit", self.done) + connect_async_signal(self.form, "submit", self.done) connect_signal(self.form, "cancel", self.cancel) rows = [] @@ -619,7 +620,7 @@ def __init__(self, parent, disk, *, partition=None, gap=None): def cancel(self, button=None): self.parent.remove_overlay() - def done(self, form): + async def done(self, form) -> None: log.debug("Add Partition Result: {}".format(form.as_data())) spec = form.as_data() if self.partition is not None and boot.is_esp(self.partition): @@ -633,9 +634,12 @@ def done(self, form): handler = self.controller.logical_volume_handler else: handler = self.controller.partition_disk_handler - handler(self.disk, spec, partition=self.partition, gap=self.gap) + + await handler(self.disk, spec, partition=self.partition, gap=self.gap) self.parent.refresh_model_inputs() self.parent.remove_overlay() + # This should probably be moved somewhere else + self.parent.request_redraw_if_visible() class FormatEntireStretchy(Stretchy): @@ -692,8 +696,13 @@ def __init__(self, parent, device): def cancel(self, button=None): self.parent.remove_overlay() - def done(self, form): + async def done(self, form): log.debug("Format Entire Result: {}".format(form.as_data())) - self.controller.add_format_handler(self.device, form.as_data()) - self.parent.refresh_model_inputs() - self.parent.remove_overlay() + + async def do_format() -> None: + await self.controller.add_format_handler(self.device, form.as_data()) + self.parent.refresh_model_inputs() + self.parent.remove_overlay() + self.parent.request_redraw_if_visible() + + run_bg_task(do_format()) diff --git a/subiquity/ui/views/filesystem/raid.py b/subiquity/ui/views/filesystem/raid.py index e42653935..097176091 100644 --- a/subiquity/ui/views/filesystem/raid.py +++ b/subiquity/ui/views/filesystem/raid.py @@ -29,6 +29,7 @@ MultiDeviceField, get_possible_components, ) +from subiquitycore.async_helpers import connect_async_signal from subiquitycore.ui.container import Pile from subiquitycore.ui.form import ( ChoiceField, @@ -162,7 +163,7 @@ def __init__(self, parent, existing=None): connect_signal(form.level.widget, "select", self._select_level) connect_signal(form.devices.widget, "change", self._change_devices) - connect_signal(form, "submit", self.done) + connect_async_signal(form, "submit", self.done) connect_signal(form, "cancel", self.cancel) rows = form.as_rows() @@ -190,13 +191,13 @@ def _change_devices(self, sender, new_devices): else: self.form.size.value = "-" - def done(self, sender): + async def done(self, sender): result = self.form.as_data() mdc = self.form.devices.widget result["devices"] = mdc.active_devices result["spare_devices"] = mdc.spare_devices log.debug("raid_done: result = {}".format(result)) - self.parent.controller.raid_handler(self.existing, result) + await self.parent.controller.raid_handler(self.existing, result) self.parent.refresh_model_inputs() self.parent.remove_overlay() diff --git a/subiquity/ui/views/filesystem/tests/test_lvm.py b/subiquity/ui/views/filesystem/tests/test_lvm.py index 30d4e81c5..9822197ec 100644 --- a/subiquity/ui/views/filesystem/tests/test_lvm.py +++ b/subiquity/ui/views/filesystem/tests/test_lvm.py @@ -13,6 +13,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import asyncio import os import pathlib import unittest @@ -39,6 +40,7 @@ def make_view(model, existing=None): return base_view, stretchy +@mock.patch("subiquitycore.async_helpers.run_bg_task", asyncio.run) class LVMViewTests(unittest.TestCase): def test_create_vg(self): model, disk = make_model_and_disk() diff --git a/subiquity/ui/views/filesystem/tests/test_partition.py b/subiquity/ui/views/filesystem/tests/test_partition.py index 8189b9a6d..0d9596864 100644 --- a/subiquity/ui/views/filesystem/tests/test_partition.py +++ b/subiquity/ui/views/filesystem/tests/test_partition.py @@ -1,3 +1,4 @@ +import asyncio import unittest from unittest import mock @@ -37,6 +38,8 @@ def make_format_entire_view(model, disk): return base_view, stretchy +@mock.patch("subiquitycore.async_helpers.run_bg_task", asyncio.run) +@mock.patch("subiquitycore.view.BaseView.request_redraw", mock.Mock()) class PartitionViewTests(unittest.TestCase): def test_initial_focus(self): model, disk = make_model_and_disk() @@ -181,6 +184,7 @@ def test_edit_boot_partition(self): view_helpers.enter_data(stretchy.form, form_data) stretchy.form.size.widget.lost_focus() + view_helpers.click(stretchy.form.done_btn.base_widget) expected_data = { "size": dehumanize_size(form_data["size"]), @@ -269,6 +273,7 @@ def test_create_partition_unaligned_size(self): view, stretchy = make_partition_view(model, disk, gap=gap) view_helpers.enter_data(stretchy.form, unaligned_data) stretchy.form.size.widget.lost_focus() + view_helpers.click(stretchy.form.done_btn.base_widget) view.controller.partition_disk_handler.assert_called_once_with( stretchy.disk, valid_data, partition=None, gap=gap diff --git a/subiquity/ui/views/filesystem/tests/test_raid.py b/subiquity/ui/views/filesystem/tests/test_raid.py index dc1e31fb3..940929081 100644 --- a/subiquity/ui/views/filesystem/tests/test_raid.py +++ b/subiquity/ui/views/filesystem/tests/test_raid.py @@ -1,3 +1,4 @@ +import asyncio import unittest from unittest import mock @@ -22,6 +23,7 @@ def make_view(model, existing=None): return base_view, stretchy +@mock.patch("subiquitycore.async_helpers.run_bg_task", asyncio.run) class RaidViewTests(unittest.TestCase): def test_create_raid(self): model, disk = make_model_and_disk()