exporter: support unpack opt for image exporter

[fuweid]

It is enhancement which allows to unpack image into containerd
snapshotter storage by --output type=image,<.>=<.>,unpack=true.

In order to support this feature, we needs to extend the Snapshotter
witwh Name() string function. Because we needs to set gc label for
snapshotter which need snapshotter name.

fix: #908

Signed-off-by: Wei Fu fuweid89@gmail.com

[fuweid]

#12 [validate 1/1] RUN --mount=target=.,rw   git add -A &&   rm -rf vendor &...
#12       digest: sha256:4a15b553b368237a127fe5e2ace8f18449882491dd16389e6ba3bdddbbd2f04e
#12         name: "[validate 1/1] RUN --mount=target=.,rw   git add -A &&   rm -rf vendor &&   cp -rf /out/* . &&   ./hack/validate-vendor check"
#12      started: 2019-03-28 13:27:58.859174333 +0000 UTC
#12 0.599 The result of "make vendor" differs
#12 0.599 
#12 0.599  M vendor/modules.txt
#12 0.599 
#12 0.599 Please vendor your package with "make vendor"
#12 0.599 
#12    completed: 2019-03-28 13:27:59.509113448 +0000 UTC
#12     duration: 649.939115ms
#12        error: "executor failed running [/bin/sh -c git add -A &&   rm -rf vendor &&   cp -rf /out/* . &&   ./hack/validate-vendor check]: exit code: 1"
error: failed to solve: rpc error: code = Unknown desc = executor failed running [/bin/sh -c git add -A &&   rm -rf vendor &&   cp -rf /out/* . &&   ./hack/validate-vendor check]: exit code: 1
make: *** [validate-vendor] Error 1
The command "make binaries validate-all && PLATFORMS="${PLATFORMS},darwin/amd64,windows/amd64" ./hack/cross" exited with 2.

is it related to environment?

[tonistiigi]

@fuweid run ./hack/update-vendor to fix

[fuweid]

@tonistiigi thanks!

diff --git a/vendor/modules.txt b/vendor/modules.txt
index b035a20..cfa19d0 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -46,13 +46,13 @@ github.com/containerd/containerd/oci
 github.com/containerd/containerd/containers
 github.com/containerd/containerd/namespaces
 github.com/containerd/containerd/errdefs
+github.com/containerd/containerd/rootfs
 github.com/containerd/containerd/images/oci
 github.com/containerd/containerd/api/services/content/v1
 github.com/containerd/containerd/content/proxy
 github.com/containerd/containerd/services/content/contentserver
 github.com/containerd/containerd/reference
 github.com/containerd/containerd/remotes/docker/schema1
-github.com/containerd/containerd/rootfs
 github.com/containerd/containerd/images/archive
 github.com/containerd/containerd/archive
 github.com/containerd/containerd/archive/compression

but this diff is confusing me 😢

[tonistiigi]

@fuweid I think go mod might put packages in import order and that changed but not sure.

[fuweid]

@fuweid I think go mod might put packages in import order and that changed but not sure.

@tonistiigi got it. will check this.

The CI is pass, please take a look. But I find a flaky case here:

--- FAIL: TestIntegration/TestImportExportReproducibleIDs/worker=containerd-1.0/frontend=client (1.82s)

        require.go:157: 

            	Error Trace:	dockerfile_test.go:3327

            	            				run.go:172

            	Error:      	Not equal: 

            	            	expected: v1.Descriptor{MediaType:"application/vnd.docker.distribution.manifest.v2+json", Digest:"sha256:40135e786634892a225a37ec4650593ebbfcbc6631996a638d282198b18958a6", Size:941, URLs:[]string(nil), Annotations:map[string]string(nil), Platform:(*v1.Platform)(nil)}

            	            	actual  : v1.Descriptor{MediaType:"application/vnd.docker.distribution.manifest.v2+json", Digest:"sha256:93dfc4ffa2c9b5785707ebcbcab1ce8f1a6039495d41bd1d81e3956ca9440fe1", Size:941, URLs:[]string(nil), Annotations:map[string]string(nil), Platform:(*v1.Platform)(nil)}

            	            	

            	            	Diff:

            	            	--- Expected

            	            	+++ Actual

            	            	@@ -2,3 +2,3 @@

            	            	  MediaType: (string) (len=52) "application/vnd.docker.distribution.manifest.v2+json",

            	            	- Digest: (digest.Digest) (len=71) sha256:40135e786634892a225a37ec4650593ebbfcbc6631996a638d282198b18958a6,

            	            	+ Digest: (digest.Digest) (len=71) sha256:93dfc4ffa2c9b5785707ebcbcab1ce8f1a6039495d41bd1d81e3956ca9440fe1,

            	            	  Size: (int64) 941,

            	Test:       	TestIntegration/TestImportExportReproducibleIDs/worker=containerd-1.0/frontend=client

        oci.go:123: stderr: /usr/bin/buildkitd

[fuweid]

any update here?

[tonistiigi]

LGTM but can you add a simple test(or modify an existing one to include this). Don't necessarily need to run the image, but at least make sure that we don't accidentally make the build fail when unpack is set.

[fuweid]

sure. will do it.

[tonistiigi]

@fuweid Need rebase as well after the userns update (sorry)

[fuweid]

@tonistiigi thanks for reminder. Still working on it. will update it.

[fuweid]

ping @tonistiigi I modify the existing buildkit cmd case, PTAL. Thanks

---

Hm.. I don't know why the CI is pass right now.😂 1 Hour ago, I have the issue like

--- FAIL: TestClientIntegration/TestBasicInlineCacheImportExport/worker=oci-rootless (1.35s)
        require.go:794: 
            	Error Trace:	client_test.go:1718
            	            				run.go:172
            	Error:      	Received unexpected error:
            	            	rpc error: code = Unknown desc = failed to copy: httpReaderSeeker: failed open: could not fetch content descriptor sha256:40f7c94fc7cf8e4e2488730dae8aee2eefcb28024cb41ed0fa9f6543c8420d44 (application/vnd.docker.container.image.v1+json) from remote: not found
            	            	failed to solve
            	            	github.com/moby/buildkit/client.(*Client).solve.func2
            	            		/src/client/solve.go:189
            	            	golang.org/x/sync/errgroup.(*Group).Go.func1
            	            		/src/vendor/golang.org/x/sync/errgroup/errgroup.go:58
            	            	runtime.goexit
            	            		/usr/local/go/src/runtime/asm_amd64.s:1337
            	Test:       	TestClientIntegration/TestBasicInlineCacheImportExport/worker=oci-rootless
        oci.go:123: stderr: /usr/bin/sudo

is it good to use dummy@sha256:40f7c94fc7cf8e4e2488730dae8aee2eefcb28024cb41ed0fa9f6543c8420d44 as image name here? I think it should be 404 here. could we use registry target cache here?

diff --git a/client/client_test.go b/client/client_test.go
index e7e4c3f..12811ea 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -1714,7 +1714,7 @@ func testBasicInlineCacheImportExport(t *testing.T, sb integration.Sandbox) {
        require.Equal(t, ok, true)

        // dgst3 != dgst, because inline cache is not exported for dgst3
-       unique3, err := readFileInImage(c, "dummy@"+dgst3, "/unique")
+       unique3, err := readFileInImage(c, target, "/unique")
        require.NoError(t, err)
        require.EqualValues(t, unique, unique3)
 }

cc @AkihiroSuda

[fuweid]

I know why that CI can pass by retry. The containerd content is removed by gc. It might be delay. If the real content isn't removed by gc before fetch dummy@sha256:40f7c94fc7cf8e4e2488730dae8aee2eefcb28024cb41ed0fa9f6543c8420d44, the fetcher will OpenWriter to check the commit exists or not.

// remotes/handlers.go
func fetch(ctx context.Context, ingester content.Ingester, fetcher Fetcher, desc ocispec.Descriptor) error {
        log.G(ctx).Debug("fetch")

        cw, err := content.OpenWriter(ctx, ingester, content.WithRef(MakeRefKey(ctx, desc)), content.WithDescriptor(desc))
        if err != nil {
                if errdefs.IsAlreadyExists(err) {
                        return nil
                }
                return err
        }
....

If the content is still there, the fetcher will not file request to registry. Therefore, the CI can pass. 😂
If not, oops, like

Here is log.

[tonistiigi]

Are you saying that the test does not do what is expected at all, or are you pointing to a race when the second build happens on the same time with gc? I couldn't reproduce this locally but I can see that the second build does do fetches. Also added a sleep to make sure gc has run. If latter, then do you have an idea where the race is, opening a writer with errexist should block the gc from deleting the data even if it was marked for removal.

[tonistiigi]

@fuweid Is this ready from your side or do you want to investigate the test flakiness above more?

[fuweid]

hi @tonistiigi , I still want to investigate the flaky test case. When I am ready, I will ping you. Thanks!

[fuweid]

@tonistiigi @AkihiroSuda I cannot reproduce this flaky case.

In my option, when we call the Prune function, the cache will remove the content related to snapshot. But the gc is controled by the throttle and async.

If the content isn't removed by gc, the next round Solve can reuse the content, including pulling image. For the failed case, the registry cache importer can use the marked deleted content. If my understanding is correct, the importer seems that it doesn't change the gc label, which means that the incoming gc still can remove the content. If the gc is done before readFileInImage.Solve, the case will fail.

However, I cannot reproduce this. 😂 If it is not blocked issue, please review the PR. Thanks.