Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install zfsutils-linux if needed #1773

Merged
merged 7 commits into from Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions subiquity/models/filesystem.py
Expand Up @@ -1275,6 +1275,7 @@ def __init__(self, bootloader=None):
self.storage_version = 1
self._probe_data = None
self.dd_target: Optional[Disk] = None
self.reset_partition: Optional[Partition] = None
self.reset()

def reset(self):
Expand Down Expand Up @@ -2037,3 +2038,11 @@ def add_zpool(
)
self._actions.append(zpool)
return zpool

async def live_packages(self):
r = []
if self.reset_partition is not None:
r.append("efibootmgr")
if self._one(type="zpool") is not None:
r.append("zfsutils-linux")
return r
8 changes: 8 additions & 0 deletions subiquity/models/subiquity.py
Expand Up @@ -390,6 +390,14 @@ async def target_packages(self):
packages.extend(await meth())
return packages

async def live_packages(self):
packages = []
for model_name in self._install_model_names.all():
meth = getattr(getattr(self, model_name), "live_packages", None)
if meth is not None:
packages.extend(await meth())
return packages

def _cloud_init_files(self):
# TODO, this should be moved to the in-target cloud-config seed so on
# first boot of the target, it reconfigures datasource_list to none
Expand Down
7 changes: 2 additions & 5 deletions subiquity/server/controllers/filesystem.py
Expand Up @@ -62,9 +62,7 @@
ArbitraryDevice,
)
from subiquity.models.filesystem import Disk as ModelDisk
from subiquity.models.filesystem import MiB
from subiquity.models.filesystem import Partition as ModelPartition
from subiquity.models.filesystem import Raid, _Device, align_down, align_up
from subiquity.models.filesystem import MiB, Raid, _Device, align_down, align_up
from subiquity.server import snapdapi
from subiquity.server.controller import SubiquityController
from subiquity.server.mounter import Mounter
Expand Down Expand Up @@ -244,7 +242,6 @@ def __init__(self, app):
# If probe data come in while we are doing partitioning, store it in
# this variable. It will be picked up on next reset.
self.queued_probe_data: Optional[Dict[str, Any]] = None
self.reset_partition: Optional[ModelPartition] = None
self.reset_partition_only: bool = False

def is_core_boot_classic(self):
Expand Down Expand Up @@ -635,7 +632,7 @@ async def guided(
reset_size = int(cp.stdout.strip().split()[0])
reset_size = align_up(int(reset_size * 1.10), 256 * MiB)
reset_gap, gap = gap.split(reset_size)
self.reset_partition = self.create_partition(
self.model.reset_partition = self.create_partition(
device=reset_gap.device,
gap=reset_gap,
spec={"fstype": "fat32"},
Expand Down
10 changes: 7 additions & 3 deletions subiquity/server/controllers/install.py
Expand Up @@ -413,7 +413,7 @@ async def run_curtin_step(name, stages, step_config, source=None):
# really write recovery_system={snapd_system_label} to
# {target}/var/lib/snapd/modeenv to get snapd to pick it up on
# first boot. But not needed for now.
rp = fs_controller.reset_partition
rp = fs_controller.model.reset_partition
if rp is not None:
mounter = Mounter(self.app)
rp_target = os.path.join(self.app.root, "factory-reset")
Expand Down Expand Up @@ -597,6 +597,11 @@ async def create_core_boot_classic_fstab(self, *, context):
with open(self.tpath("etc/fstab"), "w") as fp:
fp.write("/run/mnt/ubuntu-boot/EFI/ubuntu /boot/grub none bind\n")

@with_context(description="installing packages to live system")
async def install_live_packages(self, *, context):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will show "installing packages to live system" to the user even if there are no packages being installed. It's annoying to fix but that's kind of bad UX.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, fixed

for package in await self.model.live_packages():
await self.app.package_installer.install_pkg(package)

@with_context()
async def install(self, *, context):
context.set("is-install-context", True)
Expand Down Expand Up @@ -627,8 +632,7 @@ async def install(self, *, context):
fsc = self.app.controllers.Filesystem
for_install_path = self.model.source.get_source(fsc._info.name)

if self.app.controllers.Filesystem.reset_partition:
self.app.package_installer.start_installing_pkg("efibootmgr")
await self.install_live_packages()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably not really important, but I was letting the install in the live environment run in parallel with the other bits here, whereas this will wait for the installs to be done. Was that intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having zfsutils-linux up front is intended, we'll need them fairly early (before rsync).
efibootmgr is safe with the later call waiting for task completion.
Updated to make this more nuanced, by allowing zfsutils-linux to be required up front and efibootmgr can run in the background.


if self.model.target is not None:
if os.path.exists(self.model.target):
Expand Down