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

Rework dracut defaults into separate subpackages for more granular image configuration #9326

Merged
merged 5 commits into from
Jun 7, 2024

Conversation

Camelron
Copy link
Contributor

@Camelron Camelron commented Jun 5, 2024

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • If you are adding/removing a .spec file that has multiple-versions supported, please add @microsoft/cbl-mariner-multi-package-reviewers team as reviewer (Eg. golang has 2 versions 1.18, 1.21+)
  • Ready to merge

Summary

What does the PR accomplish, why was it needed?
Due to a change in 3.0 which has us using dracut instead of mkinitrd (#8126), we now have a required set of kernel modules which must be present for a given kernel to generate an initramfs. kernel-mshv has a much more minimal set of kernel drivers built; it by definition does not need to support being virtualized over xen, for example. Because we have this universally required set of modules and it is the superset required for all scenarios, kernel-mshv is unable to generate an initramfs which breaks our Dom0 boot.

Mariner has historically installed the superset of kernel modules needed for all scenarios. Now, we care about installing only the relevant set of kernel modules needed for early boot in a given image's scenario. We accomplish this by separating out our dracut's defaults.conf into multiple smaller .conf files.

Change Log
  • Split dracut defaults into separate packages.
  • Flag new dracut subpackages as relevant to initramfs regeneration
  • Configure existing image configs with reasonable sets of dracut modules
Does this affect the toolchain?

NO

Test Methodology
  • Tested a wide set of scenarios to ensure that images are booting in all cases.
  • Core-efi image:
    • Local hyper-v vm - success
    • bvt tests - success
    • kdump - success
    • fips enablement of core image - success
    • full iso image install - success
  • Marketplace image:
    - AKS image - success
    - lisa tests - success, see above summary
    - x86 image - success
    - Arm image - success
    - kdump - success
    Baremetal images - success

@Camelron Camelron force-pushed the user/cameronbaird/dracutSplit2 branch from b8cf338 to 09ac4b4 Compare June 5, 2024 23:38
@Camelron Camelron force-pushed the user/cameronbaird/dracutSplit2 branch from 09ac4b4 to 4cda15b Compare June 5, 2024 23:40
@@ -0,0 +1,4 @@
# kdump currently uses the host system's initrd when enrolling a crash kernel
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious what would change if we had #9333 to regenerate the crash kernel initrd rather than using the host system's version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we would make the kdump initrd gen logic pass -H to explictly turn on hostonly. We would then not install hostonly.conf in our image configs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense - I'll add this in my kdump changes

SPECS/dracut/dracut.signatures.json Outdated Show resolved Hide resolved
@@ -75,6 +79,20 @@ Requires: nss
This package requires everything which is needed to build an
initramfs with dracut, which does an integrity check.

%package hostonly
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there an implied linkage between these dracut subpackages and whether the kernel supplies the underlying modules? Should each kernels "Requires" their relevant dracut subpackages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A given kernel doesn't require any module to go in initramfs. A given image being deployed on a certain platform is what defines the need for certain modules. Hence, we are choosing to compose the set of dracut packages in the image at the imageconfig level.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense about platforms defining what modules are in the initramfs. As we were chatting offline and you should consider if a "Requires: dracut >= X" in the kernel spec would help protect against regressions due to kernel module changes in kernel package updates

@Camelron Camelron marked this pull request as ready for review June 7, 2024 22:18
@Camelron Camelron requested review from a team as code owners June 7, 2024 22:18
@Camelron Camelron force-pushed the user/cameronbaird/dracutSplit2 branch from 45453f0 to 65b9b57 Compare June 7, 2024 22:40
@@ -75,6 +79,20 @@ Requires: nss
This package requires everything which is needed to build an
initramfs with dracut, which does an integrity check.

%package hostonly
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense about platforms defining what modules are in the initramfs. As we were chatting offline and you should consider if a "Requires: dracut >= X" in the kernel spec would help protect against regressions due to kernel module changes in kernel package updates

@@ -0,0 +1,4 @@
# kdump currently uses the host system's initrd when enrolling a crash kernel
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense - I'll add this in my kdump changes

@Camelron Camelron merged commit 3ce7901 into 3.0-dev Jun 7, 2024
12 checks passed
@Camelron Camelron deleted the user/cameronbaird/dracutSplit2 branch June 7, 2024 23:21
ddstreetmicrosoft pushed a commit that referenced this pull request Jun 12, 2024
anphel31 pushed a commit that referenced this pull request Jun 19, 2024
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