container-encapsulate: Mark initramfs as exclusive component#5535
container-encapsulate: Mark initramfs as exclusive component#5535HastD wants to merge 1 commit intocoreos:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request correctly identifies that the initramfs should be treated as a component to ensure it's placed in its own layer. The change from path_packages to path_components is the right approach. I've added one comment with a suggestion to further improve the consistency of this change by aligning the metadata handling for initramfs with how other components are processed. This will make the code cleaner and more maintainable.
ae439c4 to
f9343fa
Compare
f9343fa to
73128f9
Compare
da16397 to
d8b8d12
Compare
|
Hi @HastD! You may be interested in https://github.com/jlebon/chunkah. It's a successor to build-chunked-oci to which bootable containers will likely eventually migrate. I'm having a similar discussion with myself in coreos/chunkah#16. But basically, two things I'll note:
Regardless, I also think we definitely should allow users to force any arbitrary files to be separate if they'd like/know better (filed coreos/chunkah#36 for that). Anyway, happy to start collaborating/chatting there if you're interested. The code is certainly not as baked/battle-tested as build-chunked-oci, and it still does dumb things, but it should improve quickly. |
|
Hi @jlebon, thanks for your work on Chunkah—it's a very cool project that I've been keeping an eye on. I anticipate that once it reaches a certain level of stability, BlueBuild will also add an option to use Chunkah in place of build-chunked-oci for rechunking images. In any case, while I do agree that it's not always optimal to put the initramfs in its own layer, I do think it's often still a good choice: the initramfs is large enough that it would significantly reduce average update sizes even if we can only avoid having to download its layer, say, every other update. I also agree that in some cases it might make more sense to instead special-case unpackaged As for this PR specifically, I think it's worth keeping in mind that this is simply a bug-fix: the intent of the existing code is to put the initramfs in its own layer as an exclusive component, but this doesn't happen because it's erroneously given package metadata instead of component metadata. This also has consequences that are clearly a bug: because the initramfs is incorrectly marked with package metadata, it can't be manually assigned to a layer using the |
The initramfs is not provided by an RPM package, but should be placed in its own exclusive layer. Therefore, it should be added to `path_components`, not `path_packages`, in the `MappingBuilder`, and should have component (not package) metadata associated to it. Signed-off-by: Daniel Hast <hast.daniel@protonmail.com>
d8b8d12 to
2c45ff1
Compare
The initramfs is not provided by an RPM package, but should be placed in its own exclusive layer. Therefore, it should be added to
path_components, notpath_packages, in theMappingBuilder, and should have component (not package) metadata associated to it.I've tested this change locally and it does indeed result in the initramfs being placed into its own layer as expected.
Fixes #5544.