Skip to content

Conversation

@yasminvalim
Copy link
Contributor

TICKET: As part of https://fedoraproject.org/wiki/Changes/BootLoaderUpdatesPhase1, for package mode installations, we will update the bootloader (copy from /usr to boot or ESP) as part of the posttrans scriptlet.

This is a slightly different path as the current update path where we have the systemd-run logic, and we notably don't have any prepared metadata.

We can start by creating a command that does a simple copy (like the scripts do) and then iterate on merging the logic between the package mode and the image mode use case.

See change and current sample/basic installation scripts:

https://fedoraproject.org/wiki/Changes/BootLoaderUpdatesPhase1
https://src.fedoraproject.org/rpms/grub2/pull-request/136
https://src.fedoraproject.org/rpms/shim/pull-request/4

See related issue: #926

@yasminvalim yasminvalim self-assigned this Nov 4, 2025
@yasminvalim yasminvalim marked this pull request as draft November 4, 2025 20:45
gemini-code-assist[bot]

This comment was marked as resolved.

@yasminvalim yasminvalim force-pushed the COS-3682 branch 3 times, most recently from df959d6 to 0579cb7 Compare November 5, 2025 19:27
@yasminvalim
Copy link
Contributor Author

I've added some unit tests for this feature, but I'm not satisfied with the coverage. Are kola tests recommended in this case?

@HuijingHei
Copy link
Member

Thank you @yasminvalim for working on this!
This is tracked by #927, there are many dup codes in install(), how about make them into one? WDYT?

@HuijingHei
Copy link
Member

I've added some unit tests for this feature, but I'm not satisfied with the coverage. Are kola tests recommended in this case?

Since this is for package mode, do we want them also working on CoreOS?

@yasminvalim
Copy link
Contributor Author

Thank you @yasminvalim for working on this! This is tracked by #927, there are many dup codes in install(), how about make them into one? WDYT?

I created a two function helpers to extract duplicate code :)
Can you take a look again and let me know what do you think about it?

@yasminvalim
Copy link
Contributor Author

I've added some unit tests for this feature, but I'm not satisfied with the coverage. Are kola tests recommended in this case?

Since this is for package mode, do we want them also working on CoreOS?

I think there is no mention to it in the issue and jira ticket

@cgwalters
Copy link
Member

I've added some unit tests for this feature, but I'm not satisfied with the coverage. Are kola tests recommended in this case?

Yeah, it's hard for us to cover much here with unit tests. This intersects with the work to add tmt tests here #967

@yasminvalim
Copy link
Contributor Author

I've added some unit tests for this feature, but I'm not satisfied with the coverage. Are kola tests recommended in this case?

Yeah, it's hard for us to cover much here with unit tests. This intersects with the work to add tmt tests here #967

Thanks for the context. What do you think about the next steps for this PR? Should I proceed with merging these changes, or wait until we have more tmt test coverage in place?

@travier
Copy link
Member

travier commented Nov 10, 2025

Might be good to get a test here that installs shim into a container and ensures that the files are properly setup in the right place.

Extract duplicated ESP mounting, validation, and copying logic from install() and package_mode_copy_to_boot_impl() into shared helper function  to eliminate dupe code.
@yasminvalim
Copy link
Contributor Author

Might be good to get a test here that installs shim into a container and ensures that the files are properly setup in the right place.

I added a unit test to cover this case. Creating shim, installing, ensuring that is being copied, if is a really a file, if the content matches exactly and if the directory structure is correct. I guess we have a little bit more coverage now.

@yasminvalim yasminvalim marked this pull request as ready for review November 18, 2025 14:14
Addressing review: add unit test  that installs shim into a container and ensures that the files are properly setup in the right place
@HuijingHei
Copy link
Member

@travier the command that we want to use is for package mode, also for image mode, is this right?

yasminvalim and others added 6 commits November 20, 2025 10:24
Extract duplicated ESP mounting, validation, and copying logic from install() and package_mode_copy_to_boot_impl() into shared helper function  to eliminate dupe code.
Addressing review: add unit test  that installs shim into a container and ensures that the files are properly setup in the right place
@yasminvalim yasminvalim deleted the COS-3682 branch November 20, 2025 17:01
@yasminvalim
Copy link
Contributor Author

yasminvalim commented Nov 20, 2025

Sorry folks, I encountered some issues with the git history on my previous branch, so I've pushed this new branch to resolve it. #1030

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