-
Notifications
You must be signed in to change notification settings - Fork 141
Cleanup systemd UKI support #1623
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
Conversation
64d4c27
to
5269030
Compare
5269030
to
fcfcf66
Compare
fcfcf66
to
2fb37fb
Compare
This was approved, but I think it needs a re-review |
9c447bf
to
8c73037
Compare
@cgwalters This is also up for a re-review. Added a few more commits since the last one |
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.
A lot here, I didn't do an exhaustive review. What would probably help here is for us to ensure we're refining something like docs/internals/
for how we're doing some of this.
const EFI_LINUX: &str = "EFI/Linux"; | ||
|
||
/// Timeout for systemd-boot bootloader menu | ||
const SYSTEMD_TIMEOUT: &str = "timeout 5"; |
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 relates to coreos/bootupd#978 - I think the default for systems-boot should actually be set at install time.
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.
Agreed. But, for now at least we keep it in bootc until we have support for systemd-boot in bootupd
.await | ||
} | ||
|
||
/// skopeo (in composefs-rs) doesn't understand "registry:" |
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.
Yeah...that was something I invented in ostree-ext and probably should have argued to add to the containers-transports so that podman/skopeo/etc understand.
/// Ex | ||
/// docker://quay.io/some-image | ||
/// containers-storage:some-image | ||
pub(crate) fn get_imgref(transport: &String, image: &String) -> String { |
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 one looks very unit testable
I haven't followed the flow here but ideally we do youki-dev/oci-spec-rs#205 and then we're always using a parsed representation for these things.
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.
There are two ImageReference
structs ostree_ext::container::ImageReference
and lib::spec::ImageReference
and they I think ostree_ext::container::ImageReference
is being converted to lib::spec::ImageReference
, but Display
for Transport
is just "registry"
We should probably combine these throughout the codebase?
9847935
to
db4f838
Compare
This is in prep for adding config files for BLS compliant bootloaders booting via UKI. Adds a field `cfg_type` to BLSConfig which will contain either of the following sets of keys: cfg_type - NonEFI - linux - initrd - options or cfg_type - EFI - efi Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
We did not have config files for systemd-boot and were only using UKIs which did not allow proper sorting of the UKIs. This adds .conf files to `$ESP/loader/entries` Also, preserves UKI addons' names so we don't overwrite previously added addon Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
Add logic for upgrading/switching to a deployment with systemd-boot as the bootloader. Also update finalize-staged service to handle systemd-boot bootloader entries for UKIs Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
Allows installing only some of the addons depending upon the list of addons passed in as cli options. Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
skopeo (in composefs-rs) doesn't understand the transport "registry:", so we convert it to "docker://" when passing it to skopeo Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
Use UTF8Path in BLSConfig Use `ok_or_else` so error objects are lazily evaluated Add tests for `get_imgref` Update UKI path for systemd-boot to `EFI/Linux/bootc` Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com> Signed-off-by: Colin Walters <walters@verbum.org>
db4f838
to
1a732db
Compare
Changes in this PR
cfg_type
to BLSConfig which will contain either of thefollowing key:
We did not have config files for systemd-boot and were only using UKIs
which did not allow proper sorting of the UKIs. This adds .conf files
to
$ESP/loader/entries
Preserves UKI addons' names so we don't overwrite previously added addon
Adds a cli option
--uki-addon
to select the addons to installAdd logic for upgrading/switching to a deployment with systemd-boot as the bootloader