diff --git a/documentation/autoinstall-reference.md b/documentation/autoinstall-reference.md index ea2b4076..8e82cf21 100644 --- a/documentation/autoinstall-reference.md +++ b/documentation/autoinstall-reference.md @@ -311,6 +311,24 @@ When using the "lvm" layout, LUKS encryption can be enabled by supplying a passw The default is to use the lvm layout. +#### sizing-policy + +The lvm layout will, by default, attempt to leave room for snapshots and further expansion. A sizing-policy key may be supplied to control this behavior. + +**type:** string (enumeration) +**default:** scaled + +Supported values are: + + * `scaled` -> adjust space allocated to the root LV based on space available to the VG + * `all` -> allocate all remaining VG space to the root LV + +The scaling system is currently as follows: + * Less than 10 GiB: use all remaining space for root filesystem + * Between 10-20 GiB: 10 GiB root filesystem + * Between 20-200 GiB: use half of remaining space for root filesystem + * Greater than 200 GiB: 100 GiB root filesystem + #### action-based config For full flexibility, the installer allows storage configuration to be done using a syntax which is a superset of that supported by curtin, described at https://curtin.readthedocs.io/en/latest/topics/storage.html. diff --git a/subiquity/common/filesystem/sizes.py b/subiquity/common/filesystem/sizes.py index 5bfdfe8d..dabb2881 100644 --- a/subiquity/common/filesystem/sizes.py +++ b/subiquity/common/filesystem/sizes.py @@ -149,3 +149,21 @@ def calculate_suggested_install_min(source_min: int, room_for_swap = swap.suggested_swapsize() total = source_min + room_for_boot + room_to_grow + room_for_swap return align_up(total, part_align) + + +# Scale the usage of the vg to leave room for snapshots and such. We should +# use more of a smaller disk to avoid the user running into out of space errors +# earlier than they probably expect to. +def scaled_rootfs_size(available: int): + if available < 10 * (1 << 30): + # Use all of a small (<10G) disk. + return available + elif available < 20 * (1 << 30): + # Use 10G of a smallish (<20G) disk. + return 10 * (1 << 30) + elif available < 200 * (1 << 30): + # Use half of a larger (<200G) disk. + return available // 2 + else: + # Use at most 100G of a large disk. + return 100 * (1 << 30) diff --git a/subiquity/common/tests/test_types.py b/subiquity/common/tests/test_types.py new file mode 100644 index 00000000..cc59e4fc --- /dev/null +++ b/subiquity/common/tests/test_types.py @@ -0,0 +1,32 @@ +# Copyright 2023 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +import unittest + +from subiquity.common.types import SizingPolicy + + +class TestSizingPolicy(unittest.TestCase): + def test_all(self): + actual = SizingPolicy.from_string('all') + self.assertEqual(SizingPolicy.ALL, actual) + + def test_scaled_size(self): + actual = SizingPolicy.from_string('scaled') + self.assertEqual(SizingPolicy.SCALED, actual) + + def test_default(self): + actual = SizingPolicy.from_string(None) + self.assertEqual(SizingPolicy.SCALED, actual) diff --git a/subiquity/common/types.py b/subiquity/common/types.py index 2bffefa2..90c12d81 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -386,6 +386,19 @@ class StorageResponseV2: install_minimum_size: Optional[int] = None +class SizingPolicy(enum.Enum): + SCALED = enum.auto() + ALL = enum.auto() + + @classmethod + def from_string(cls, value): + if value is None or value == 'scaled': + return cls.SCALED + if value == 'all': + return cls.ALL + raise Exception(f'Unknown SizingPolicy value {value}') + + @attr.s(auto_attribs=True) class GuidedResizeValues: install_max: int @@ -436,6 +449,8 @@ class GuidedChoiceV2: target: GuidedStorageTarget use_lvm: bool = False password: Optional[str] = attr.ib(default=None, repr=False) + sizing_policy: Optional[SizingPolicy] = \ + attr.ib(default=SizingPolicy.SCALED) @staticmethod def from_guided_choice(choice: GuidedChoice): @@ -443,6 +458,7 @@ class GuidedChoiceV2: target=GuidedStorageTargetReformat(disk_id=choice.disk_id), use_lvm=choice.use_lvm, password=choice.password, + sizing_policy=SizingPolicy.SCALED, ) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index bff3d2c5..1c98aced 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -70,6 +70,7 @@ from subiquity.common.types import ( ProbeStatus, ReformatDisk, StorageEncryptionSupport, + SizingPolicy, StorageResponse, StorageResponseV2, StorageSafety, @@ -270,7 +271,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): spec = dict(fstype="ext4", mount="/") self.create_partition(device=gap.device, gap=gap, spec=spec) - def guided_lvm(self, gap, lvm_options=None): + 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) @@ -285,26 +286,17 @@ class FilesystemController(SubiquityController, FilesystemManipulator): i += 1 vg_name = 'ubuntu-vg-{}'.format(i) spec = dict(name=vg_name, devices=set([part])) - if lvm_options and lvm_options['encrypt']: - spec['passphrase'] = lvm_options['luks_options']['passphrase'] + if choice.password is not None: + spec['passphrase'] = choice.password vg = self.create_volgroup(spec) - # There's no point using LVM and unconditionally filling the - # VG with a single LV, but we should use more of a smaller - # disk to avoid the user running into out of space errors - # earlier than they probably expect to. - if vg.size < 10 * (1 << 30): - # Use all of a small (<10G) disk. + if choice.sizing_policy == SizingPolicy.SCALED: + lv_size = sizes.scaled_rootfs_size(vg.size) + lv_size = align_down(lv_size, LVM_CHUNK_SIZE) + elif choice.sizing_policy == SizingPolicy.ALL: lv_size = vg.size - elif vg.size < 20 * (1 << 30): - # Use 10G of a smallish (<20G) disk. - lv_size = 10 * (1 << 30) - elif vg.size < 200 * (1 << 30): - # Use half of a larger (<200G) disk. - lv_size = vg.size // 2 else: - # Use at most 100G of a large disk. - lv_size = 100 * (1 << 30) - lv_size = align_down(lv_size, LVM_CHUNK_SIZE) + raise Exception(f'Unhandled size policy {choice.sizing_policy}') + log.debug(f'lv_size {lv_size} for {choice.sizing_policy}') self.create_logical_volume( vg=vg, spec=dict( size=lv_size, @@ -354,17 +346,6 @@ class FilesystemController(SubiquityController, FilesystemManipulator): raise Exception(f'gap not found after resize, pgs={pgs}') return gap - def build_lvm_options(self, passphrase): - if passphrase is None: - return None - else: - return { - 'encrypt': True, - 'luks_options': { - 'passphrase': passphrase, - }, - } - def guided(self, choice: GuidedChoiceV2): self.model.guided_configuration = choice @@ -378,8 +359,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): raise Exception('failed to locate gap after adding boot') if choice.use_lvm: - lvm_options = self.build_lvm_options(choice.password) - self.guided_lvm(gap, lvm_options=lvm_options) + self.guided_lvm(gap, choice) else: self.guided_direct(gap) @@ -917,8 +897,11 @@ class FilesystemController(SubiquityController, FilesystemManipulator): f'using {target}') use_lvm = name == 'lvm' password = layout.get('password', None) - self.guided(GuidedChoiceV2(target=target, use_lvm=use_lvm, - password=password)) + sizing_policy = SizingPolicy.from_string( + layout.get('sizing-policy', None)) + self.guided( + GuidedChoiceV2(target=target, use_lvm=use_lvm, + password=password, sizing_policy=sizing_policy)) def validate_layout_mode(self, mode): if mode not in ('reformat_disk', 'use_gap'): diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 793fa3f2..b9d311d8 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -21,6 +21,7 @@ from subiquitycore.tests.parameterized import parameterized from subiquitycore.snapd import AsyncSnapd, get_fake_connection from subiquitycore.tests.mocks import make_app +from subiquitycore.utils import matching_dicts from subiquitycore.tests.util import random_string from subiquity.common.filesystem import gaps @@ -31,7 +32,9 @@ from subiquity.common.types import ( GuidedStorageTargetResize, GuidedStorageTargetUseGap, ProbeStatus, + SizingPolicy, ) +from subiquity.models.filesystem import dehumanize_size from subiquity.models.tests.test_filesystem import ( make_disk, make_model, @@ -462,6 +465,45 @@ class TestGuidedV2(IsolatedAsyncioTestCase): disk_size - (1 << 20), parts[-1].offset + parts[-1].size, disk_size) + async def _sizing_setup(self, bootloader, ptable, disk_size, policy): + self._setup(bootloader, ptable, size=disk_size) + + resp = await self.fsc.v2_guided_GET() + reformat = [target for target in resp.possible + if isinstance(target, GuidedStorageTargetReformat)][0] + data = GuidedChoiceV2(target=reformat, + use_lvm=True, + sizing_policy=policy) + await self.fsc.v2_guided_POST(data=data) + resp = await self.fsc.GET() + + [vg] = matching_dicts(resp.config, type='lvm_volgroup') + [part_id] = vg['devices'] + [part] = matching_dicts(resp.config, id=part_id) + part_size = part['size'] # already an int + [lvm_partition] = matching_dicts(resp.config, type='lvm_partition') + size = dehumanize_size(lvm_partition['size']) + return size, part_size + + @parameterized.expand(bootloaders_and_ptables) + async def test_scaled_disk(self, bootloader, ptable): + size, part_size = await self._sizing_setup( + bootloader, ptable, 50 << 30, SizingPolicy.SCALED) + # expected to be about half, differing by boot and ptable types + self.assertLess(20 << 30, size) + self.assertLess(size, 30 << 30) + + @parameterized.expand(bootloaders_and_ptables) + async def test_unscaled_disk(self, bootloader, ptable): + size, part_size = await self._sizing_setup( + bootloader, ptable, 50 << 30, SizingPolicy.ALL) + # there is some subtle differences in sizing depending on + # ptable/bootloader and how the rounding goes + self.assertLess(part_size - (5 << 20), size) + self.assertLess(size, part_size) + # but we should using most of the disk, minus boot partition(s) + self.assertLess(45 << 30, size) + class TestManualBoot(IsolatedAsyncioTestCase): def _setup(self, bootloader, ptable, **kw): diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index a71bb3ed..48127115 100644 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -28,7 +28,10 @@ from unittest.mock import patch from urllib.parse import unquote from subiquitycore.tests import SubiTestCase -from subiquitycore.utils import astart_command +from subiquitycore.utils import ( + astart_command, + matching_dicts, + ) default_timeout = 10 @@ -37,8 +40,7 @@ def match(items, **kw): typename = kw.pop('_type', None) if typename is not None: kw['$type'] = typename - return [item for item in items - if all(item.get(k) == v for k, v in kw.items())] + return matching_dicts(items, **kw) def timeout(multiplier=1): diff --git a/subiquitycore/utils.py b/subiquitycore/utils.py index 40307c81..648c61d5 100644 --- a/subiquitycore/utils.py +++ b/subiquitycore/utils.py @@ -19,7 +19,7 @@ import logging import os import random import subprocess -from typing import List, Sequence +from typing import Any, Dict, List, Sequence log = logging.getLogger("subiquitycore.utils") @@ -176,3 +176,10 @@ def disable_subiquity(): "snap.subiquity.subiquity-service.service", "serial-subiquity@*.service"]) return + + +def matching_dicts(items: Sequence[Dict[Any, Any]], **criteria): + """Given an input sequence of dictionaries, return a list of dicts where + the supplied keyword arguments all match those items.""" + return [item for item in items + if all(k in item and item[k] == v for k, v in criteria.items())]