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

Shrink /boot by removing duplicate and unused grub binaries/modules #1955

Merged
merged 4 commits into from
Jun 21, 2024

Conversation

jepio
Copy link
Member

@jepio jepio commented Apr 25, 2024

Shrink /boot

Remove grub modules from disk that are assembled into the grub executable.
Remove grub executables that are copies that are not used in the boot path.

This should have no user visible impact.

How to use

[ describe what reviewers need to do in order to validate this PR ]

Testing done

[Describe the testing you have done before submitting this PR. Please include both the commands you issued as well as the output you got.]

  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

@jepio jepio requested a review from a team April 25, 2024 10:33
@@ -183,6 +183,12 @@ sudo grub-mkimage \
--output "${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}" \
"${CORE_MODULES[@]}"

if [[ "${FLAGS_target}" != i386-pc ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Since they become part of the core.img it should be possible to remove this, or? I think we can try this out and either BIOS will boot or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will also try this out and see if BIOS works or not.

Copy link
Contributor

@ader1990 ader1990 Jun 20, 2024

Choose a reason for hiding this comment

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

With @pothos suggestion to remove also the i386-pc parts:

used 61681
free 67358

versus before:

used 61782
free 67258

The image boots correctly as a Hyper-V gen 1 VM.

Copy link

github-actions bot commented Jun 19, 2024

Build action triggered: https://github.com/flatcar/scripts/actions/runs/9614448176

@ader1990
Copy link
Contributor

ader1990 commented Jun 20, 2024

This is the current difference on AMD64:

this PR amd64 /boot 48% used:
used 61782
free 67258

current alpha amd64 /boot 50% used:
used 63483
free 65556

The flatcar image was tested and booted correctly on both Gen1 and Gen2 Hyper-V (BIOS and UEFI).

@ader1990
Copy link
Contributor

Hello @jepio, I have verified that the image boots on both Gen 1/Gen 2 for Hyper-V AMD64.

@ader1990
Copy link
Contributor

Verified on ARM64 and the free space looks good too:

Filesystem               1K-blocks Used Available Use% Mounted on
/dev/nvme0n1p1    129039   61556     67483  48% /boot

jepio and others added 3 commits June 21, 2024 11:04
We currently carry multiple copies of the same grub core.elf or core.efi
on the boot partition. Save some space by removing duplicates that are
never used at runtime. CPIO build needed to be adapted because it
publishes grub efi files.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
Since we build them into the grub executable, they are not needed on
disk. The only case I am unsure of is legacy BIOS boot, so left those
on disk.

Signed-off-by: Jeremi Piotrowski <jpiotrowski@microsoft.com>
Signed-off-by: Adrian Vladu <avladu@cloudbasesolutions.com>
The image also boots on Hyper-V Generation 1 VM (BIOS) if the modules
are removed.

Signed-off-by: Adrian Vladu <avladu@cloudbasesolutions.com>
Signed-off-by: Adrian Vladu <avladu@cloudbasesolutions.com>
@ader1990 ader1990 changed the title Shrink /boot Shrink /boot by removing duplicate and unused grub binaries/modules Jun 21, 2024
@tormath1 tormath1 added the main label Jun 21, 2024
@ader1990 ader1990 merged commit 023ecbf into main Jun 21, 2024
1 check failed
@jepio
Copy link
Member Author

jepio commented Jun 21, 2024

@ader1990 keep in mind that existing nodes will still have these files so this PR is just a cleanup, and doesn't let us ship a larger kernel.

@ader1990
Copy link
Contributor

ader1990 commented Jun 21, 2024

@ader1990 keep in mind that existing nodes will still have these files so this PR is just a cleanup, and doesn't let us ship a larger kernel.

Definitely, this PR will help a little to not go beyond the 50% logical threshold in the near future with things like natural size increase of the Linux kernel / other portage stable minor versions.

@ader1990
Copy link
Contributor

@jepio I get what you mean, so we need to be careful not to go beyond a certain threshold with the vmlinuz kernel file. I will post the numbers for both amd64 / arm64 on the main issue so that we remember the sizes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants