Skip to content
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

Builder: support tar build #351

Merged
merged 12 commits into from
Apr 7, 2022
Merged

Conversation

imeoer
Copy link
Collaborator

@imeoer imeoer commented Mar 25, 2022

Introduce a new build mode named tar build in nydusify and builder (nydus-image).

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.

Support merge sub-command in builder, it will be used in converter package, and this
package will be imported by acceleration-service and buildkit to support the conversion
or build of nydus image.

@imeoer imeoer force-pushed the nydus-tar-build branch 6 times, most recently from 90799d1 to 64394b8 Compare March 25, 2022 09:30
@imeoer imeoer changed the title Builder: support tar build [WIP] Builder: support tar build Mar 25, 2022
@imeoer imeoer force-pushed the nydus-tar-build branch 2 times, most recently from 0b40a9f to 226b58c Compare March 28, 2022 02:32
@imeoer imeoer changed the title [WIP] Builder: support tar build Builder: support tar build Mar 28, 2022
@imeoer imeoer force-pushed the nydus-tar-build branch 2 times, most recently from 64b0a70 to 4286bcf Compare March 31, 2022 03:35
@imeoer imeoer force-pushed the nydus-tar-build branch 4 times, most recently from 9c05f17 to a923990 Compare April 2, 2022 06:55
Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Some(_) => {
// Insert removal operations at the head, so they will be handled first when
// applying to lower layer.
nodes.insert(0, node);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits: Insert whiteouts at the head, xxxx

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

flags = Some(current_flags);
}

let parent_blobs = rs.superblock.get_blob_infos();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply bail if len(parent_blobs) > 1? Then the parent_blob_added logic can be dropped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is assumed that the nydus-image create at each layer and nydus-image merge commands use the same chunk dict bootstrap, so there is at most one blob from the parent bootstrap and all other blobs from the chunk dict bootstrap. Added a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more detailed comment.


// Blob list recorded in manifest annotation should be equal with
// the blob list recorded in blob table of bootstrap
if !reflect.DeepEqual(bootstrap.Blobs, blobListInLayer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still validate that all the blobs in bootstrap are in the image manifest, to make sure we don't reference any non-existing blobs in bootstrap.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! fixed.

@imeoer imeoer force-pushed the nydus-tar-build branch 2 times, most recently from d3629cb to 12fd452 Compare April 6, 2022 11:41
Add `--whiteout-spec none` option for builer command, which will
build all `.wh.*` and `.wh..wh..opq` files into bootstrap.

This is used for tar build, packing blob and bootstrap into a tar
stream, so that correspond to the OCI tar layer one by one.

Signed-off-by: Yan Song <[email protected]>
Match blob table by fs version in `boostrap.dump()`.

Signed-off-by: Yan Song <[email protected]>
The `merge` sub-command merges multiple nydus boostraps (from every layer
of image) to a final boostrap.

Signed-off-by: Yan Song <[email protected]>
Nydus tar build converts a OCI formatted tar stream to a nydus formatted tar stream.

The nydus blob tar stream contains blob and bootstrap files with the following
file tree structure:

/image
├── image.blob
├── image.boot

So for the chunk of files in the nydus boostreap, a blob compressed offset
of 1024 (size_of(tar_header) * 2) is required.

Add `--blob-offset` option to builder to support this case.

Signed-off-by: Yan Song <[email protected]>
Add `--chunk-dict` option support for merge sub-command,
We need get blob list from chunk-dict bootstrap to fill
blob table in final bootstrap.

Signed-off-by: Yan Song <[email protected]>
Add `--prefetch-policy` option support for merge sub-command, this
option has the same function as in the create sub-command, it is
used to enable prefetch to optimize inode/chunk layout.

Signed-off-by: Yan Song <[email protected]>
Set node's layer index to distinguish same inode number (from bootstrap)
between different layers.

Signed-off-by: Yan Song <[email protected]>
1. reuse options with create sub-command;
2. add context error message for user friendly;

Signed-off-by: Yan Song <[email protected]>
For the tar build scene, the regular layer represents blob+bootstrap,
for an empty data layer (for example `chmod +x`), the blob table in
bootstrap doesn't include the blob entry of this layer, so we need
remove the check of comparing the number of blobs and the blob order
between bootstrap and layers on the manifest, just ensure nydus blobs
in the blob table of bootstrap should all appear in the layers of manifest.

Signed-off-by: Yan Song <[email protected]>
Keep final bootstrap following superblock flags of source bootstraps.

Signed-off-by: Yan Song <[email protected]>
The behavior change of RafsSuper::walk_inodes api break diff
build, adjust it to ignore non-regular file handle.

Signed-off-by: Yan Song <[email protected]>
Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bergwolf bergwolf merged commit 947a13c into dragonflyoss:master Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants