Support for exporting nydus compression type

[imeoer]

Related issue: #2046

Nydus image is a container accelerated image format provided by the Dragonfly image-service project, which offers the ability to pull image data on demand, without waiting for the entire image pull to complete and then start the container. It has been put in production usage and shown vast improvements over the OCI v1 image format in terms of container launching speed, image space, and network bandwidth efficiency, as well as data integrity. Nydus image can be flexibly configured as a FUSE-based user-space filesystem or in-kernel FS with nydus daemon in user-space:

• Lightweight integration with VM-based containers runtime like KataContainers. KataContainers is supported Nydus as a native image acceleration solution.
• Nydus closely cooperates with Linux in-kernel disk filesystem containers' rootfs can directly be set up by EROFS over Fscache with lazy pulling capability. The corresponding changes had been merged into Linux kernel since v5.19.

Nydus has provided a conversion tool Nydusify for converting OCIv1 image to Nydus image and integrated into Harbor Acceld as a conversion driver, which assumes that the OCI image is already available in the registry, but a better way would be to build the Nydus images directly from the build system instead of using the conversion tool, which would increase the speed of the image export, so we experimentally integrated the Nydus export in Buildkit.

A manifest example of nydus image can be found here. Unlike other compression formats (gzip, zstd, eStargz, etc.) in OCI image, nydus is divided into two types of layer, blob and bootstrap, where blob serves as the data and metadata for all files of each layer of the image, and bootstrap serves as the metadata of the whole image, the bootstrap is equivalent to the view of the whole image filesystem after all layers overlay, nydus use this layer form to reduce the number of HTTP request and FUSE overlay's overhead on preparing the container's rootfs. For example, for an OCI image with 3 layers, the corresponding nydus image is 4 layers (3 layers of blob + 1 layer of bootstrap).

The converter package in nydus-snapshotter do the actual layer compression, this PR imports the package to implement the export of nydus compression type.

[imeoer]

@ktock @AkihiroSuda PTAL

[ktock]

BuildKit doesn't seem to be able to use the exported nydus-compressed image and cache.
e.g. the second build in the following with inline cache panics.

# mkdir -p /tmp/ctx
cat <<EOF > /tmp/ctx/Dockerfile
FROM registry2-buildkit:5000/ubuntu:20.04
RUN echo hello > /hello
EOF
# buildctl build --progress=plain --frontend=dockerfile.v0 --local context=/tmp/ctx --local dockerfile=/tmp/ctx \
                 --output type=image,name=registry2-buildkit:5000/ubuntu:20.04-hello,push=true,compression=nydus,oci-mediatypes=true \
                 --export-cache type=inline
# buildctl build --progress=plain --frontend=dockerfile.v0 --local context=/tmp/ctx --local dockerfile=/tmp/ctx \
                 --output type=image,name=registry2-buildkit:5000/ubuntu:20.04-hello-zstd,push=true,compression=zstd,oci-mediatypes=true \
                 --import-cache type=registry,ref=registry2-buildkit:5000/ubuntu:20.04-hello
...
panic: runtime error: index out of range [4] with length 4

goroutine 748 [running]:
github.com/moby/buildkit/cache/remotecache.(*contentCacheImporter).importInlineCache.func1.1()
	/go/src/github.com/moby/buildkit/cache/remotecache/import.go:188 +0xda5
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/go/src/github.com/moby/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:57 +0x67
created by golang.org/x/sync/errgroup.(*Group).Go
	/go/src/github.com/moby/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:54 +0x92

[imeoer]

BuildKit doesn't seem to be able to use the exported nydus-compressed image and cache. e.g. the second build in the following with inline cache panics.

@ktock Thanks for the testing, the panic indeed occurs.

This seems to be a tricky problem. Nydus image consists of multiple blobs and a bootstrap (one more layer than the cache records), and the blob does not correspond to the cache record one by one (probably fewer layers than the cache record).

Is it possible to consider not supporting the inline cache export for nydus image? Will submit a new commit to do this.

[imeoer]

@ktock rebased code to resolve the conflicts with the PR and add workaround for inline export on the commit. PTAL.

[ktock]

Now we seem to have 2 implementation of computeBlobChain(computeBlobChain and computeNydusBlobChain).
Some concerns about maintainability:

• How can guarantee/keep the compatibility of these implementations?
• Will we need 2 duplicated patches on these implementations for 1 futural change?

[AkihiroSuda]

This is likely to break the integration-tests-base on other platforms (arm, s390x, ppc64le, riscv64).

Can we just build nydus in this stage, or could you upload binaries for all the archs?

[imeoer]

Added os/arch check, since nydus has only been tested in production on linux/amd64 and linux/arm64 machines, it is currently only guaranteed to be available on these two platforms, in the future we will add support for other linux archs or macOS, etc, thanks.

Need to depend on some rust components if build in this stage, as said here, how about letting us remove the arch check and keep available on amd64 and arm64 first? nydus will release more arches in the future.

[imeoer]

Will release more other platforms for making sure the integration tests are available as much as possible.

[ktock]

It looks like that this breaks the cache if a conversion from a layer which is tar-ed by non-go implementation into nydus then later another conversion happens from that nydus blob to another compression.

[imeoer]

Thanks for the reply! I want to explain that nydus is not an archive format, if we convert a system tar to nydus and then convert nydus back to tar, although nydus can guarantee filesystem consistency, but it can’t do a complete restore of the system tar, it will change its file entry path, for example, ./foo1 -> foo1 (it lost ./ for file path in the tar header), but nydus makes sure that the decompressed tar is as consistent as possible with the hash of OCI tar format (in go implementation), and does strictly guarantee in filesystem consistency. So can we consider skipping the hash check test case between nydus and system tar first?

[ktock]

Another idea for moving this integration forward: #3037

[imeoer]

Another idea for moving this integration forward: #3037

@ktock Good idea, this proposal might make cleaner for the job of appending an extra bootstrap layer to nydus image, is it a blocker for this PR? Does the cache changes in this PR still available? Maybe we can move this change forward first, and then use the exporter plugin to refactor.

[imeoer]

These are the points that I think are different between nydus and other compression types involved in this PR. Hope we can continue to give some opinions on whether these will block moving on. cc @AkihiroSuda @ktock

1. Nydus compression type can't be mixed with other compression types in the same image, so if cache record is other/nydus layer, but the target is another compression type, need to convert forcibly (force-compression=true).

2. Need to append an extra bootstrap layer to the manifest of nydus image, this layer represents the whole metadata of filesystem view for the entire image, this requires skipping the layer when importing inline cache.

3. Nydus is not an archive format, but a filesystem format, if we convert a system tar to nydus and then convert nydus back to tar, although nydus can guarantee filesystem consistency, but it can’t do a complete restore of the system tar, it will change its file entry path, for example, ./foo1 -> foo1 (it lost ./ for file path in the tar header), but nydus makes sure that the decompressed tar is as consistent as possible with the hash of OCI tar format (in go implementation), and does strictly guarantee in filesystem consistency.

[imeoer]

ping @AkihiroSuda @ktock @tonistiigi ~

[ktock]

Yes, I agree that they are blockers. And another consideration is how can we maintain it (e.g. how can we guarantee the emitted nydus isn't broken and maintain all nydus-related exceptions). #3037 is another approach for solving these issues. #2045 looks like integrates nydus as an exporter implementation so I thought that #3037 helps that approach moving forward.

[bergwolf]

@AkihiroSuda @ktock @imeoer @bergwolf had a meeting discussing this PR and reached the following consensus:

1. We can live with the current three Nydus exceptions in the PR, with proper unit tests to help validate them.
2. We will add go -tags to mark out nydus code to allow non-nydus build (which is the default build mode).
3. Meanwhile, @ktock and @imeoer will work on an exporter plugin framework to support compression types like Nydus.
4. The Nydus team commit to helping maintain Nydus functionality in buildkit, including incremental improvements and bugfixes.

@tonistiigi what do you think of the above? If it sounds ok to you, we'll proceed with the change.

[AkihiroSuda]

(Follow-up: the go tag is the short-term solution; the long term solution is the exporter plugin)

[tonistiigi]

what do you think of the above?

sgtm. Make sure that as much code as possible is in the gotag files and that there aren't many exceptions in the shared files.

[imeoer]

cc @AkihiroSuda @ktock @tonistiigi @bergwolf

Hi, we have moved the nydus codes to the gotag files, please take a look, the following points need to be clarified:

1. the nydus features and related tests will only be run when -tags=nydus is enabled. the unit test for system tar handling by the nydus compression type will still be skipped in manager_test.go, and this part of the code can be removed when migrating to the future exporter plugin.

2. because the nydus image has an additional bootstrap layer, in order to avoid exception changes to the remote cache, nydus does not support cache import/export (maybe we can support it in exporter plugin), the relevant limitations have been added to nydus.md.

[ktock]

If we introduce -tags=nydus, I think we should move all if comp.Type == compression.Nydus and case compression.Nydus into *_nydus.go file.

[imeoer]

Now if -tags=nydus not to be enabled, the nydus related condition branch will return errdefs.ErrNotImplemented in cache/not_nydus.go. Seems it's difficult to put case and if code paths into a separate go file, any suggestions for this?

[imeoer]

Can we still keep the constant compression.Nydus ? If we try to put it into *_nydus.go, we need to reimplement some methods, I think the maintainability can become poor.

[imeoer]

@ktock Adjusted some methods for reducing nydusify package dependency, PTAL.

If we want to completely remove the compression.Nydus dependency when -tags=nydus not be enabled, I think we could implement an interface like this for each compression type:

type Compression interface {
	String() string
	Compress(context.Context, compression.Config) (...)
	Decompress(context.Context, ocispecs.Descriptor) (...)
	NeedsConversion(context.Context, content.Store, ocispecs.Descriptor) (bool, error)
	...
}

The advantage of this is that we can remove switch case code path, and ensure that don't miss any compression.Type handling, WDYT? If possible, I can submit a separate PR to implement it first.

[imeoer]

The advantage of this is that we can remove switch case code path, and ensure that don't miss any compression.Type handling, WDYT? If possible, I can submit a separate PR to implement it first.

@ktock @AkihiroSuda Introduced a new compression.Compression interface in #3136, PTAL.

[imeoer]

@AkihiroSuda @ktock Fixed, PTAL, thanks!

[bergwolf]

@AkihiroSuda @ktock @tonistiigi With #3136 merged, this PR is pretty clean in terms of code invasion in the general code path. Could you give it another spin and see if it is suitable for merging? As discussed earlier, we'll continue improving the nydus functionality in buildkit and we look forward to putting it into large-scale production use.

[imeoer]

@ktock @AkihiroSuda Updated, please help to take a glance again, thanks!

[imeoer]

Thanks for all! cc @tonistiigi Please help to take a final look, thanks!