From cabb8dda8e682b04f9fa25a2833dbd53b6f73cfe Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Tue, 1 Nov 2022 13:48:32 +0100 Subject: [PATCH] add the ability to only generate a subset of curtin fs actions DEVICES == everything up to and including partitioning FORMAT_MOUNT == formatting and mounting --- subiquity/models/filesystem.py | 51 +++++++++++++++++++--- subiquity/models/tests/test_filesystem.py | 50 +++++++++++++++++++++ subiquity/server/controllers/filesystem.py | 3 +- 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 785e9031..4ee50b74 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -17,6 +17,7 @@ from abc import ABC, abstractmethod import attr import collections import copy +import enum import fnmatch import itertools import logging @@ -1025,6 +1026,25 @@ class PartitionAlignmentData: ebr_space: int = 0 +class ActionRenderMode(enum.Enum): + # The default for FilesystemModel.render() is to render actions + # for devices that have changes, but not e.g. a hard drive that + # will be untouched by the installation process. + DEFAULT = enum.auto() + # ALL means render actions for all model objects. This is used to + # send information to the client. + ALL = enum.auto() + # DEVICES means to just render actions for setting up block + # devices, e.g. partitioning disks and assembling RAIDs but not + # any format or mount actions. + DEVICES = enum.auto() + # FORMAT_MOUNT means to just render actions to format and mount + # the block devices. References to block devices will be replaced + # by "type: device" actions that just refer to the block devices + # by path. + FORMAT_MOUNT = enum.auto() + + class FilesystemModel(object): target = None @@ -1371,7 +1391,8 @@ class FilesystemModel(object): return objs - def _render_actions(self, include_all=False): + def _render_actions(self, + mode: ActionRenderMode = ActionRenderMode.DEFAULT): # The curtin storage config has the constraint that an action must be # preceded by all the things that it depends on. We handle this by # repeatedly iterating over all actions and checking if we can emit @@ -1425,9 +1446,11 @@ class FilesystemModel(object): mountpoints = {m.path: m.id for m in self.all_mounts()} log.debug('mountpoints %s', mountpoints) - work = [ - a for a in self._actions - if not getattr(a, 'preserve', False) or include_all + if mode == ActionRenderMode.ALL: + work = list(self._actions) + else: + work = [ + a for a in self._actions if not getattr(a, 'preserve', False) ] while work: @@ -1444,13 +1467,29 @@ class FilesystemModel(object): raise Exception("\n".join(msg)) work = next_work + if mode == ActionRenderMode.DEVICES: + r = [act for act in r if act['type'] not in ('format', 'mount')] + if mode == ActionRenderMode.FORMAT_MOUNT: + r = [act for act in r if act['type'] in ('format', 'mount')] + devices = [] + for act in r: + if act['type'] == 'format': + device = { + 'type': 'device', + 'id': 'synth-device-{}'.format(len(devices)), + 'path': self._one(id=act['volume']).path, + } + devices.append(device) + act['volume'] = device['id'] + r = devices + r + return r - def render(self): + def render(self, mode: ActionRenderMode = ActionRenderMode.DEFAULT): config = { 'storage': { 'version': self.storage_version, - 'config': self._render_actions(), + 'config': self._render_actions(mode=mode), }, } if self.swap is not None: diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index b45b1800..6278ec5e 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -20,6 +20,7 @@ import attr from parameterized import parameterized from subiquity.models.filesystem import ( + ActionRenderMode, Bootloader, dehumanize_size, Disk, @@ -932,6 +933,55 @@ class TestAutoInstallConfig(unittest.TestCase): self.assertTrue(disk2.id not in rendered_ids) self.assertTrue(disk2p1.id not in rendered_ids) + def test_render_all_does_include_unreferenced(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk2 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + disk2p1 = make_partition(model, disk2, preserve=True) + fs = model.add_filesystem(disk1p1, 'ext4') + model.add_mount(fs, '/') + rendered_ids = { + action['id'] + for action in model._render_actions(ActionRenderMode.ALL) + } + self.assertTrue(disk1.id in rendered_ids) + self.assertTrue(disk1p1.id in rendered_ids) + self.assertTrue(disk2.id in rendered_ids) + self.assertTrue(disk2p1.id in rendered_ids) + + def test_render_devices_skips_format_mount(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + fs = model.add_filesystem(disk1p1, 'ext4') + mnt = model.add_mount(fs, '/') + rendered_ids = { + action['id'] + for action in model._render_actions(ActionRenderMode.DEVICES) + } + self.assertTrue(disk1.id in rendered_ids) + self.assertTrue(disk1p1.id in rendered_ids) + self.assertTrue(fs.id not in rendered_ids) + self.assertTrue(mnt.id not in rendered_ids) + + def test_render_format_mount(self): + model = make_model(Bootloader.NONE) + disk1 = make_disk(model, preserve=True) + disk1p1 = make_partition(model, disk1, preserve=True) + disk1p1.path = '/dev/vda1' + fs = model.add_filesystem(disk1p1, 'ext4') + mnt = model.add_mount(fs, '/') + actions = model._render_actions(ActionRenderMode.FORMAT_MOUNT) + rendered_by_id = {action['id']: action for action in actions} + self.assertTrue(disk1.id not in rendered_by_id) + self.assertTrue(disk1p1.id not in rendered_by_id) + self.assertTrue(fs.id in rendered_by_id) + self.assertTrue(mnt.id in rendered_by_id) + vol_id = rendered_by_id[fs.id]['volume'] + self.assertEqual(rendered_by_id[vol_id]['type'], 'device') + self.assertEqual(rendered_by_id[vol_id]['path'], '/dev/vda1') + def test_render_includes_all_partitions(self): model = make_model(Bootloader.NONE) disk1 = make_disk(model, preserve=True) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 4e2fbbdb..7e3b814f 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -73,6 +73,7 @@ from subiquity.common.types import ( StorageResponseV2, ) from subiquity.models.filesystem import ( + ActionRenderMode, align_up, align_down, _Device, @@ -370,7 +371,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): bootloader=self.model.bootloader, error_report=self.full_probe_error(), orig_config=self.model._orig_config, - config=self.model._render_actions(include_all=True), + config=self.model._render_actions(mode=ActionRenderMode.ALL), blockdev=self.model._probe_data['blockdev'], dasd=self.model._probe_data.get('dasd', {}), storage_version=self.model.storage_version)