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
Conversation
Add live_packages model method where install models may specify packages needed in the live system. Move efibootmgr there.
pre-Mantic installs can now zfsroot without autoinstall workarounds.
e561bca
to
96c0c91
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved but a couple of comments/questions
@@ -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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@@ -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): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, fixed
Split packages that must be done early from ones that can be done in parallel with early install steps.
8370ecc
to
0bbec68
Compare
The installdeps change is to address the CI failure on mantic due to conffile prompt. Prompt issue reported at https://bugs.launchpad.net/ubuntu/+source/ubuntu-advantage-tools/+bug/2033308 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like this might be a bit overengineered now but well whatever, you've done all the required typing!
No description provided.