-
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
Add converter package for tar build #34
Conversation
Introduce a new nydus image build mode named tar build. In order to correspond to the OCI image layer, we need a new build mode, that is to convert the OCI-formatted tar stream directly into a nydus-formatted tar stream. The nydus tar stream contains blob and bootstrap files, and the workflow is divided into two steps, Convert and Merge. Convert: converts each layer of the OCI tar stream to a nydus tar stream. Merge: reads the bootstrap files from each layer of the nydus tar stream and merges them into a final bootstrap. This package will be imported by acceleration-service and buildkit to support the conversion or build of nydus image. Signed-off-by: Yan Song <[email protected]>
d2d8f2d
to
bb373bc
Compare
3f86b5a
to
9fb4dfe
Compare
Waiting for dragonflyoss/nydus#351 to be merged, then we can continue to resolve CI issue. |
Implement io.Seeker and io.Reader for io.ReaderAt, so that let tar unpacking skip blob data fragment, then seek to the start of bootstrap data fragment directly. Signed-off-by: Yan Song <[email protected]>
Enable `WithTar` option to put bootstrap into a tar stream (no gzip), so that we can make the bootstrap as an image layer. Signed-off-by: Yan Song <[email protected]>
So that importing converter packge in buildkit without ambiguity. Signed-off-by: Yan Song <[email protected]>
63bfb06
to
46bcd1d
Compare
Codecov Report
@@ Coverage Diff @@
## main #34 +/- ##
==========================================
+ Coverage 34.46% 36.07% +1.60%
==========================================
Files 18 20 +2
Lines 1294 1519 +225
==========================================
+ Hits 446 548 +102
- Misses 777 867 +90
- Partials 71 104 +33
Continue to review full report at Codecov.
|
Moving converter package test to smoke test suite, make ut testing cleaner. Signed-off-by: Yan Song <[email protected]>
8b2be10
to
7ff301d
Compare
e000df6
to
5d24120
Compare
The converter package needs use sudo to access privileged xattr of file (containerd apply oci whiteout file to overlayfs whiteout). Signed-off-by: Yan Song <[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.
Generally lgtm. A few comments inline.
pkg/converter/tool/builder.go
Outdated
// Add blob offset for chunk info with size_of(tar_header) * 2. | ||
"1024", | ||
} | ||
if option.RafsVersion != "" { |
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.
args cannot work if RafsVersion
is empty. Do you mean to disable check for rafs v6? Also IMO it is better to expose the checker option to users/callers instead of hardcoding it here.
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.
Fixed, added RafsVersion
option to ConvertOption
.
pkg/converter/tool/nydusd.go
Outdated
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
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.
Should this live in pkg/nydussdk
instead?
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.
Only use it in converter tests, moved to tests/nydusd.go
.
pkg/converter/tool/nydusd.go
Outdated
"threads_count": 10, | ||
"merging_size": 131072 | ||
}, | ||
"digest_validate": true, |
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.
If it is to be used during build process, I'd suggest that we disable digest validation since it affects performance a lot. For tests, we can just enable it for sure. Or are you planning to add an option to set it later on?
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.
Only use it in converter tests, moved to tests/nydusd.go
.
Signed-off-by: Yan Song <[email protected]>
Support user specified rafs version when call Convert(). Signed-off-by: Yan Song <[email protected]>
Introduce a new nydus image build mode named tar build.
In order to correspond to the OCI image layer, we need a new build mode, that is
to convert the OCI-formatted tar stream directly into a nydus-formatted tar stream.
The nydus tar stream contains blob and bootstrap files, and the workflow is divided
into two steps, Convert and Merge.
Convert: converts each layer of the OCI tar stream to a nydus tar stream.
Merge: reads the bootstrap files from each layer of the nydus tar stream and merges
them into a final bootstrap.
This package will be imported by acceleration-service and buildkit to support the
conversion or build of nydus image.
Depends on merge sub-command in builder(nydus-image).
Related PR: moby/buildkit#3136, moby/buildkit#2581
Signed-off-by: Yan Song [email protected]