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

Add uefi boot support to compose data [RHELDST-18235] #356

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

amcmahon-rh
Copy link
Contributor

From RHEL 8.9 and RHEL 9.3 onwards UEFI will be available. The boot mode must be specified for AMI images to make use of the change. This change adds the boot mode to the metadata parsed in from composes. Data will be used by pubtools-ami and passed to cloudimg

From RHEL 8.9 and RHEL 9.3 onwards UEFI will be available. The boot mode must be
specified for AMI images to make use of the change. This change adds the boot
mode to the metadata parsed in from composes. Data will be used by
pubtools-ami and passed to cloudimg
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b809f7b) 100.00% compared to head (de4cff9) 100.00%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #356   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           53        53           
  Lines         2241      2242    +1     
=========================================
+ Hits          2241      2242    +1     
Files Changed Coverage Δ
src/pushsource/_impl/backend/staged/staged_ami.py 100.00% <ø> (ø)
src/pushsource/_impl/model/ami.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rohanpm rohanpm merged commit ce238b6 into release-engineering:master Aug 15, 2023
7 checks passed
type=bool, default=None, validator=optional(instance_of(bool))
)
"""``True`` if the image supports UEFI boot."""

Choose a reason for hiding this comment

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

@amcmahon-rh It is not such simple boolean value.
Image builder provides additional metadata in koji build e.g.

 'extra': {'typeinfo': {'image': {'rhel-azure-9.3-20230823.14.aarch64.vhd.xz': {'arch': 'aarch64',
                                                                                'boot_mode': 'uefi'},
                                  'rhel-azure-9.3-20230823.14.x86_64.vhd.xz': {'arch': 'x86_64',
                                                                               'boot_mode': 'hybrid'}}}},

aarch64 obviously has to use uefi. But with x86_64 it should try uefi but it can do fallback
in AWS terminology, it is 3 state: "uefi-preferred", "legacy-bios" and "uefi"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @lslebodn, I don't quite understand, could you elaborate please?

I understand that AWS requires specific string values. The idea of using a boolean was to keep the value abstract up to the point where we actually interact with AWS, rather than passing string values around different libraries.

Would this be resolved by having a second boolean? One for uefi and one for bios support?

Choose a reason for hiding this comment

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

@amcmahon-rh there are 4 possible ways for adding "BootMode" for image.
a) missing option "BootMode"
b) one of following values: uefi, legacy-bios, or uefi-preferred as it is described in AWS documentation(https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ami-boot-mode.html)

And we cannot achieve it with current with current "type=bool".

Because based on value of boolean cloudimg can use either "uefi-preferred" or "legacy-bios"

For aarch64 images, "uefi-preferred" is not ideal. It should be either "uefi" or the "BootMode" should not be specified at all when uploading image as it was before this change

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.

4 participants