rechunker: Support exclusive layers #5431
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces support for exclusive layers in the rechunker by adding a mechanism to map individual files to specific layers via the xattr user.component field. The changes include modifications to the MappingBuilder struct and associated methods, as well as the addition of a new function get_user_component_xattr to read the user.component xattr from a file path. The code also includes new unit tests to verify the functionality of the changes.
ckyrouac
force-pushed
the
rechunk-xattr
branch
2 times, most recently
from
217c33e to
3114e20
Compare
ckyrouac
changed the title
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Thanks again for working on this! Nothing truly blocking from my end but some open questions and nits
| echo "Regular system file" > /usr/share/regular/system.conf | ||
| # Set component xattrs to identify exclusive layers | ||
| RUN setfattr -n user.component -v "webapp" /usr/share/webapp/app.js && \ |
There was a problem hiding this comment.
Unrelated to this specific line but just looking at this...I think the 95% case here is that people will want this to operate recursively, right?
A nice way to do this is to make it supported to set user.component on a directory to mean applying to all of its children or so. And here debate whether that's including directory children i.e. recursive or not. If it is recursive we'd probably need an opt-out (setting the xattr to the empty string?)
Hmm and now that I look/think about this slightly more I guess the challenge is that the whole codebase leading up to this is object-based and isn't operating on the directory tree.
But in theory we could (while traversing to build up the object set) add this mapping if we detect the parent directory has it set.
To be clear I'm OK with this as is but I'd probably say we should change the tests and docs to show this via e.g.
find /usr/share/webapp -type -f -o -type l -exec setfattr -n user.component -v webapp {} \; or so?
ckyrouac
force-pushed
the
rechunk-xattr
branch
3 times, most recently
from
547c42e to
ae7030a
Compare
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
One nit otherwise looks sane to me! I only very superficially skimmed the tests
Also react to updated bwrap code, see gtk-rs/gtk-rs-core@037ae39 Signed-off-by: ckyrouac <[email protected]>
Parse the user.components xattr from the filesystem to create separate layers for each user specified component. "Exclusive" layers are given priority over package based layers. Assited-by: Claude Code Signed-off-by: ckyrouac <[email protected]>
|
@ckyrouac: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
depends on bootc-dev/bootc#1397.
Adds a mechanism to the rechunker to support mapping individual files to specific layers via the xattr user.component field.