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 (and optionally upgrade) available OEM meta-packages #1700

Merged
merged 3 commits into from
Jun 30, 2023

Conversation

ogayot
Copy link
Member

@ogayot ogayot commented Jun 21, 2023

  • Available OEM meta-packages are now installed unconditionally.
  • If online, the meta-packages will also get upgraded to their version from the OEM archive - essentially pulling a kernel with it.
  • We don't call ubuntu-drivers install --no-oem because it would also install third-party drivers on systems affected by https://bugs.launchpad.net/ubuntu/+source/ubuntu-drivers-common/+bug/1966413
  • We don't call ubuntu-drivers --no-oem install either because it would crash on affected systems
  • Instead we implement more or less the same logic as in ubuntu-drivers-common - using subsequent apt commands

Copy link
Collaborator

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

This is nice, and actually a lot less painful than I thought it would be. Having to copy/paste bits of ubuntu-drivers-common is unfortunate but I guess it's not actually that much code in the end.

I do think you should fix the "might install two kernels" thing before merging though.

"apt-get", "update",
"-o", f"Dir::Etc::SourceList={source_list}",
"-o", "Dir::Etc::SourceParts=/dev/null",
"--no-list-cleanup",
Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL --no-list-cleanup, that's interesting!


if self.model.network.has_network:
for pkg in self.model.oem.metapkgs:
source_list = f"/etc/apt/sources.list.d/{pkg}.list"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was going to ask if we can be sure that this is what the sources list entry is called, but I see this code is from ubuntu-drivers so presumably yes!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'm not sure either. I originally wanted to parse the output of dpkg -L or similar but I ended up taking the ubuntu-drivers approach instead.

@mwhudson
Copy link
Collaborator

Oh I forgot to say -- might be worth splitting the server/apt.py changes out to a separate PR.

private_mounts=False)

# NOTE In ubuntu-drivers, this is done in a single call to
# apt-get install.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think (but am not sure) that in any realistic scenario there is only one metapackage for any system. Do you know differently?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. I haven't seen the scenario where we would have more than one in practice but I only played around with a single certified laptop. I guess I should add a note somewhere that only one (or zero) is expected - but I don't think we should bail if ubuntu-drivers gives us two.

@ogayot
Copy link
Member Author

ogayot commented Jun 22, 2023

Thanks for the review!

This is nice, and actually a lot less painful than I thought it would be. Having to copy/paste bits of ubuntu-drivers-common is unfortunate but I guess it's not actually that much code in the end.

Agreed, I'm a bit worried that implementations might divert in the future so I hope we stay on sync with ubuntu-drivers. It would be good to add a install-oem command of some sort to ubuntu-drivers but realistically we wouldn't be able to use it on old ISOs and old ubuntu verisons, so I don't know if we should try.

I do think you should fix the "might install two kernels" thing before merging though.

Agreed. I've made progress on that part yesterday and I believe I have something that could work on ubuntu-desktop but does not on ubuntu-server. More on that later :)

h I forgot to say -- might be worth splitting the server/apt.py changes out to a separate PR.

Good point, I'll go ahead and do that.
EDIT: moved to #1701

@mwhudson
Copy link
Collaborator

Agreed. I've made progress on that part yesterday and I believe I have something that could work on ubuntu-desktop but does not on ubuntu-server. More on that later :)

OK, that sounds a bit confusing but I'll wait for you to enlighten me!

Agreed, I'm a bit worried that implementations might divert in the future so I hope we stay on sync with ubuntu-drivers. It would be good to add a install-oem command of some sort to ubuntu-drivers but realistically we wouldn't be able to use it on old ISOs and old ubuntu verisons, so I don't know if we should try.

I think we probably should. We aren't going to be making Jammy ISOs for that much longer and we do have a mechanism for holding updates back from older releases.

@ogayot
Copy link
Member Author

ogayot commented Jun 23, 2023

Agreed, I'm a bit worried that implementations might divert in the future so I hope we stay on sync with ubuntu-drivers. It would be good to add a install-oem command of some sort to ubuntu-drivers but realistically we wouldn't be able to use it on old ISOs and old ubuntu verisons, so I don't know if we should try.

I think we probably should. We aren't going to be making Jammy ISOs for that much longer and we do have a mechanism for holding updates back from older releases.

OK, I filed a feature request against ubuntu-drivers:

https://bugs.launchpad.net/ubuntu/+source/ubuntu-drivers-common/+bug/2024883

ogayot added 3 commits June 23, 2023 10:05
When we want to list third-party drivers, we want to avoid listing OEM
meta-packages as well. The ubuntu-drivers --no-oem list would do it but
we can't reliably use it for now. Let's manually exclude oem-*-meta
packages from the list.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When installing third-party drivers, we now pass the --no-oem option to
the ubuntu-drivers install command. Because of LP #1966413, the option
might get ignored, which is not ideal but sort of acceptable - because
we want OEM meta-packages installed unconditionally anyway.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
When OEM meta-packages are available, install them before running
curthooks. If we are online, we also upgrade the metapackages so that
they are upgraded to their version in the relevant OEM archive. This
will effectively pull the kernel.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
@ogayot
Copy link
Member Author

ogayot commented Jun 23, 2023

Rebased on top of main to drop from the diff the APT changes that were merged as part of #1701

@ogayot
Copy link
Member Author

ogayot commented Jun 23, 2023

We seem to be missing a call to apt-get update prior to running ubuntu-drivers list-oem. Some of the OEM meta-packages (e.g., oem-sutton-balin-meta) are not available in the package index when the installer boots.

@mwhudson
Copy link
Collaborator

We seem to be missing a call to apt-get update prior to running ubuntu-drivers list-oem. Some of the OEM meta-packages (e.g., oem-sutton-balin-meta) are not available in the package index when the installer boots.

Hm I think this might be more of a race condition, perhaps.

We run apt-get update before the install (InstallController.install -> InstallController.configure_apt -> AptConfigurer.configure_for_install) and then send the APT_CONFIGURED signal, which triggers the OEM controller to start looking for metapackages, but nothing waits on the task that starts that I can see? It seems a bit unlikely that ubuntu-drivers list-oem would take longer than block-meta and extract but could that explain what you are seeing?

@ogayot
Copy link
Member Author

ogayot commented Jun 29, 2023

We seem to be missing a call to apt-get update prior to running ubuntu-drivers list-oem. Some of the OEM meta-packages (e.g., oem-sutton-balin-meta) are not available in the package index when the installer boots.

Hm I think this might be more of a race condition, perhaps.

We run apt-get update before the install (InstallController.install -> InstallController.configure_apt -> AptConfigurer.configure_for_install) and then send the APT_CONFIGURED signal, which triggers the OEM controller to start looking for metapackages, but nothing waits on the task that starts that I can see? It seems a bit unlikely that ubuntu-drivers list-oem would take longer than block-meta and extract but could that explain what you are seeing?

Yes, I think that's possible. It might be about the gymnastics I'm doing to test OEM on non-certified hardware - I'm faking the output of some commands so maybe it does have an impact.

for pkg in self.model.oem.metapkgs:
await self.install_package(package=pkg)

if self.model.network.has_network:
Copy link
Collaborator

Choose a reason for hiding this comment

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

If someone is offline (as in an unfortunately timed network drop), either temporarily or (from the point of view of the install) permanently, what is the desired behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the general ISO, I doubt we'll include metapackages in the pool, so they won't get suggested. For an OEM image, all the relevant metapackages and their dependencies will be in the pool, so installing them will install all the relevant drivers. It will also install the sources.list.d entry, so we shouldn't run apt-get update as it will fail (hmm... I wonder if that will be a problem the way things are today?)

This is all a bit more complicated than I thought when starting this reply so maybe it should go in a comment somewhere :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add a comment regarding the pool.

If we're temporarily offline, then there will be retries when trying to download the required packages - and then we will bail. However, I haven't implemented a retry for the apt-get call - and there isn't any in the ubuntu-drivers install command either.

@ogayot ogayot merged commit fc458ff into canonical:main Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants