Merge pull request #1794 from dbungert/lunar-security-pocket

lunar: security archive

.github/workflows/build.yaml

@@ -20,7 +20,7 @@ jobs:
   lint:
     runs-on: ubuntu-20.04
     strategy:
-      fail-fast: true
+      fail-fast: false
       matrix:
         image:
           - ubuntu-daily:jammy    # match the core snap we're running against

console_conf/core.py

@@ -54,8 +54,8 @@ class RecoveryChooser(TuiApplication):
     ]
 
     def __init__(self, opts, chooser_input, chooser_output):
-        """Takes the options and raw input/output streams for communicating with the
+        """Takes the options and raw input/output streams for communicating
-        chooser parent process.
+        with the chooser parent process.
         """
         self._chooser_output = chooser_output
         # make_model is used by super()'s constructor, but we need to use the

console_conf/ui/views/chooser.py

@@ -58,8 +58,8 @@ class ChooserBaseView(BaseView):
 
 
 def by_preferred_action_type(action):
-    """Order action entries by having the 'run' mode first, then 'recover', then
+    """Order action entries by having the 'run' mode first, then 'recover',
-    'install', the rest is ordered alphabetically."""
+    then 'install', the rest is ordered alphabetically."""
     priority = {"run": 0, "recover": 1, "install": 2}
     return (priority.get(action.mode, 100), action.title.lower())
 

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.

examples/desktop-sources.yaml

@@ -0,0 +1,41 @@
+- 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
+  preinstalled_langs:
+  - de
+  - en
+  - es
+  - fr
+  - it
+  - pt
+  - ru
+  - zh
+  - ''
+  size: 7604871168
+  type: fsimage-layered
+  variant: desktop

scripts/runtests.sh

@@ -41,6 +41,18 @@ validate () {
             echo "password leaked into log file"
             exit 1
         fi
+        # After the lunar release and the introduction of mirror testing, it
+        # came to our attention that new Ubuntu installations have the security
+        # repository configured with the primary mirror URL (i.e.,
+        # http://<cc>.archive.ubuntu.com/ubuntu) instead of
+        # http://security.ubuntu.com/ubuntu. Let's ensure we instruct curtin
+        # not to do that.
+        # If we run an autoinstall that customizes the security section as part
+        # of the test-suite, we will need to adapt this test.
+        python3 scripts/check-yaml-fields.py $tmpdir/var/log/installer/subiquity-curtin-apt.conf \
+            apt.security[0].uri='"http://security.ubuntu.com/ubuntu/"' \
+            apt.security[0].arches='["amd64", "i386"]' \
+            apt.security[1].uri='"http://ports.ubuntu.com/ubuntu-ports"'
         netplan generate --root $tmpdir
     elif [ "${mode}" = "system_setup" ]; then
         setup_mode="$2"

scripts/slimy-update-snap.sh

@@ -35,7 +35,9 @@ add_overlay() {
         local upper="$(mktemp -dp "${tmpdir}")"
     fi
     chmod go+rx "${work}" "${upper}"
-    do_mount -t overlay overlay -o lowerdir="${lower}",upperdir="${upper}",workdir="${work}" "${mountpoint}"
+    do_mount -t overlay overlay \
+        -o lowerdir="${lower}",upperdir="${upper}",workdir="${work}" \
+        "${mountpoint}"
 }
 
 
@@ -53,11 +55,22 @@ do_mount $old old
 add_overlay old new
 
 rm -rf new/lib/python3.10/site-packages/curtin
-rm -rf new/lib/python3.10/site-packages/subiquity
+
-rm -rf new/lib/python3.10/site-packages/subiquitycore
+if [ -d new/lib/python3.10/site-packages/subiquity ] ; then
+    subiquity_dest=new/lib/python3.10/site-packages
+elif [ -d new/bin/subiquity/subiquity ] ; then
+    subiquity_dest=new/bin/subiquity
+else
+    echo "unrecognized snap" >&2
+    exit 1
+fi
+
+rm -rf "${subiquity_dest}/subiquity"
+rm -rf "${subiquity_dest}/subiquitycore"
 
 (cd "${src}" && ./scripts/update-part.py curtin)
 
-rsync -a --chown 0:0 $src/subiquity $src/subiquitycore $src/curtin/curtin new/lib/python3.10/site-packages
+rsync -a --chown 0:0 $src/curtin/curtin new/lib/python3.10/site-packages
+rsync -a --chown 0:0 $src/subiquity $src/subiquitycore $subiquity_dest
 
 snapcraft pack new --output $new

snapcraft.yaml

@@ -61,7 +61,8 @@ parts:
 
     source: https://git.launchpad.net/curtin
     source-type: git
-    source-commit: b1f4da3bec92356e8ef389c1c581cfdcd1b36c42
+    # We're traking the branch ubuntu/lunar here.
+    source-commit: 9e9f66e835c79e00d17e61593cc01a66d055c2e9
 
     override-pull: |
       craftctl default
@@ -84,6 +85,15 @@ parts:
     organize:
       lib/python*/site-packages/usr/lib/curtin: usr/lib/
 
+  ubuntu-wsl-setup:
+    plugin: nil
+    source: .
+    source-type: git
+
+    override-build: |
+      mkdir -p $CRAFT_PART_INSTALL/bin
+      cp system_setup/ubuntu-wsl-setup $CRAFT_PART_INSTALL/bin/ubuntu-wsl-setup
+
   subiquity:
     plugin: nil
 
@@ -134,7 +144,6 @@ parts:
       bin/subiquity-service: usr/bin/subiquity-service
       bin/subiquity-server: usr/bin/subiquity-server
       bin/subiquity-cmd: usr/bin/subiquity-cmd
-      $CRAFT_PART_BUILD/system_setup/ubuntu-wsl-setup: bin/ubuntu-wsl-setup
 
     build-attributes:
       - enable-patchelf
@@ -207,7 +216,7 @@ parts:
 
     source: https://github.com/canonical/probert.git
     source-type: git
-    source-commit: 40479d08fce0370a0c41a140e6d322d2846c2a8f
+    source-commit: dacf369e3dedc50018e4b3b86d4d919459da3cc6
 
     override-build: *pyinstall
 

subiquity/client/controllers/filesystem.py

@@ -241,7 +241,7 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator):
                     clean_suffix='vg'):
                 pass
         elif action['action'] == 'done':
-            if not self.ui.body.done.enabled:
+            if not self.ui.body.done_btn.enabled:
                 raise Exception("answers did not provide complete fs config")
             await self.app.confirm_install()
             self.finish()

subiquity/cmd/server.py

@@ -159,10 +159,11 @@ def main():
 
     logger = logging.getLogger('subiquity')
     version = os.environ.get("SNAP_REVISION", "unknown")
-    logger.info("Starting Subiquity server revision {}".format(version))
+    snap = os.environ.get("SNAP", "unknown")
-    logger.info("Arguments passed: {}".format(sys.argv))
+    logger.info(f"Starting Subiquity server revision {version} of snap {snap}")
-    logger.debug("Kernel commandline: {}".format(opts.kernel_cmdline))
+    logger.info(f"Arguments passed: {sys.argv}")
-    logger.debug("Storage version: {}".format(opts.storage_version))
+    logger.debug(f"Kernel commandline: {opts.kernel_cmdline}")
+    logger.debug(f"Environment: {os.environ}")
 
     async def run_with_loop():
         server = SubiquityServer(opts, block_log_dir)
@@ -171,6 +172,10 @@ def main():
             "InstallerServerLog", logfiles['debug'])
         server.note_file_for_apport(
             "InstallerServerLogInfo", logfiles['info'])
+        server.note_file_for_apport(
+            "UdiLog",
+            os.path.realpath(
+                "/var/log/installer/ubuntu_desktop_installer.log"))
         await server.run()
 
     asyncio.run(run_with_loop())

subiquity/cmd/tui.py

@@ -113,8 +113,10 @@ def main():
 
     logger = logging.getLogger('subiquity')
     version = os.environ.get("SNAP_REVISION", "unknown")
-    logger.info("Starting Subiquity revision {}".format(version))
+    snap = os.environ.get("SNAP", "unknown")
-    logger.info("Arguments passed: {}".format(sys.argv))
+    logger.info(f"Starting Subiquity TUI revision {version} of snap {snap}")
+    logger.info(f"Arguments passed: {sys.argv}")
+    logger.debug(f"Environment: {os.environ}")
 
     if opts.answers is None and os.path.exists(AUTO_ANSWERS_FILE):
         logger.debug("Autoloading answers from %s", AUTO_ANSWERS_FILE)

subiquity/common/api/server.py

@@ -60,6 +60,27 @@ def trim(text):
         return text
 
 
+async def check_controllers_started(definition, controller, request):
+    if not hasattr(controller, 'app'):
+        return
+
+    if getattr(definition, 'allowed_before_start', False):
+        return
+
+    # Most endpoints should not be responding to requests
+    # before the controllers have started, that's just bound to
+    # be a big pool of timing bugs that we want nothing to do
+    # with.  A few chosen methods should be safe, so allow
+    # those to opt-in.  Everybody else blocks until the
+    # controllers complete their start().
+    if controller.app.controllers_have_started.is_set():
+        return
+
+    log.debug(f'{request.path} waiting on start')
+    await controller.app.controllers_have_started.wait()
+    log.debug(f'{request.path} resuming')
+
+
 def _make_handler(controller, definition, implementation, serializer,
                   serialize_query_args):
     def_sig = inspect.signature(definition)
@@ -134,18 +155,8 @@ def _make_handler(controller, definition, implementation, serializer,
                     args['context'] = context
                 if 'request' in impl_params:
                     args['request'] = request
-                if not getattr(
+                await check_controllers_started(
-                        definition, 'allowed_before_start', False):
+                        definition, controller, request)
-                    # Most endpoints should not be responding to requests
-                    # before the controllers have started, that's just bound to
-                    # be a big pool of timing bugs that we want nothing to do
-                    # with.  A few chosen methods should be safe, so allow
-                    # those to opt-in.  Everybody else blocks until the
-                    # controllers complete their start().
-                    if not controller.app.controllers_have_started.is_set():
-                        log.debug(f'{request.path} waiting on start')
-                        await controller.app.controllers_have_started.wait()
-                        log.debug(f'{request.path} resuming')
                 result = await implementation(**args)
                 resp = web.json_response(
                     serializer.serialize(def_ret_ann, result),

subiquity/common/apidef.py

@@ -277,6 +277,10 @@ class API:
 
         def POST(config: Payload[list]): ...
 
+        class dry_run_wait_probe:
+            """This endpoint only works in dry-run mode."""
+            def POST() -> None: ...
+
         class reset:
             def POST() -> StorageResponse: ...
 
@@ -302,20 +306,24 @@ class API:
             class reset:
                 def POST() -> StorageResponseV2: ...
 
+            class ensure_transaction:
+                """This call will ensure that a transaction is initiated.
+                During a transaction, storage probing runs are not permitted to
+                reset the partitioning configuration.
+                A transaction will also be initiated by any v2_storage POST
+                request that modifies the partitioning configuration (e.g.,
+                add_partition, edit_partition, ...) but ensure_transaction can
+                be called early if desired. """
+                def POST() -> None: ...
+
             class reformat_disk:
                 def POST(data: Payload[ReformatDisk]) \
                     -> StorageResponseV2: ...
 
-            class potential_boot_disks:
-                """Obtain the list of disks which can be made bootable.
-                This list can be empty if there are no disks or if none of them
-                are plausible sources of a boot disk."""
-                def GET() -> List[str]: ...
-
             class add_boot_partition:
                 """Mark a given disk as bootable, which may cause a partition
                 to be added to the disk.  It is an error to call this for a
-                disk not in the list from potential_boot_disks."""
+                disk for which can_be_boot_device is False."""
                 def POST(disk_id: str) -> StorageResponseV2: ...
 
             class add_partition:

subiquity/common/errorreport.py

@@ -18,9 +18,11 @@ import fcntl
 import json
 import logging
 import os
+import re
 import sys
 import time
 import traceback
+from typing import Iterable, Set
 
 import apport
 import apport.crashdb
@@ -138,6 +140,10 @@ class ErrorReport(metaclass=urwid.MetaSignals):
             if not self.reporter.dry_run:
                 self.pr.add_hooks_info(None)
                 apport.hookutils.attach_hardware(self.pr)
+            self.pr['Syslog'] = apport.hookutils.recent_syslog(re.compile('.'))
+            snap_name = os.environ.get('SNAP_NAME', '')
+            if snap_name != '':
+                self.add_tags([snap_name])
             # Because apport-cli will in general be run on a different
             # machine, we make some slightly obscure alterations to the report
             # to make this go better.
@@ -327,6 +333,20 @@ class ErrorReport(metaclass=urwid.MetaSignals):
             oops_id=self.oops_id,
             )
 
+    # with core24 these tag methods can be dropped for equivalent methods
+    # that will be on the report object
+    def get_tags(self) -> Set[str]:
+        """Return the set of tags."""
+        if "Tags" not in self.pr:
+            return set()
+        return set(self.pr["Tags"].split(" "))
+
+    def add_tags(self, tags: Iterable[str]) -> None:
+        """Add tags to the report. Duplicates are dropped."""
+        current_tags = self.get_tags()
+        new_tags = current_tags.union(tags)
+        self.pr["Tags"] = " ".join(sorted(new_tags))
+
 
 class ErrorReporter(object):
 

subiquity/common/filesystem/gaps.py

@@ -149,6 +149,9 @@ def find_disk_gaps_v2(device, info=None):
     for part in parts + [None]:
         if part is None:
             gap_end = ad(device.size - info.min_end_offset)
+        else:
+            if part.is_logical:
+                gap_end = ad(part.offset - info.ebr_space)
             else:
                 gap_end = ad(part.offset)
 

subiquity/common/filesystem/labels.py

@@ -306,6 +306,7 @@ def _for_client_disk(disk, *, min_size=0):
         usage_labels=usage_labels(disk),
         partitions=[for_client(p) for p in gaps.parts_and_gaps(disk)],
         boot_device=boot.is_boot_device(disk),
+        can_be_boot_device=boot.can_be_boot_device(disk),
         ok_for_guided=disk.size >= min_size,
         model=getattr(disk, 'model', None),
         vendor=getattr(disk, 'vendor', None))

subiquity/common/filesystem/manipulator.py

@@ -68,8 +68,9 @@ class FilesystemManipulator:
                 volume.flag = ""
         if spec.get('fstype') == "swap":
             self.model.add_mount(fs, "")
-        if spec.get('fstype') is None and spec.get('use_swap'):
+        elif spec.get('fstype') is None and spec.get('use_swap'):
             self.model.add_mount(fs, "")
+        else:
             self.create_mount(fs, spec)
         return fs
 

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)

subiquity/common/filesystem/tests/test_gaps.py

@@ -333,6 +333,32 @@ class TestDiskGaps(unittest.TestCase):
                 gaps.Gap(d, 50, 50, False),
             ])
 
+    def test_gap_before_primary(self):
+        # 0----10---20---30---40---50---60---70---80---90---100
+        # [        g1             ][    p1 (primary)          ]
+        info = PartitionAlignmentData(
+            part_align=5, min_gap_size=1, min_start_offset=0, min_end_offset=0,
+            ebr_space=1, primary_part_limit=10)
+        m, d = make_model_and_disk(size=100, ptable='dos')
+        p1 = make_partition(m, d, offset=50, size=50)
+        self.assertEqual(
+            gaps.find_disk_gaps_v2(d, info),
+            [gaps.Gap(d, 0, 50, False), p1])
+
+    def test_gap_in_extended_before_logical(self):
+        # 0----10---20---30---40---50---60---70---80---90---100
+        # [      p1 (extended)                                ]
+        #   [      g1            ] [    p5 (logical)          ]
+        info = PartitionAlignmentData(
+            part_align=5, min_gap_size=1, min_start_offset=0, min_end_offset=0,
+            ebr_space=1, primary_part_limit=10)
+        m, d = make_model_and_disk(size=100, ptable='dos')
+        p1 = make_partition(m, d, offset=0, size=100, flag='extended')
+        p5 = make_partition(m, d, offset=50, size=50, flag='logical')
+        self.assertEqual(
+            gaps.find_disk_gaps_v2(d, info),
+            [p1, gaps.Gap(d, 5, 40, True), p5])
+
     def test_unusable_gap_primaries(self):
         info = PartitionAlignmentData(
             part_align=10, min_gap_size=1, min_start_offset=0,

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 <http://www.gnu.org/licenses/>.
+
+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)

subiquity/common/types.py

@@ -158,6 +158,12 @@ AnyStep = Union[StepPressKey, StepKeyPresent, StepResult]
 
 @attr.s(auto_attribs=True)
 class KeyboardSetting:
+    # This data structure represents a subset of the XKB options.
+    # As explained in the XKB configuration guide, XkbLayout and
+    # XkbVariant can hold multiple comma-separated values.
+    # http://www.xfree86.org/current/XKB-Config2.html#4
+    # Ideally, we would internally represent a keyboard setting as a
+    # toggle + a list of [layout, variant].
     layout: str
     variant: str = ''
     toggle: Optional[str] = None
@@ -311,6 +317,7 @@ class Disk:
     preserve: bool
     path: Optional[str]
     boot_device: bool
+    can_be_boot_device: bool
     model: Optional[str] = None
     vendor: Optional[str] = None
 
@@ -386,6 +393,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 +456,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 +465,7 @@ class GuidedChoiceV2:
                 target=GuidedStorageTargetReformat(disk_id=choice.disk_id),
                 use_lvm=choice.use_lvm,
                 password=choice.password,
+                sizing_policy=SizingPolicy.SCALED,
                 )
 
 

subiquity/models/filesystem.py

@@ -820,16 +820,22 @@ class Raid(_Device):
     wipe = attr.ib(default=None)
     ptable = attributes.ptable()
     metadata = attr.ib(default=None)
-    path = attr.ib(default=None)
+    _path = attr.ib(default=None)
     container = attributes.ref(backlink="_subvolumes", default=None)  # Raid
     _subvolumes = attributes.backlink(default=attr.Factory(list))
 
     @property
     def path(self):
+        if self._path is not None:
+            return self._path
         # This is just here to make for_client(raid-with-partitions) work. It
         # might not be very accurate.
         return '/dev/md/' + self.name
 
+    @path.setter
+    def path(self, value):
+        self._path = value
+
     @property
     def size(self):
         if self.preserve and self._m._probe_data:

subiquity/models/keyboard.py

@@ -43,6 +43,15 @@ BACKSPACE="guess"
 """
 
 
+class InconsistentMultiLayoutError(ValueError):
+    """ Exception to raise when a multi layout has a different number of
+    layouts and variants. """
+    def __init__(self, layouts: str, variants: str) -> None:
+        super().__init__(
+                f'inconsistent multi-layout: layouts="{layouts}"'
+                f' variants="{variants}"')
+
+
 def from_config_file(config_file):
     with open(config_file) as fp:
         content = fp.read()
@@ -91,13 +100,21 @@ class KeyboardModel:
         self._setting = value
 
     def validate_setting(self, setting: KeyboardSetting) -> None:
-        kbd_layout = self.keyboard_list.layout_map.get(setting.layout)
+        layout_tokens = setting.layout.split(",")
+        variant_tokens = setting.variant.split(",")
+
+        if len(layout_tokens) != len(variant_tokens):
+            raise InconsistentMultiLayoutError(
+                    layouts=setting.layout, variants=setting.variant)
+
+        for layout, variant in zip(layout_tokens, variant_tokens):
+            kbd_layout = self.keyboard_list.layout_map.get(layout)
             if kbd_layout is None:
-            raise ValueError(f'Unknown keyboard layout "{setting.layout}"')
+                raise ValueError(f'Unknown keyboard layout "{layout}"')
-        if not any(variant.code == setting.variant
+            if not any(kbd_variant.code == variant
-                   for variant in kbd_layout.variants):
+                       for kbd_variant in kbd_layout.variants):
-            raise ValueError(f'Unknown keyboard variant "{setting.variant}" '
+                raise ValueError(f'Unknown keyboard variant "{variant}" '
-                             f'for layout "{setting.layout}"')
+                                 f'for layout "{layout}"')
 
     def render_config_file(self):
         options = ""

subiquity/models/locale.py

@@ -37,22 +37,19 @@ class LocaleModel:
     def switch_language(self, code):
         self.selected_language = code
 
-    async def gen_localedef(self) -> None:
+    async def localectl_set_locale(self) -> None:
-        language, charmap = locale.normalize(self.selected_language).split(".")
         cmd = [
-            "localedef",
+            'localectl',
-            "-f", charmap,
+            'set-locale',
-            "-i", language,
+            locale.normalize(self.selected_language)
-            "--",
-            f"{language}.{charmap}",
         ]
         await arun_command(cmd, check=True)
 
-    async def try_gen_localedef(self) -> None:
+    async def try_localectl_set_locale(self) -> None:
         try:
-            await self.gen_localedef()
+            await self.localectl_set_locale()
         except subprocess.CalledProcessError as exc:
-            log.warning("Could not generate locale: %r", exc)
+            log.warning("Could not localectl set-locale: %r", exc)
 
     def __repr__(self):
         return "<Selected: {}>".format(self.selected_language)

subiquity/models/mirror.py

@@ -89,7 +89,9 @@ from curtin.commands.apt_config import (
     get_arch_mirrorconfig,
     get_mirror,
     PORTS_ARCHES,
+    PORTS_MIRRORS,
     PRIMARY_ARCHES,
+    PRIMARY_ARCH_MIRRORS,
     )
 from curtin.config import merge_config
 
@@ -100,8 +102,8 @@ except ImportError:
 
 log = logging.getLogger('subiquity.models.mirror')
 
-DEFAULT_SUPPORTED_ARCHES_URI = "http://archive.ubuntu.com/ubuntu"
+DEFAULT_SUPPORTED_ARCHES_URI = PRIMARY_ARCH_MIRRORS["PRIMARY"]
-DEFAULT_PORTS_ARCHES_URI = "http://ports.ubuntu.com/ubuntu-ports"
+DEFAULT_PORTS_ARCHES_URI = PORTS_MIRRORS["PRIMARY"]
 
 LEGACY_DEFAULT_PRIMARY_SECTION = [
     {
@@ -113,6 +115,17 @@ LEGACY_DEFAULT_PRIMARY_SECTION = [
     },
 ]
 
+DEFAULT_SECURITY_SECTION = [
+    {
+        "arches": PRIMARY_ARCHES,
+        "uri": PRIMARY_ARCH_MIRRORS["SECURITY"],
+    },
+    {
+        "arches": PORTS_ARCHES,
+        "uri": PORTS_MIRRORS["SECURITY"],
+    },
+]
+
 DEFAULT = {
     "preserve_sources_list": False,
 }
@@ -311,6 +324,10 @@ class MirrorModel(object):
 
         config = copy.deepcopy(self.config)
         config["disable_components"] = sorted(self.disabled_components)
+
+        if "security" not in config:
+            config["security"] = DEFAULT_SECURITY_SECTION
+
         return config
 
     def _get_apt_config_using_candidate(
@@ -321,7 +338,15 @@ class MirrorModel(object):
 
     def get_apt_config_staged(self) -> Dict[str, Any]:
         assert self.primary_staged is not None
-        return self._get_apt_config_using_candidate(self.primary_staged)
+        config = self._get_apt_config_using_candidate(self.primary_staged)
+
+        # For mirror testing, we disable the -security suite - so that we only
+        # test the primary mirror, not the security archive.
+        if "disable_suites" not in config:
+            config["disable_suites"]: List[str] = []
+        if "security" not in config["disable_suites"]:
+            config["disable_suites"].append("security")
+        return config
 
     def get_apt_config_elected(self) -> Dict[str, Any]:
         assert self.primary_elected is not None

subiquity/models/network.py

@@ -14,8 +14,10 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
+import subprocess
 
 from subiquitycore.models.network import NetworkModel as CoreNetworkModel
+from subiquitycore.utils import arun_command
 
 log = logging.getLogger('subiquity.models.network')
 
@@ -72,3 +74,16 @@ class NetworkModel(CoreNetworkModel):
             return ['wpasupplicant']
         else:
             return []
+
+    async def is_nm_enabled(self):
+        try:
+            cp = await arun_command(("nmcli", "networking"), check=True)
+        except subprocess.CalledProcessError as exc:
+            log.warning("failed to run nmcli networking,"
+                        " considering NetworkManager disabled.")
+            log.debug("stderr: %s", exc.stderr)
+            return False
+        except FileNotFoundError:
+            return False
+
+        return cp.stdout == "enabled\n"

subiquity/models/tests/test_keyboard.py

@@ -18,7 +18,10 @@ from subiquitycore.tests.parameterized import parameterized
 from subiquitycore.tests import SubiTestCase
 
 from subiquity.common.types import KeyboardSetting
-from subiquity.models.keyboard import KeyboardModel
+from subiquity.models.keyboard import (
+    InconsistentMultiLayoutError,
+    KeyboardModel,
+)
 
 
 class TestKeyboardModel(SubiTestCase):
@@ -45,6 +48,25 @@ class TestKeyboardModel(SubiTestCase):
             self.model.setting = val
         self.assertEqual(initial, self.model.setting)
 
+    def testMultiLayout(self):
+        val = KeyboardSetting(layout='us,ara', variant=',')
+        self.model.setting = val
+        self.assertEqual(self.model.setting, val)
+
+    def testInconsistentMultiLayout(self):
+        initial = self.model.setting
+        val = KeyboardSetting(layout='us,ara', variant='')
+        with self.assertRaises(InconsistentMultiLayoutError):
+            self.model.setting = val
+        self.assertEqual(self.model.setting, initial)
+
+    def testInvalidMultiLayout(self):
+        initial = self.model.setting
+        val = KeyboardSetting(layout='us,ara', variant='zz,')
+        with self.assertRaises(ValueError):
+            self.model.setting = val
+        self.assertEqual(self.model.setting, initial)
+
     @parameterized.expand([
         ['ast_ES.UTF-8', 'es', 'ast'],
         ['de_DE.UTF-8', 'de', ''],

subiquity/models/tests/test_locale.py

@@ -28,31 +28,29 @@ class TestLocaleModel(unittest.IsolatedAsyncioTestCase):
         self.model.switch_language("fr_FR.UTF-8")
         self.assertEqual(self.model.selected_language, "fr_FR.UTF-8")
 
-    async def test_gen_localedef(self):
+    async def test_localectl_set_locale(self):
         expected_cmd = [
-            "localedef",
+            "localectl",
-            "-f", "UTF-8",
+            "set-locale",
-            "-i", "fr_FR",
-            "--",
             "fr_FR.UTF-8",
         ]
         self.model.selected_language = "fr_FR.UTF-8"
         with mock.patch("subiquity.models.locale.arun_command") as arun_cmd:
-            await self.model.gen_localedef()
+            await self.model.localectl_set_locale()
         arun_cmd.assert_called_once_with(expected_cmd, check=True)
         self.model.selected_language = "fr_FR"
         with mock.patch("subiquity.models.locale.arun_command") as arun_cmd:
             # Currently, the default for fr_FR is fr_FR.ISO8859-1
             with mock.patch("subiquity.models.locale.locale.normalize",
                             return_value="fr_FR.UTF-8"):
-                await self.model.gen_localedef()
+                await self.model.localectl_set_locale()
         arun_cmd.assert_called_once_with(expected_cmd, check=True)
 
-    async def test_try_gen_localedef(self):
+    async def test_try_localectl_set_locale(self):
         self.model.selected_language = "fr_FR.UTF-8"
         exc = subprocess.CalledProcessError(returncode=1, cmd=["localedef"])
         with mock.patch("subiquity.models.locale.arun_command",
                         side_effect=exc):
-            await self.model.try_gen_localedef()
+            await self.model.try_localectl_set_locale()
         with mock.patch("subiquity.models.locale.arun_command"):
-            await self.model.try_gen_localedef()
+            await self.model.try_localectl_set_locale()

subiquity/models/tests/test_mirror.py

@@ -19,6 +19,7 @@ from unittest import mock
 
 from subiquity.models.mirror import (
     countrify_uri,
+    DEFAULT_SECURITY_SECTION,
     LEGACY_DEFAULT_PRIMARY_SECTION,
     MirrorModel,
     MirrorSelectionFallback,
@@ -150,7 +151,7 @@ class TestMirrorModel(unittest.TestCase):
                 self.assertIn(
                     country_mirror_candidate.uri,
                     [
-                        "http://CC.archive.ubuntu.com/ubuntu",
+                        "http://CC.archive.ubuntu.com/ubuntu/",
                         "http://CC.ports.ubuntu.com/ubuntu-ports",
                     ])
 
@@ -288,3 +289,96 @@ class TestMirrorModel(unittest.TestCase):
                 return_value=iter([PrimaryEntry(parent=self.model)]))
         with country_mirror_candidates:
             self.assertTrue(self.model.wants_geoip())
+
+    def test_get_apt_config_staged_default_config(self):
+        self.model.legacy_primary = False
+        self.model.primary_candidates = [
+            PrimaryEntry(
+                uri="http://mirror.local/ubuntu",
+                arches=None,
+                parent=self.model
+            ),
+        ]
+        self.model.primary_candidates[0].stage()
+        config = self.model.get_apt_config_staged()
+        self.assertEqual(
+            config["primary"],
+            [
+                {
+                    "uri": "http://mirror.local/ubuntu",
+                    "arches": ["default"],
+                }
+            ],
+        )
+        self.assertEqual(
+            set(config["disable_components"]),
+            set(self.model.disabled_components)
+        )
+        self.assertEqual(set(config["disable_suites"]), {"security"})
+        self.assertEqual(config["security"], DEFAULT_SECURITY_SECTION)
+
+    def test_get_apt_config_staged_with_config(self):
+        self.model.legacy_primary = False
+        self.model.primary_candidates = [
+            PrimaryEntry(
+                uri="http://mirror.local/ubuntu",
+                arches=None,
+                parent=self.model
+            ),
+        ]
+        self.model.primary_candidates[0].stage()
+        security_config = [
+            {
+                "arches": ["default"],
+                "uri": "http://security.ubuntu.com/ubuntu",
+            },
+        ]
+        self.model.config = {
+            "disable_suites": ["updates"],
+            "security": security_config,
+        }
+        config = self.model.get_apt_config_staged()
+        self.assertEqual(
+            config["primary"],
+            [
+                {
+                    "uri": "http://mirror.local/ubuntu",
+                    "arches": ["default"],
+                }
+            ],
+        )
+        self.assertEqual(
+            set(config["disable_components"]),
+            set(self.model.disabled_components)
+        )
+        self.assertEqual(
+            set(config["disable_suites"]),
+            {"security", "updates"}
+        )
+        self.assertEqual(config["security"], security_config)
+
+    def test_get_apt_config_elected_default_config(self):
+        self.model.legacy_primary = False
+        self.model.primary_candidates = [
+            PrimaryEntry(
+                uri="http://mirror.local/ubuntu",
+                arches=None,
+                parent=self.model
+            ),
+        ]
+        self.model.primary_candidates[0].elect()
+        config = self.model.get_apt_config_elected()
+        self.assertEqual(
+            config["primary"],
+            [
+                {
+                    "uri": "http://mirror.local/ubuntu",
+                    "arches": ["default"],
+                }
+            ],
+        )
+        self.assertEqual(
+            set(config["disable_components"]),
+            set(self.model.disabled_components)
+        )
+        self.assertEqual(config["security"], DEFAULT_SECURITY_SECTION)

subiquity/models/tests/test_network.py

@@ -0,0 +1,47 @@
+# 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 <http://www.gnu.org/licenses/>.
+
+import subprocess
+import unittest
+from unittest import mock
+
+from subiquity.models.network import (
+    NetworkModel,
+)
+
+
+class TestNetworkModel(unittest.IsolatedAsyncioTestCase):
+    def setUp(self):
+        self.model = NetworkModel()
+
+    async def test_is_nm_enabled(self):
+        with mock.patch("subiquity.models.network.arun_command") as arun:
+            arun.return_value = subprocess.CompletedProcess([], 0)
+            arun.return_value.stdout = "enabled\n"
+            self.assertTrue(await self.model.is_nm_enabled())
+
+        with mock.patch("subiquity.models.network.arun_command") as arun:
+            arun.return_value = subprocess.CompletedProcess([], 0)
+            arun.return_value.stdout = "disabled\n"
+            self.assertFalse(await self.model.is_nm_enabled())
+
+        with mock.patch("subiquity.models.network.arun_command") as arun:
+            arun.side_effect = FileNotFoundError
+            self.assertFalse(await self.model.is_nm_enabled())
+
+        with mock.patch("subiquity.models.network.arun_command") as arun:
+            arun.side_effect = subprocess.CalledProcessError(
+                    1, [], None, "error")
+            self.assertFalse(await self.model.is_nm_enabled())

subiquity/server/ad_joiner.py

@@ -16,6 +16,7 @@
 import asyncio
 from contextlib import contextmanager
 import logging
+import os
 from socket import gethostname
 from subprocess import CalledProcessError
 from subiquitycore.utils import arun_command, run_command
@@ -29,18 +30,28 @@ log = logging.getLogger('subiquity.server.ad_joiner')
 
 
 @contextmanager
-def hostname_context(hostname: str):
+def joining_context(hostname: str, root_dir: str):
-    """ Temporarily adjusts the host name to [hostname] and restores it
+    """ Temporarily adjusts the host name to [hostname] and bind-mounts
-        back in the end of the caller scope. """
+        interesting system directories in preparation for running realm
+        in target's [root_dir], undoing it all on exit. """
     hostname_current = gethostname()
-    hostname_process = run_command(['hostname', hostname])
+    binds = ("/proc", "/sys", "/dev", "/run")
     try:
+        hostname_process = run_command(['hostname', hostname])
+        for bind in binds:
+            bound_dir = os.path.join(root_dir, bind[1:])
+            if bound_dir != bind:
+                run_command(["mount", "--bind", bind, bound_dir])
         yield hostname_process
     finally:
         # Restoring the live session hostname.
         hostname_process = run_command(['hostname', hostname_current])
         if hostname_process.returncode:
             log.info("Failed to restore live session hostname")
+        for bind in reversed(binds):
+            bound_dir = os.path.join(root_dir, bind[1:])
+            if bound_dir != bind:
+                run_command(["umount", "-f", bound_dir])
 
 
 class AdJoinStrategy():
@@ -54,14 +65,14 @@ class AdJoinStrategy():
             -> AdJoinResult:
         """ This method changes the hostname and perform a real AD join, thus
             should only run in a live session. """
+        root_dir = self.app.base_model.target
         # Set hostname for AD to determine FQDN (no FQDN option in realm join,
         # only adcli, which only understands the live system, but not chroot)
-        with hostname_context(hostname) as host_process:
+        with joining_context(hostname, root_dir) as host_process:
             if host_process.returncode:
                 log.info("Failed to set live session hostname for adcli")
                 return AdJoinResult.JOIN_ERROR
 
-            root_dir = self.app.root
             cp = await arun_command([self.realm, "join", "--install", root_dir,
                                      "--user", info.admin_name,
                                      "--computer-name", hostname,

subiquity/server/apt.py

@@ -33,7 +33,7 @@ from curtin.config import merge_config
 
 from subiquitycore.file_util import write_file, generate_config_yaml
 from subiquitycore.lsb_release import lsb_release
-from subiquitycore.utils import astart_command
+from subiquitycore.utils import astart_command, orig_environ
 
 from subiquity.server.curtin import run_curtin_command
 from subiquity.server.mounter import (
@@ -183,7 +183,7 @@ class AptConfigurer:
         for target in get_index_targets():
             apt_cmd.append(f"-o{target}::DefaultEnabled=false")
 
-        env = os.environ.copy()
+        env = orig_environ(None)
         env["LANG"] = self.app.base_model.locale.selected_language
         with tempfile.NamedTemporaryFile(mode="w+") as config_file:
             env["APT_CONFIG"] = config_file.name

subiquity/server/controllers/ad.py

@@ -128,7 +128,11 @@ class AdController(SubiquityController):
     def interactive(self):
         # Since we don't accept the domain admin password in the autoinstall
         # file, this cannot be non-interactive.
-        return True
+
+        # HACK: the interactive behavior is causing some autoinstalls with
+        # desktop to block.
+        # return True
+        return False
 
     def __init__(self, app):
         super().__init__(app)

subiquity/server/controllers/filesystem.py

@@ -22,7 +22,7 @@ import os
 import pathlib
 import select
 import time
-from typing import Dict, List, Optional
+from typing import Any, Dict, List, Optional
 
 from curtin.storage_config import ptable_uuid_to_flag_entry
 
@@ -70,6 +70,7 @@ from subiquity.common.types import (
     ProbeStatus,
     ReformatDisk,
     StorageEncryptionSupport,
+    SizingPolicy,
     StorageResponse,
     StorageResponseV2,
     StorageSafety,
@@ -153,6 +154,10 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
         self._role_to_device: Dict[str: _Device] = {}
         self._device_to_structure: Dict[_Device: snapdapi.OnVolume] = {}
         self.use_tpm: bool = False
+        self.locked_probe_data = False
+        # If probe data come in while we are doing partitioning, store it in
+        # this variable. It will be picked up on next reset will pick it up.
+        self.queued_probe_data: Optional[Dict[str, Any]] = None
 
     def is_core_boot_classic(self):
         return self._system is not None
@@ -270,7 +275,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 +290,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']:
+        if choice.password is not None:
-            spec['passphrase'] = lvm_options['luks_options']['passphrase']
+            spec['passphrase'] = choice.password
         vg = self.create_volgroup(spec)
-        # There's no point using LVM and unconditionally filling the
+        if choice.sizing_policy == SizingPolicy.SCALED:
-        # VG with a single LV, but we should use more of a smaller
+            lv_size = sizes.scaled_rootfs_size(vg.size)
-        # 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.
-            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)
+        elif choice.sizing_policy == SizingPolicy.ALL:
+            lv_size = vg.size
+        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(
             vg=vg, spec=dict(
                 size=lv_size,
@@ -354,17 +350,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 +363,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, choice)
-            self.guided_lvm(gap, lvm_options=lvm_options)
         else:
             self.guided_direct(gap)
 
@@ -664,9 +648,22 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
 
     async def v2_reset_POST(self) -> StorageResponseV2:
         log.info("Resetting Filesystem model")
+        # From the API standpoint, it seems sound to set locked_probe_data back
+        # to False after a reset. But in practise, v2_reset_POST can be called
+        # during manual partitioning ; and we don't want to reenable automatic
+        # loading of probe data. Going forward, this could be controlled by an
+        # optional parameter maybe?
+        if self.queued_probe_data is not None:
+            log.debug("using newly obtained probe data")
+            self.model.load_probe_data(self.queued_probe_data)
+            self.queued_probe_data = None
+        else:
             self.model.reset()
         return await self.v2_GET()
 
+    async def v2_ensure_transaction_POST(self) -> None:
+        self.locked_probe_data = True
+
     async def v2_guided_GET(self, wait: bool = False) \
             -> GuidedStorageResponseV2:
         """Acquire a list of possible guided storage configuration scenarios.
@@ -722,21 +719,19 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
     async def v2_guided_POST(self, data: GuidedChoiceV2) \
             -> GuidedStorageResponseV2:
         log.debug(data)
+        self.locked_probe_data = True
         self.guided(data)
         return await self.v2_guided_GET()
 
     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)
         return await self.v2_GET()
 
-    async def v2_potential_boot_disks_GET(self) -> List[str]:
-        disks = self.potential_boot_disks(check_boot=True,
-                                          with_reformatting=False)
-        return [disk.id for disk in disks]
-
     async def v2_add_boot_partition_POST(self, disk_id: str) \
             -> StorageResponseV2:
+        self.locked_probe_data = True
         disk = self.model._one(id=disk_id)
         if boot.is_boot_device(disk):
             raise ValueError('device already has bootloader partition')
@@ -748,6 +743,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
     async def v2_add_partition_POST(self, data: AddPartitionV2) \
             -> StorageResponseV2:
         log.debug(data)
+        self.locked_probe_data = True
         if data.partition.boot is not None:
             raise ValueError('add_partition does not support changing boot')
         disk = self.model._one(id=data.disk_id)
@@ -769,6 +765,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
     async def v2_delete_partition_POST(self, data: ModifyPartitionV2) \
             -> StorageResponseV2:
         log.debug(data)
+        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)
@@ -777,6 +774,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
     async def v2_edit_partition_POST(self, data: ModifyPartitionV2) \
             -> StorageResponseV2:
         log.debug(data)
+        self.locked_probe_data = True
         disk = self.model._one(id=data.disk_id)
         partition = self.get_partition(disk, data.partition.number)
         if data.partition.size not in (None, partition.size) \
@@ -798,10 +796,19 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
         self.partition_disk_handler(disk, spec, partition=partition)
         return await self.v2_GET()
 
+    async def dry_run_wait_probe_POST(self) -> None:
+        if not self.app.opts.dry_run:
+            raise NotImplementedError
+
+        # This will start the probe task if not yet started.
+        self.ensure_probing()
+
+        await self._probe_task.task
+
     @with_context(name='probe_once', description='restricted={restricted}')
     async def _probe_once(self, *, context, restricted):
         if restricted:
-            probe_types = {'blockdev'}
+            probe_types = {'blockdev', 'filesystem'}
             fname = 'probe-data-restricted.json'
             key = "ProbeDataRestricted"
         else:
@@ -822,7 +829,11 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
         with open(fpath, 'w') as fp:
             json.dump(storage, fp, indent=4)
         self.app.note_file_for_apport(key, fpath)
+        if not self.locked_probe_data:
+            self.queued_probe_data = None
             self.model.load_probe_data(storage)
+        else:
+            self.queued_probe_data = storage
 
     @with_context()
     async def _probe(self, *, context=None):
@@ -917,8 +928,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,
+        sizing_policy = SizingPolicy.from_string(
-                                   password=password))
+                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'):
@@ -965,6 +979,14 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
         loop = asyncio.get_running_loop()
         loop.remove_reader(self._monitor.fileno())
 
+    def ensure_probing(self):
+        try:
+            self._probe_task.start_sync()
+        except TaskAlreadyRunningError:
+            log.debug('Skipping run of Probert - probe run already active')
+        else:
+            log.debug('Triggered Probert run on udev event')
+
     def _udev_event(self):
         cp = run_command(['udevadm', 'settle', '-t', '0'])
         if cp.returncode != 0:
@@ -981,12 +1003,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator):
         while select.select([self._monitor.fileno()], [], [], 0)[0]:
             action, dev = self._monitor.receive_device()
             log.debug("_udev_event %s %s", action, dev)
-        try:
+        self.ensure_probing()
-            self._probe_task.start_sync()
-        except TaskAlreadyRunningError:
-            log.debug('Skipping run of Probert - probe run already active')
-        else:
-            log.debug('Triggered Probert run on udev event')
 
     def make_autoinstall(self):
         rendered = self.model.render()

subiquity/server/controllers/install.py

@@ -34,6 +34,7 @@ from subiquitycore.async_helpers import (
     )
 from subiquitycore.context import with_context
 from subiquitycore.file_util import write_file, generate_config_yaml
+from subiquitycore.utils import log_process_streams
 
 from subiquity.common.errorreport import ErrorReportKind
 from subiquity.common.types import (
@@ -480,7 +481,11 @@ class InstallController(SubiquityController):
                 self.app, context, "in-target", "-t", self.tpath(),
                 "--", "unattended-upgrades", "-v",
                 private_mounts=True)
+            try:
                 await self.unattended_upgrades_cmd.wait()
+            except subprocess.CalledProcessError as cpe:
+                log_process_streams(logging.ERROR, cpe, 'Unattended upgrades')
+                context.description = f"FAILED to apply {policy} updates"
             self.unattended_upgrades_cmd = None
             self.unattended_upgrades_ctx = None
 

subiquity/server/controllers/keyboard.py

@@ -14,7 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
-from typing import Dict, Optional, Sequence
+from typing import Dict, Optional, Sequence, Tuple
 import os
 import pwd
 
@@ -48,7 +48,7 @@ standard_non_latin_layouts = set(
 default_desktop_user = 'ubuntu'
 
 
-def latinizable(layout_code, variant_code):
+def latinizable(layout_code, variant_code) -> Optional[Tuple[str, str]]:
     """
     If this setting does not allow the typing of latin characters,
     return a setting that can be switched to one that can.

subiquity/server/controllers/locale.py

@@ -67,5 +67,5 @@ class LocaleController(SubiquityController):
     async def POST(self, data: str):
         log.debug(data)
         self.model.switch_language(data)
-        async_helpers.run_bg_task(self.model.try_gen_localedef())
+        async_helpers.run_bg_task(self.model.try_localectl_set_locale())
         await self.configured()

subiquity/server/controllers/mirror.py

@@ -155,6 +155,7 @@ class MirrorController(SubiquityController):
             self._promote_mirror)
         self.apt_configurer = None
         self.mirror_check: Optional[MirrorCheck] = None
+        self.autoinstall_apply_started = False
 
     def load_autoinstall_data(self, data):
         if data is None:
@@ -273,6 +274,7 @@ class MirrorController(SubiquityController):
 
     @with_context()
     async def apply_autoinstall_config(self, context):
+        self.autoinstall_apply_started = True
         await self.run_mirror_selection_or_fallback(context)
 
     def on_geoip(self):
@@ -281,6 +283,12 @@ class MirrorController(SubiquityController):
         self.cc_event.set()
 
     def on_source(self):
+        if self.autoinstall_apply_started:
+            # Alternatively, we should cancel and restart the
+            # apply_autoinstall_config but this is out of scope.
+            raise RuntimeError("source model has changed but autoinstall"
+                               " configuration is already being applied")
+
         # FIXME disabled until we can sort out umount
         # if self.apt_configurer is not None:
         #     await self.apt_configurer.cleanup()
@@ -323,8 +331,14 @@ class MirrorController(SubiquityController):
 
     async def run_mirror_testing(self, output: io.StringIO) -> None:
         await self.source_configured_event.wait()
-        await self.apt_configurer.apply_apt_config(self.context, final=False)
+        # If the source model changes at the wrong time, there is a chance that
-        await self.apt_configurer.run_apt_config_check(output)
+        # self.apt_configurer will be replaced between the call to
+        # apply_apt_config and run_apt_config_check. Just make sure we still
+        # use the original one.
+        configurer = self.apt_configurer
+        await configurer.apply_apt_config(
+            self.context, final=False)
+        await configurer.run_apt_config_check(output)
 
     async def wait_config(self):
         await self._apply_apt_config_task.wait()

subiquity/server/controllers/network.py

@@ -239,13 +239,26 @@ class NetworkController(BaseNetworkController, SubiquityController):
 
     @with_context()
     async def apply_autoinstall_config(self, context):
+        want_apply_config = True
         if self.ai_data is None:
+            if not await self.model.is_nm_enabled():
                 with context.child("wait_initial_config"):
                     await self.initial_config
                 self.update_initial_configs()
                 self.apply_config(context)
+            else:
+                log.debug("NetworkManager is enabled and no network"
+                          " autoinstall section was found. Not applying"
+                          " network settings.")
+                want_apply_config = False
+        if want_apply_config:
             with context.child("wait_for_apply"):
                 await self.apply_config_task.wait()
+        else:
+            # Make sure we have read at least once the routing table.
+            # Careful, the following is a blocking call. But running it in a
+            # separate thread without locking sounds unsafe too.
+            self.network_event_receiver.probe_default_routes()
         self.model.has_network = self.network_event_receiver.has_default_route
 
     async def _apply_config(self, *, context=None, silent=False):
@@ -364,7 +377,7 @@ class NetworkController(BaseNetworkController, SubiquityController):
             if state == WLANSupportInstallState.INSTALLING:
                 self.pending_wlan_devices.add(dev)
                 return
-            elif state in [WLANSupportInstallState.FAILED.
+            elif state in [WLANSupportInstallState.FAILED,
                            WLANSupportInstallState.NOT_AVAILABLE]:
                 return
             # WLANSupportInstallState.DONE falls through

subiquity/server/controllers/shutdown.py

@@ -100,6 +100,18 @@ class ShutdownController(SubiquityController):
         if self.opts.dry_run:
             os.makedirs(target_logs, exist_ok=True)
         else:
+            # Preserve ephemeral boot cloud-init logs if applicable
+            cloudinit_logs = [
+               cloudinit_log
+               for cloudinit_log in (
+                    "/var/log/cloud-init.log",
+                    "/var/log/cloud-init-output.log"
+               )
+               if os.path.exists(cloudinit_log)
+            ]
+            if cloudinit_logs:
+                await arun_command(
+                    ['cp', '-a'] + cloudinit_logs + ['/var/log/installer'])
             await arun_command(
                 ['cp', '-aT', '/var/log/installer', target_logs])
             # Close the permissions from group writes on the target.

subiquity/server/controllers/tests/test_filesystem.py

@@ -21,17 +21,27 @@ 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
+from subiquity.common.filesystem.actions import DeviceAction
 from subiquity.common.types import (
+    AddPartitionV2,
     Bootloader,
+    Gap,
+    GapUsable,
     GuidedChoiceV2,
     GuidedStorageTargetReformat,
     GuidedStorageTargetResize,
     GuidedStorageTargetUseGap,
+    ModifyPartitionV2,
+    Partition,
     ProbeStatus,
+    ReformatDisk,
+    SizingPolicy,
     )
+from subiquity.models.filesystem import dehumanize_size
 from subiquity.models.tests.test_filesystem import (
     make_disk,
     make_model,
@@ -48,18 +58,23 @@ bootloaders_and_ptables = [(bl, pt)
 
 
 class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase):
+    MOCK_PREFIX = 'subiquity.server.controllers.filesystem.'
+
     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.app.prober = mock.Mock()
+        self.app.block_log_dir = '/inexistent'
+        self.app.note_file_for_apport = mock.Mock()
         self.fsc = FilesystemController(app=self.app)
         self.fsc._configured = True
 
     async def test_probe_restricted(self):
         await self.fsc._probe_once(context=None, restricted=True)
-        self.app.prober.get_storage.assert_called_with({'blockdev'})
+        expected = {'blockdev', 'filesystem'}
+        self.app.prober.get_storage.assert_called_with(expected)
 
     async def test_probe_os_prober_false(self):
         self.app.opts.use_os_prober = False
@@ -74,6 +89,197 @@ class TestSubiquityControllerFilesystem(IsolatedAsyncioTestCase):
         actual = self.app.prober.get_storage.call_args.args[0]
         self.assertTrue({'defaults', 'os'} <= actual)
 
+    async def test_probe_once_fs_configured(self):
+        self.fsc._configured = True
+        self.fsc.queued_probe_data = None
+        with mock.patch.object(self.fsc.model, 'load_probe_data') as load:
+            await self.fsc._probe_once(restricted=True)
+        self.assertIsNone(self.fsc.queued_probe_data)
+        load.assert_not_called()
+
+    @mock.patch('subiquity.server.controllers.filesystem.open',
+                mock.mock_open())
+    async def test_probe_once_locked_probe_data(self):
+        self.fsc._configured = False
+        self.fsc.locked_probe_data = True
+        self.fsc.queued_probe_data = None
+        self.app.prober.get_storage = mock.Mock(return_value={})
+        with mock.patch.object(self.fsc.model, 'load_probe_data') as load:
+            await self.fsc._probe_once(restricted=True)
+        self.assertEqual(self.fsc.queued_probe_data, {})
+        load.assert_not_called()
+
+    @mock.patch('subiquity.server.controllers.filesystem.open',
+                mock.mock_open())
+    async def test_probe_once_unlocked_probe_data(self):
+        self.fsc._configured = False
+        self.fsc.locked_probe_data = False
+        self.fsc.queued_probe_data = None
+        self.app.prober.get_storage = mock.Mock(return_value={})
+        with mock.patch.object(self.fsc.model, 'load_probe_data') as load:
+            await self.fsc._probe_once(restricted=True)
+        self.assertIsNone(self.fsc.queued_probe_data, {})
+        load.assert_called_once_with({})
+
+    async def test_v2_reset_POST_no_queued_data(self):
+        self.fsc.queued_probe_data = None
+        with mock.patch.object(self.fsc.model, 'load_probe_data') as load:
+            await self.fsc.v2_reset_POST()
+        load.assert_not_called()
+
+    async def test_v2_reset_POST_queued_data(self):
+        self.fsc.queued_probe_data = {}
+        with mock.patch.object(self.fsc.model, 'load_probe_data') as load:
+            await self.fsc.v2_reset_POST()
+        load.assert_called_once_with({})
+
+    async def test_v2_ensure_transaction_POST(self):
+        self.fsc.locked_probe_data = False
+        await self.fsc.v2_ensure_transaction_POST()
+        self.assertTrue(self.fsc.locked_probe_data)
+
+    async def test_v2_reformat_disk_POST(self):
+        self.fsc.locked_probe_data = False
+        with mock.patch.object(self.fsc, 'reformat') as reformat:
+            await self.fsc.v2_reformat_disk_POST(
+                    ReformatDisk(disk_id='dev-sda'))
+        self.assertTrue(self.fsc.locked_probe_data)
+        reformat.assert_called_once()
+
+    @mock.patch(MOCK_PREFIX + 'boot.is_boot_device',
+                mock.Mock(return_value=True))
+    async def test_v2_add_boot_partition_POST_existing_bootloader(self):
+        self.fsc.locked_probe_data = False
+        with mock.patch.object(self.fsc, 'add_boot_disk') as add_boot_disk:
+            with self.assertRaisesRegex(ValueError,
+                                        r'already\ has\ bootloader'):
+                await self.fsc.v2_add_boot_partition_POST('dev-sda')
+        self.assertTrue(self.fsc.locked_probe_data)
+        add_boot_disk.assert_not_called()
+
+    @mock.patch(MOCK_PREFIX + 'boot.is_boot_device',
+                mock.Mock(return_value=False))
+    @mock.patch(MOCK_PREFIX + 'DeviceAction.supported',
+                mock.Mock(return_value=[]))
+    async def test_v2_add_boot_partition_POST_not_supported(self):
+        self.fsc.locked_probe_data = False
+        with mock.patch.object(self.fsc, 'add_boot_disk') as add_boot_disk:
+            with self.assertRaisesRegex(ValueError,
+                                        r'does\ not\ support\ boot'):
+                await self.fsc.v2_add_boot_partition_POST('dev-sda')
+        self.assertTrue(self.fsc.locked_probe_data)
+        add_boot_disk.assert_not_called()
+
+    @mock.patch(MOCK_PREFIX + 'boot.is_boot_device',
+                mock.Mock(return_value=False))
+    @mock.patch(MOCK_PREFIX + 'DeviceAction.supported',
+                mock.Mock(return_value=[DeviceAction.TOGGLE_BOOT]))
+    async def test_v2_add_boot_partition_POST(self):
+        self.fsc.locked_probe_data = False
+        with mock.patch.object(self.fsc, 'add_boot_disk') as add_boot_disk:
+            await self.fsc.v2_add_boot_partition_POST('dev-sda')
+        self.assertTrue(self.fsc.locked_probe_data)
+        add_boot_disk.assert_called_once()
+
+    async def test_v2_add_partition_POST_changing_boot(self):
+        self.fsc.locked_probe_data = False
+        data = AddPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(
+                    format='ext4',
+                    mount='/',
+                    boot=True,
+                ), gap=Gap(
+                    offset=1 << 20,
+                    size=1000 << 20,
+                    usable=GapUsable.YES,
+                ))
+        with mock.patch.object(self.fsc, 'create_partition') as create_part:
+            with self.assertRaisesRegex(ValueError,
+                                        r'does\ not\ support\ changing\ boot'):
+                await self.fsc.v2_add_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        create_part.assert_not_called()
+
+    async def test_v2_add_partition_POST_too_large(self):
+        self.fsc.locked_probe_data = False
+        data = AddPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(
+                    format='ext4',
+                    mount='/',
+                    size=2000 << 20,
+                ), gap=Gap(
+                    offset=1 << 20,
+                    size=1000 << 20,
+                    usable=GapUsable.YES,
+                ))
+        with mock.patch.object(self.fsc, 'create_partition') as create_part:
+            with self.assertRaisesRegex(ValueError, r'too\ large'):
+                await self.fsc.v2_add_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        create_part.assert_not_called()
+
+    @mock.patch(MOCK_PREFIX + 'gaps.at_offset')
+    async def test_v2_add_partition_POST(self, at_offset):
+        at_offset.split = mock.Mock(return_value=[mock.Mock()])
+        self.fsc.locked_probe_data = False
+        data = AddPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(
+                    format='ext4',
+                    mount='/',
+                ), gap=Gap(
+                    offset=1 << 20,
+                    size=1000 << 20,
+                    usable=GapUsable.YES,
+                ))
+        with mock.patch.object(self.fsc, 'create_partition') as create_part:
+            await self.fsc.v2_add_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        create_part.assert_called_once()
+
+    async def test_v2_delete_partition_POST(self):
+        self.fsc.locked_probe_data = False
+        data = ModifyPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(number=1),
+                )
+        with mock.patch.object(self.fsc, 'delete_partition') as del_part:
+            with mock.patch.object(self.fsc, 'get_partition'):
+                await self.fsc.v2_delete_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        del_part.assert_called_once()
+
+    async def test_v2_edit_partition_POST_change_boot(self):
+        self.fsc.locked_probe_data = False
+        data = ModifyPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(number=1, boot=True),
+                )
+        existing = Partition(number=1, size=1000 << 20, boot=False)
+        with mock.patch.object(self.fsc, 'partition_disk_handler') as handler:
+            with mock.patch.object(self.fsc, 'get_partition',
+                                   return_value=existing):
+                with self.assertRaisesRegex(ValueError, r'changing\ boot'):
+                    await self.fsc.v2_edit_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        handler.assert_not_called()
+
+    async def test_v2_edit_partition_POST(self):
+        self.fsc.locked_probe_data = False
+        data = ModifyPartitionV2(
+                disk_id='dev-sda',
+                partition=Partition(number=1),
+                )
+        existing = Partition(number=1, size=1000 << 20, boot=False)
+        with mock.patch.object(self.fsc, 'partition_disk_handler') as handler:
+            with mock.patch.object(self.fsc, 'get_partition',
+                                   return_value=existing):
+                await self.fsc.v2_edit_partition_POST(data)
+        self.assertTrue(self.fsc.locked_probe_data)
+        handler.assert_called_once()
+
 
 class TestGuided(IsolatedAsyncioTestCase):
     boot_expectations = [
@@ -462,33 +668,75 @@ 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):
         self.app = make_app()
         self.app.opts.bootloader = bootloader.value
         self.fsc = FilesystemController(app=self.app)
+        self.fsc.calculate_suggested_install_min = mock.Mock()
+        self.fsc.calculate_suggested_install_min.return_value = 10 << 30
         self.fsc.model = self.model = make_model(bootloader)
+        self.model.storage_version = 2
+        self.fsc._probe_task.task = mock.Mock()
+        self.fsc._get_system_task.task = mock.Mock()
 
     @parameterized.expand(bootloaders_and_ptables)
     async def test_get_boot_disks_only(self, bootloader, ptable):
         self._setup(bootloader, ptable)
-        disk = make_disk(self.model)
+        make_disk(self.model)
-        self.assertEqual([disk.id],
+        resp = await self.fsc.v2_GET()
-                         await self.fsc.v2_potential_boot_disks_GET())
+        [d] = resp.disks
-
+        self.assertTrue(d.can_be_boot_device)
-    @parameterized.expand(bootloaders_and_ptables)
-    async def test_get_boot_disks_none(self, bootloader, ptable):
-        self._setup(bootloader, ptable)
-        self.assertEqual([], await self.fsc.v2_potential_boot_disks_GET())
 
     @parameterized.expand(bootloaders_and_ptables)
     async def test_get_boot_disks_all(self, bootloader, ptable):
         self._setup(bootloader, ptable)
-        d1 = make_disk(self.model)
+        make_disk(self.model)
-        d2 = make_disk(self.model)
+        make_disk(self.model)
-        self.assertEqual(set([d1.id, d2.id]),
+        resp = await self.fsc.v2_GET()
-                         set(await self.fsc.v2_potential_boot_disks_GET()))
+        [d1, d2] = resp.disks
+        self.assertTrue(d1.can_be_boot_device)
+        self.assertTrue(d2.can_be_boot_device)
 
     @parameterized.expand(bootloaders_and_ptables)
     async def test_get_boot_disks_some(self, bootloader, ptable):
@@ -500,11 +748,12 @@ class TestManualBoot(IsolatedAsyncioTestCase):
                        preserve=True)
         if bootloader == Bootloader.NONE:
             # NONE will always pass the boot check, even on a full disk
-            expected = set([d1.id, d2.id])
+            bootable = set([d1.id, d2.id])
         else:
-            expected = set([d2.id])
+            bootable = set([d2.id])
-        self.assertEqual(expected,
+        resp = await self.fsc.v2_GET()
-                         set(await self.fsc.v2_potential_boot_disks_GET()))
+        for d in resp.disks:
+            self.assertEqual(d.id in bootable, d.can_be_boot_device)
 
 
 class TestCoreBootInstallMethods(IsolatedAsyncioTestCase):

subiquity/server/runner.py

@@ -79,9 +79,10 @@ class LoggedCommandRunner:
             -> subprocess.CompletedProcess:
         stdout, stderr = await proc.communicate()
         # .communicate() forces returncode to be set to a value
-        assert(proc.returncode is not None)
+        assert proc.returncode is not None
         if proc.returncode != 0:
-            raise subprocess.CalledProcessError(proc.returncode, proc.args)
+            raise subprocess.CalledProcessError(
+                proc.returncode, proc.args, output=stdout, stderr=stderr)
         else:
             return subprocess.CompletedProcess(
                 proc.args, proc.returncode, stdout=stdout, stderr=stderr)

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
+    return matching_dicts(items, **kw)
-            if all(item.get(k) == v for k, v in kw.items())]
 
 
 def timeout(multiplier=1):
@@ -263,8 +265,9 @@ async def start_server_factory(factory, *args, **kwargs):
 
 
 @contextlib.asynccontextmanager
-async def start_server(*args, **kwargs):
+async def start_server(*args, set_first_source=True, **kwargs):
     async with start_server_factory(Server, *args, **kwargs) as instance:
+        if set_first_source:
             sources = await instance.get('/source')
             if sources is None:
                 raise Exception('unexpected /source response')
@@ -1411,6 +1414,37 @@ class TestRegression(TestAPI):
             [p] = resp['disks'][0]['partitions']
             self.assertEqual(orig_p, p)
 
+    @timeout()
+    async def test_no_change_edit_swap(self):
+        '''LP: 2002413 - editing a swap partition would fail with
+        > Exception: Filesystem(fstype='swap', ...) is already mounted
+        Make sure editing the partition is ok now.
+        '''
+        cfg = 'examples/simple.json'
+        extra = ['--storage-version', '2']
+        async with start_server(cfg, extra_args=extra) as inst:
+            resp = await inst.get('/storage/v2')
+            [d] = resp['disks']
+            [g] = d['partitions']
+            data = {
+                "disk_id": 'disk-sda',
+                "gap": g,
+                "partition": {
+                    "size": 8589934592,  # 8 GiB
+                    "format": "swap",
+                }
+            }
+            resp = await inst.post('/storage/v2/add_partition', data)
+            [p, gap] = resp['disks'][0]['partitions']
+            self.assertEqual('swap', p['format'])
+
+            orig_p = p.copy()
+
+            data = {"disk_id": 'disk-sda', "partition": p}
+            resp = await inst.post('/storage/v2/edit_partition', data)
+            [p, gap] = resp['disks'][0]['partitions']
+            self.assertEqual(orig_p, p)
+
     @timeout()
     async def test_can_create_unformatted_partition(self):
         '''We want to offer the same list of fstypes for Subiquity and U-D-I,
@@ -1436,6 +1470,81 @@ class TestRegression(TestAPI):
             v1resp = await inst.get('/storage')
             self.assertEqual([], match(v1resp['config'], type='format'))
 
+    @timeout()
+    async def test_guided_v2_resize_logical_middle_partition(self):
+        '''LP: #2015521 - a logical partition that wasn't the physically last
+        logical partition was resized to allow creation of more partitions, but
+        the 1MiB space was not left between the newly created partition and the
+        physically last partition.'''
+        cfg = 'examples/threebuntu-on-msdos.json'
+        extra = ['--storage-version', '2']
+        async with start_server(cfg, extra_args=extra) as inst:
+            resp = await inst.get('/storage/v2/guided')
+            [resize] = match(resp['possible'], partition_number=5,
+                             _type='GuidedStorageTargetResize')
+            data = {
+                'target': resize,
+                'use_lvm': False,
+                }
+            resp = await inst.post('/storage/v2/guided', data)
+            self.assertEqual(resize, resp['configured']['target'])
+
+            resp = await inst.get('/storage')
+            parts = match(resp['config'], type='partition', flag='logical')
+            logicals = []
+            for part in parts:
+                part['end'] = part['offset'] + part['size']
+                logicals.append(part)
+
+            logicals.sort(key=lambda p: p['offset'])
+            for i in range(len(logicals) - 1):
+                cur, nxt = logicals[i:i+2]
+                self.assertLessEqual(cur['end'] + (1 << 20), nxt['offset'],
+                                     f'partition overlap {cur} {nxt}')
+
+    @timeout()
+    async def test_probert_result_during_partitioning(self):
+        '''If a probert run finished during manual partition, we used to
+        load the probing data, essentially discarding changes made by the user
+        so far.  This test creates a new partition, simulates the end of a
+        probert run, and then tries to edit the previously created partition.
+        The edit operation would fail in earlier versions, because the new
+        partition would be discarded.
+        '''
+        cfg = 'examples/simple.json'
+        extra = ['--storage-version', '2']
+        async with start_server(cfg, extra_args=extra) as inst:
+            names = ['locale', 'keyboard', 'source', 'network', 'proxy',
+                     'mirror']
+            await inst.post('/meta/mark_configured', endpoint_names=names)
+            resp = await inst.get('/storage/v2')
+            [d] = resp['disks']
+            [g] = d['partitions']
+            data = {
+                'disk_id': 'disk-sda',
+                'gap': g,
+                'partition': {
+                    'size': -1,
+                    'mount': '/',
+                    'format': 'ext4',
+                }
+            }
+            add_resp = await inst.post('/storage/v2/add_partition', data)
+            [sda] = add_resp['disks']
+            [root] = match(sda['partitions'], mount='/')
+
+            # Now let's make sure we get the results from a probert run to kick
+            # in.
+            await inst.post('/storage/dry_run_wait_probe')
+            data = {
+                'disk_id': 'disk-sda',
+                'partition': {
+                    'number': root['number'],
+                }
+            }
+            # We should be able to modify the created partition.
+            await inst.post('/storage/v2/edit_partition', data)
+
 
 class TestCancel(TestAPI):
     @timeout()
@@ -1619,7 +1728,8 @@ class TestAutoinstallServer(TestAPI):
             '--autoinstall', 'examples/autoinstall-short.yaml',
             '--source-catalog', 'examples/install-sources.yaml',
         ]
-        async with start_server(cfg, extra_args=extra) as inst:
+        async with start_server(cfg, extra_args=extra,
+                                set_first_source=False) as inst:
             view_request_unspecified, resp = await inst.get(
                     '/locale',
                     full_response=True)
@@ -1764,7 +1874,8 @@ class TestActiveDirectory(TestAPI):
             '--kernel-cmdline', 'autoinstall',
         ]
         try:
-            async with start_server(cfg, extra_args=extra) as inst:
+            async with start_server(cfg, extra_args=extra,
+                                    set_first_source=False) as inst:
                 endpoint = '/active_directory'
                 logdir = inst.output_base()
                 self.assertIsNotNone(logdir)

subiquity/ui/views/filesystem/filesystem.py

@@ -495,10 +495,10 @@ class FilesystemView(BaseView):
         return TablePile(rows)
 
     def _build_buttons(self):
-        self.done = Toggleable(done_btn(_("Done"), on_press=self.done))
+        self.done_btn = Toggleable(done_btn(_("Done"), on_press=self.done))
 
         return [
-            self.done,
+            self.done_btn,
             reset_btn(_("Reset"), on_press=self.reset),
             back_btn(_("Back"), on_press=self.cancel),
             ]
@@ -524,7 +524,7 @@ class FilesystemView(BaseView):
         # This is an awful hack, actual thinking required:
         self.lb.base_widget._select_first_selectable()
         can_install = self.model.can_install()
-        self.done.enabled = can_install
+        self.done_btn.enabled = can_install
         if self.showing_guidance:
             del self.frame.contents[0]
         guidance = self._guidance()

subiquity/ui/views/filesystem/lvm.py

@@ -157,15 +157,27 @@ class VolGroupStretchy(Stretchy):
             label = _('Save')
             devices = {}
             key = ""
+            encrypt = False
             for d in existing.devices:
                 if d.type == "dm_crypt":
+                    encrypt = True
+                    # If the DM_Crypt object was created using information
+                    # sent by the server (this happens when the passphrase was
+                    # provided in the Guided Storage screen), it will not
+                    # contain a key but a path to a keyfile (d.keyfile). The
+                    # client may not have permission to read the keyfile so it
+                    # seems simpler to just present an empty passphrase field
+                    # and ask the user to fill the passphrase again if they
+                    # want to make adjustments to the VG.
+                    # TODO make this more user friendly.
+                    if d.key is not None:
                         key = d.key
                     d = d.volume
                 devices[d] = 'active'
             initial = {
                 'devices': devices,
                 'name': existing.name,
-                'encrypt': bool(key),
+                'encrypt': encrypt,
                 'passphrase': key,
                 'confirm_passphrase': key,
                 }

subiquity/ui/views/filesystem/partition.py

@@ -21,6 +21,7 @@ configuration.
 """
 import logging
 import re
+from typing import Optional
 
 from urwid import connect_signal, Text
 
@@ -91,6 +92,7 @@ class FSTypeField(FormField):
 class SizeWidget(StringEditor):
     def __init__(self, form):
         self.form = form
+        self.accurate_value: Optional[int] = None
         super().__init__()
 
     def lost_focus(self):
@@ -112,6 +114,9 @@ class SizeWidget(StringEditor):
                 ('info_minor',
                  _("Capped partition size at {size}").format(
                      size=self.form.size_str)))
+            # This will invoke self.form.clean_size() and it is expected that
+            # size_str (and therefore self.value) are propertly aligned.
+            self.accurate_value = self.form.size
         else:
             aligned_sz = align_up(sz, self.form.alignment)
             aligned_sz_str = humanize_size(aligned_sz)
@@ -120,6 +125,7 @@ class SizeWidget(StringEditor):
                 self.form.size.show_extra(
                     ('info_minor', _("Rounded size up to {size}").format(
                         size=aligned_sz_str)))
+            self.accurate_value = aligned_sz
 
 
 class SizeField(FormField):

subiquity/ui/views/filesystem/tests/test_partition.py

@@ -9,6 +9,7 @@ from subiquitycore.view import BaseView
 from subiquity.client.controllers.filesystem import FilesystemController
 from subiquity.common.filesystem import gaps
 from subiquity.models.filesystem import (
+    MiB,
     dehumanize_size,
     )
 from subiquity.models.tests.test_filesystem import (
@@ -64,6 +65,7 @@ class PartitionViewTests(unittest.TestCase):
         gap = gaps.Gap(device=disk, offset=1 << 20, size=99 << 30)
         view, stretchy = make_partition_view(model, disk, gap=gap)
         view_helpers.enter_data(stretchy.form, valid_data)
+        stretchy.form.size.widget.lost_focus()
         view_helpers.click(stretchy.form.done_btn.base_widget)
         valid_data['mount'] = '/'
         valid_data['size'] = dehumanize_size(valid_data['size'])
@@ -82,6 +84,7 @@ class PartitionViewTests(unittest.TestCase):
         view, stretchy = make_partition_view(model, disk, partition=partition)
         self.assertTrue(stretchy.form.done_btn.enabled)
         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']),
@@ -117,6 +120,7 @@ class PartitionViewTests(unittest.TestCase):
         self.assertFalse(stretchy.form.size.enabled)
         self.assertTrue(stretchy.form.done_btn.enabled)
         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 = {
             'fstype': 'xfs',
@@ -184,6 +188,7 @@ class PartitionViewTests(unittest.TestCase):
         self.assertEqual(stretchy.form.mount.value, "/boot/efi")
 
         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']),
@@ -252,3 +257,26 @@ class PartitionViewTests(unittest.TestCase):
         view, stretchy = make_format_entire_view(model, disk)
         self.assertEqual(
             stretchy.form.fstype.value, None)
+
+    def test_create_partition_unaligned_size(self):
+        # In LP: #2013201, the user would type in 1.1G and the partition
+        # created would not be aligned to a MiB boundary.
+        unaligned_data = {
+            'size': '1.1G',  # Corresponds to 1181116006.4 bytes (not an int)
+            'fstype': 'ext4',
+        }
+        valid_data = {
+            'mount': '/',
+            'size': 1127 * MiB,  # ~1.10058 GiB
+            'use_swap': False,
+            'fstype': 'ext4',
+        }
+        model, disk = make_model_and_disk()
+        gap = gaps.Gap(device=disk, offset=1 << 20, size=99 << 30)
+        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
+        )

subiquitycore/controllers/network.py

@@ -46,6 +46,7 @@ from subiquitycore.ui.views.network import (
     )
 from subiquitycore.utils import (
     arun_command,
+    orig_environ,
     run_command,
     )
 
@@ -358,9 +359,13 @@ class BaseNetworkController(BaseController):
                          'systemd-networkd.service',
                          'systemd-networkd.socket'],
                         check=True)
+                env = orig_environ(None)
                 try:
-                    await arun_command(['netplan', 'apply'], check=True)
+                    await arun_command(['netplan', 'apply'],
-                except subprocess.CalledProcessError:
+                                       env=env, check=True)
+                except subprocess.CalledProcessError as cpe:
+                    log.debug('CalledProcessError: '
+                              f'stdout[{cpe.stdout}] stderr[{cpe.stderr}]')
                     error("apply")
                     raise
                 if devs_to_down or devs_to_delete:

subiquitycore/tests/test_utils.py

@@ -40,10 +40,16 @@ class TestOrigEnviron(SubiTestCase):
         expected = {}
         self.assertEqual(expected, orig_environ(env))
 
+    def test_no_ld_library_path(self):
+        env = {'LD_LIBRARY_PATH': 'a'}
+        expected = {}
+        self.assertEqual(expected, orig_environ(env))
+
     def test_practical(self):
         snap = '/snap/subiquity/1234'
         env = {
             'TERM': 'linux',
+            'LD_LIBRARY_PATH': '/var/lib/snapd/lib/gl',
             'PYTHONIOENCODING_ORIG': '',
             'PYTHONIOENCODING': 'utf-8',
             'SUBIQUITY_ROOT_ORIG': '',

subiquitycore/ui/form.py

@@ -554,6 +554,10 @@ class Form(object, metaclass=MetaForm):
         data = {}
         for field in self._fields:
             if field.enabled:
+                accurate_value = getattr(field.widget, "accurate_value", None)
+                if accurate_value is not None:
+                    data[field.field.name] = accurate_value
+                else:
                     data[field.field.name] = field.value
         return data
 

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")
 
@@ -36,6 +36,8 @@ def _clean_env(env, *, locale=True):
 
 
 def orig_environ(env):
+    """Generate an environment dict that is suitable for use for running
+    programs that live outside the snap."""
     if env is None:
         env = os.environ
     ret = env.copy()
@@ -47,6 +49,7 @@ def orig_environ(env):
             else:
                 del ret[key_to_restore]
             del ret[key]
+    ret.pop('LD_LIBRARY_PATH', None)
     return ret
 
 
@@ -102,7 +105,7 @@ async def arun_command(cmd: Sequence[str], *,
             stderr = stderr.decode(encoding)
     log.debug("arun_command %s exited with code %s", cmd, proc.returncode)
     # .communicate() forces returncode to be set to a value
-    assert(proc.returncode is not None)
+    assert proc.returncode is not None
     if check and proc.returncode != 0:
         raise subprocess.CalledProcessError(proc.returncode, cmd,
                                             stdout, stderr)
@@ -139,6 +142,26 @@ def start_command(cmd: Sequence[str], *,
                             env=_clean_env(env, locale=clean_locale), **kw)
 
 
+def _log_stream(level: int, stream, name: str):
+    if stream:
+        log.log(level, f'{name}: ------------------------------------------')
+        for line in stream.splitlines():
+            log.log(level, line)
+    elif stream is None:
+        log.log(level, f'<{name} is None>')
+    else:
+        log.log(level, f'<{name} is empty>')
+
+
+def log_process_streams(level: int,
+                        cpe: subprocess.CalledProcessError,
+                        command_msg: str):
+    log.log(level, f'{command_msg} exited with result: {cpe.returncode}')
+    _log_stream(level, cpe.stdout, 'stdout')
+    _log_stream(level, cpe.stderr, 'stderr')
+    log.log(level, '--------------------------------------------------')
+
+
 # FIXME: replace with passlib and update package deps
 def crypt_password(passwd, algo='SHA-512'):
     # encryption algo - id pairs for crypt()
@@ -173,3 +196,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())]

system_setup/server/controllers/configure.py

@@ -91,7 +91,9 @@ class ConfigureController(SubiquityController):
 
     def __update_locale_cmd(self, lang) -> List[str]:
         """ Add mocking cli to update-locale if in dry-run."""
-        updateLocCmd = ["update-locale", "LANG={}".format(lang),
+        # A fixed path should be ok here since all releases (so far) ship
+        # the locales package.
+        updateLocCmd = ["/usr/sbin/update-locale", "LANG={}".format(lang),
                         "--no-checks"]
         if not self.app.opts.dry_run:
             return updateLocCmd
@@ -123,13 +125,12 @@ class ConfigureController(SubiquityController):
 
         return True
 
-    async def __recommended_language_packs(self, lang) \
+    async def __recommended_language_packs(self, lang, env) \
             -> Optional[List[str]]:
         """ Return a list of package names recommended by
          check-language-support (or a fake list if in dryrun).
          List returned can be empty on success. None for failure.
         """
-        clsCommand = "check-language-support"
         # lang code may be separated by @, dot or spaces.
         # clsLang = lang.split('@')[0].split('.')[0].split(' ')[0]
         pattern = re.compile(r'([^.@\s]+)', re.IGNORECASE)
@@ -154,13 +155,27 @@ class ConfigureController(SubiquityController):
         # ever by just '.'. On the other hand in dry-run we want it pointing to
         # '/' if not properly set.
         snap_dir = snap_dir if snap_dir != '.' else '/'
-        data_dir = os.path.join(snap_dir, "usr/share/language-selector")
+        data_dir_base = "usr/share/language-selector"
+        data_dir = os.path.join(snap_dir, data_dir_base)
+        # jammy does not (yet?) ship language-selector seeded.
+        # being defensive to prevent crashes.
+        clsCommand = "check-language-support"
+        envcp = None
         if not os.path.exists(data_dir):
-            log.error("Misconfigured snap environment pointed L-S-C data dir"
+            log.error("Language selector data dir %s seems not to be part"
-                      " to %s", data_dir)
+                      " of the snap.", data_dir)
+            # Try seeded L-S-C
+            data_dir = os.path.join(self.model.root, data_dir_base)
+            if not os.path.exists(data_dir):
+                log.error("Cannot find language selector data directory.")
                 return None
 
-        cp = await arun_command([clsCommand, "-d", data_dir, "-l", clsLang])
+            # The env parameter is only needed if the package isn't in the snap
+            envcp = env
+            clsCommand = os.path.join("/usr/bin/", clsCommand)
+
+        cp = await arun_command([clsCommand, "-d", data_dir, "-l", clsLang],
+                                env=envcp)
         if cp.returncode != 0:
             log.error('Command "%s" failed with return code %d',
                       cp.args, cp.returncode)
@@ -181,7 +196,7 @@ class ConfigureController(SubiquityController):
         """ Install recommended packages.
         lang is expected to be one single language/locale.
         """
-        packages = await self.__recommended_language_packs(lang)
+        packages = await self.__recommended_language_packs(lang, env)
         # Hardcoded path is necessary to ensure escaping out of the snap env.
         aptCommand = "/usr/bin/apt"
         if packages is None: