Merge pull request #885 from tonistiigi/content-gc

content: fix setting wrong gc labels on config query

frontend/dockerfile/dockerfile_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/containerd/containerd/platforms"
+	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/continuity/fs/fstest"
 	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/client/llb"
@@ -49,6 +50,7 @@ var allTests = []integration.Test{
 	testExportedHistory,
 	testExposeExpansion,
 	testUser,
+	testCacheReleased,
 	testDockerignore,
 	testDockerignoreInvalid,
 	testDockerfileFromGit,
@@ -212,6 +214,42 @@ WORKDIR /
 	require.Equal(t, true, fi.IsDir())
 }
 
+func testCacheReleased(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	checkAllReleasable(t, c, sb, true)
+}
+
 func testSymlinkedDockerfile(t *testing.T, sb integration.Sandbox) {
 	f := getFrontend(t, sb)
 
@@ -3568,6 +3606,86 @@ func checkAllRemoved(t *testing.T, c *client.Client, sb integration.Sandbox) {
 	}
 }
 
+func checkAllReleasable(t *testing.T, c *client.Client, sb integration.Sandbox, checkContent bool) {
+	retries := 0
+loop0:
+	for {
+		require.True(t, 20 > retries)
+		retries++
+		du, err := c.DiskUsage(context.TODO())
+		require.NoError(t, err)
+		for _, d := range du {
+			if d.InUse {
+				time.Sleep(500 * time.Millisecond)
+				continue loop0
+			}
+		}
+		break
+	}
+
+	err := c.Prune(context.TODO(), nil, client.PruneAll)
+	require.NoError(t, err)
+
+	du, err := c.DiskUsage(context.TODO())
+	require.NoError(t, err)
+	require.Equal(t, 0, len(du))
+
+	// examine contents of exported tars (requires containerd)
+	var cdAddress string
+	if cd, ok := sb.(interface {
+		ContainerdAddress() string
+	}); !ok {
+		return
+	} else {
+		cdAddress = cd.ContainerdAddress()
+	}
+
+	// TODO: make public pull helper function so this can be checked for standalone as well
+
+	client, err := newContainerd(cdAddress)
+	require.NoError(t, err)
+	defer client.Close()
+
+	ctx := namespaces.WithNamespace(context.Background(), "buildkit")
+	snapshotService := client.SnapshotService("overlayfs")
+
+	retries = 0
+	for {
+		count := 0
+		err = snapshotService.Walk(ctx, func(context.Context, snapshots.Info) error {
+			count++
+			return nil
+		})
+		require.NoError(t, err)
+		if count == 0 {
+			break
+		}
+		require.True(t, 20 > retries)
+		retries++
+		time.Sleep(500 * time.Millisecond)
+	}
+
+	if !checkContent {
+		return
+	}
+
+	retries = 0
+	for {
+		count := 0
+		err = client.ContentStore().Walk(ctx, func(content.Info) error {
+			count++
+			return nil
+		})
+		require.NoError(t, err)
+		if count == 0 {
+			break
+		}
+		require.True(t, 20 > retries)
+		retries++
+		time.Sleep(500 * time.Millisecond)
+	}
+}
+
 func newContainerd(cdAddress string) (*containerd.Client, error) {
 	return containerd.New(cdAddress, containerd.WithTimeout(60*time.Second))
 }

snapshot/containerd/content.go

@@ -82,10 +82,13 @@ func (c *nsContent) writer(ctx context.Context, retries int, opts ...content.Wri
 			return nil, err
 		}
 	}
+	_, noRoot := wOpts.Desc.Annotations["buildkit/noroot"]
+	delete(wOpts.Desc.Annotations, "buildkit/noroot")
+	opts = append(opts, content.WithDescriptor(wOpts.Desc))
 	ctx = namespaces.WithNamespace(ctx, c.ns)
 	w, err := c.Store.Writer(ctx, opts...)
 	if err != nil {
-		if errdefs.IsAlreadyExists(err) && wOpts.Desc.Digest != "" && retries > 0 {
+		if !noRoot && errdefs.IsAlreadyExists(err) && wOpts.Desc.Digest != "" && retries > 0 {
 			_, err2 := c.Update(ctx, content.Info{
 				Digest: wOpts.Desc.Digest,
 				Labels: map[string]string{

util/imageutil/config.go

@@ -63,7 +63,7 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co
 	}
 
 	handlers := []images.Handler{
-		remotes.FetchHandler(cache, fetcher),
+		fetchWithoutRoot(remotes.FetchHandler(cache, fetcher)),
 		childrenConfigHandler(cache, platform),
 	}
 	if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {
@@ -82,6 +82,16 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co
 	return desc.Digest, dt, nil
 }
 
+func fetchWithoutRoot(fetch images.HandlerFunc) images.HandlerFunc {
+	return func(ctx context.Context, desc specs.Descriptor) ([]specs.Descriptor, error) {
+		if desc.Annotations == nil {
+			desc.Annotations = map[string]string{}
+		}
+		desc.Annotations["buildkit/noroot"] = "true"
+		return fetch(ctx, desc)
+	}
+}
+
 func childrenConfigHandler(provider content.Provider, platform platforms.MatchComparer) images.HandlerFunc {
 	return func(ctx context.Context, desc specs.Descriptor) ([]specs.Descriptor, error) {
 		var descs []specs.Descriptor