-
Notifications
You must be signed in to change notification settings - Fork 3.8k
ctr: add EROFS image conversion support #12555
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
base: main
Are you sure you want to change the base?
Conversation
|
cc @hsiangkao |
|
I think the main consideration here is still how to represent mediatype, which is similar to another PR: #12506. -- We could use some non-OCI mediatypes if containerd allows them, much like the old Although there is already a PR related to this, I don't think EROFS's near-term roadmap has to be tied to OCI mediatypes, and I don't want to speak of SquashFS because whether EROFS succeeds or not in this area shouldn't be involved with other unrelated stuff (It took me a lot of time to develop several kernel features [file-backed mounts, fsmerge, external devices, etc.] in order to make EROFS suitable for both container/VM workloads). |
Cc @dmcgowan. I think after EROFS native images become successful, it will be more convincing to standardize them in OCI or other organizations, rather than just having endless theoretical discussions in meetings. |
6b26370 to
f9ebd52
Compare
|
I have created a PR to define the erofs media type: #12567 |
f9ebd52 to
9f53b42
Compare
core/images/converter/erofs/erofs.go
Outdated
| } | ||
| log.G(ctx).Debugf("converted %s to EROFS using tar index mode", desc.Digest) | ||
| } else { | ||
| if err := erofsutils.ConvertTarErofs(ctx, sr, blobPath, "", convertOpts.mkfsExtraOpts); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use erofs applier directly instead of calling erofsutils.ConvertTarErofs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reusing erofsDiff.Apply is inconvenient:
- It forces output to
layer.erofsinside a mount-resolved directory. Since the converter works with temp files/streams, we'd have to hack together a fake mount.Mount object just to make it work. - It pulls in content.Store, which isn't strictly necessary for just converting streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My real concern is that we should apply the differ's mkfs_options by default (unless users use an additional convertOpts.mkfsExtraOpts to overlap), and we should use the same UUID calculation: u := uuid.NewSHA1(uuid.NameSpaceURL, []byte("erofs:blobs/"+desc.Digest))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at least you'd better to align the UUID calculation, I'm fine with no differ tangle.
b2ebabd to
95e8258
Compare
428e32c to
e6b6784
Compare
|
since there is no compress-hint files, I think you could just leave: as an example. |
Add EROFS conversion support to ctr convert command with configurable options for tar-index mode and mkfs parameters. Usage: ctr image convert --erofs src:tag dst:tag ctr image convert --erofs --erofs-compression='lz4hc,12' src:tag dst:tag Signed-off-by: ChengyuZhu6 <[email protected]>
| }, | ||
| &cli.StringFlag{ | ||
| Name: "erofs-mkfs-opts", | ||
| Usage: "Extra options for mkfs.erofs (e.g., '-zlz4')", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
erofs-mkfs-options: Extra mkfs options applied when converting EROFS layers. (e.g. '-Efragments,dedupe')
| }, | ||
| &cli.StringFlag{ | ||
| Name: "erofs-compression", | ||
| Usage: "Compression algorithms for EROFS, separated by colon (e.g., 'lz4:lz4hc,12')", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just Specify compression algorithm list when converting EROFS layers,
no need to give an example since compress-hint file is unsupported for now.
| e.g., 'ctr convert --uncompress --oci example.com/foo:orig example.com/foo:converted' | ||
| 'ctr convert --erofs example.com/foo:orig example.com/foo:erofs' | ||
| 'ctr convert --erofs --erofs-compression='lzma:lz4hc,12' example.com/foo:orig example.com/foo:erofs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctr convert --erofs --erofs-compression='lz4hc,12' example.com/foo:orig example.com/foo:erofs
Add EROFS conversion support to ctr convert command with configurable options for mkfs parameters.
Usage:
ctr image convert --erofs src:tag dst:tag
ctr image convert --erofs --erofs-compression='lz4hc,12' src:tag dst:tag
Depends-on: #12567