-
Notifications
You must be signed in to change notification settings - Fork 100
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
Refine tarfs overall design and implementation details #499
Conversation
ff33840
to
2795e50
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #499 +/- ##
==========================================
- Coverage 37.68% 37.55% -0.13%
==========================================
Files 60 60
Lines 7115 7139 +24
==========================================
Hits 2681 2681
- Misses 4122 4146 +24
Partials 312 312
|
With this PR applied:
|
a7dad0d
to
7edc269
Compare
Configuration for above test:
|
8067384
to
2eec840
Compare
e1ef00f
to
a89976f
Compare
f77d93b
to
4e0db99
Compare
Tested with some top images on dockerhub, compare rootfs generated by tarfs and overlayfs snapshotter for runc containers, there is no difference. Tested-By Xin Yin |
As this PR is quite large, I tried to review it a few times but they were not easy for me. So please forgive us for the slowwww procedure of reviewing it. :-) |
Maybe add this to a formal doc for the tarfs feature. |
if pm != nil { | ||
switch pm.FsDriver { | ||
case config.FsDriverBlockdev: | ||
fs.blockdevManager = pm |
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.
The manager manages all the nydusd daemons which serve nydus image, but looks like tarfs does not need any nydusd daemon. Why there is a blockdev manager which never abord a nydusd daemon.
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.
Though tarfs doesn't use daemon, it does use Manager to support:
- cache space
- recovery
- metrics
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 we already have a dedicated cache manager which can work alone without a daemon manager.
As tarfs is an erofs mount, nydus-snapshotter does not have to recover any nydus daemon.
Tarfs is running as an Erofs mount without a nydusd daemon, I think there is no metric exported.
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.
Thanks, maybe we need more work to do
4c9d88e
to
34e8534
Compare
Add 'enable_tarfs' option in config.toml, to enable run oci image with erofs tarfs. Snapshotter will download all image layers, uncompress to tar files and generate tarfs boostraps. Finally mount erofs based on loop device as the image read only rootfs. Signed-off-by: Xin Yin <[email protected]>
Add 'tarfs_max_concurrent_proc' option to set number of concurrent goroutines for tarfs blob processing. Signed-off-by: Xin Yin <[email protected]>
Support VIEW for tarfs snapshot. Signed-off-by: Xin Yin <[email protected]>
Syntax only changes: - fix some typos - rename manifest to manifestDigest Signed-off-by: Jiang Liu <[email protected]>
Limit maximum size of image manifest and config, to avoid DoS attack. Signed-off-by: Jiang Liu <[email protected]>
Only caches needed information according to manager configuration. Signed-off-by: Jiang Liu <[email protected]>
1) move loopdev setup from Prepare()/Merge() into Mount(), preparing for support of nodev mode in addition to blockdev. 2) store tar file in the global cache directory, mount tarfs at "mnt" instead of "tarfs", to keep consistence with fusedev/fscache drivers. 3) use tempfile/rename mechanism to easy error recover 4) use lock to avoid some possible race conditions 5) use mode 0750 instead of 0755 for new directories 6) associate a Manager with filesystems managed by blockdev Signed-off-by: Jiang Liu <[email protected]>
Remove function prepareRemoteSnapshot() to simplify the code and improve readability. Signed-off-by: Jiang Liu <[email protected]>
The overlay volatile option is only supported for OCIv1 images, so enhance nydus remote images to support it too. Signed-off-by: Jiang Liu <[email protected]>
Change if else to switch according to golint suggestions. Signed-off-by: Jiang Liu <[email protected]>
Enhance tarfs implementation to support following operations: - export an tarfs image as a block device - export an tarfs image as a block device with verity - export an tarfs layer as a block device - export an tarfs layer as a block device with verity Signed-off-by: Jiang Liu <[email protected]>
Simplify tarfs implementation by: 1) simplify the way to detech images without data blob 2) do not regenerate tarfs data when it already exists Signed-off-by: Jiang Liu <[email protected]>
Add documentation for tarfs. Signed-off-by: Jiang Liu <[email protected]>
Move ExtraOption to dedicated file and extend it for more usage cases. Signed-off-by: Jiang Liu <[email protected]>
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.
LGTM~ Thanks
This PR is based on #497 , and improve tarfs by: