Merge pull request moby#3972 from tonistiigi/fix-layer-labels

containerimage: keep layer labels for exported images

Author: tonistiigi

client/client_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/content/local"
+	"github.com/containerd/containerd/content/proxy"
 	ctderrdefs "github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/namespaces"
@@ -182,6 +183,7 @@ func TestIntegration(t *testing.T) {
 		testExportAnnotations,
 		testExportAnnotationsMediaTypes,
 		testExportAttestations,
+		testExportedImageLabels,
 		testAttestationDefaultSubject,
 		testSourceDateEpochLayerTimestamps,
 		testSourceDateEpochClamp,
@@ -385,6 +387,134 @@ func testHostNetworking(t *testing.T, sb integration.Sandbox) {
 	}
 }
 
+func testExportedImageLabels(t *testing.T, sb integration.Sandbox) {
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	cdAddress := sb.ContainerdAddress()
+	if cdAddress == "" {
+		t.Skip("only supported with containerd")
+	}
+
+	ctx := sb.Context()
+
+	def, err := llb.Image("busybox").Run(llb.Shlexf("echo foo > /foo")).Marshal(ctx)
+	require.NoError(t, err)
+
+	target := "docker.io/buildkit/build/exporter:labels"
+
+	_, err = c.Solve(ctx, def, SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type: ExporterImage,
+				Attrs: map[string]string{
+					"name": target,
+				},
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	client, err := newContainerd(cdAddress)
+	require.NoError(t, err)
+	defer client.Close()
+
+	ctx = namespaces.WithNamespace(ctx, "buildkit")
+
+	img, err := client.GetImage(ctx, target)
+	require.NoError(t, err)
+
+	store := client.ContentStore()
+
+	info, err := store.Info(ctx, img.Target().Digest)
+	require.NoError(t, err)
+
+	dt, err := content.ReadBlob(ctx, store, img.Target())
+	require.NoError(t, err)
+
+	var mfst ocispecs.Manifest
+	err = json.Unmarshal(dt, &mfst)
+	require.NoError(t, err)
+
+	require.Equal(t, 2, len(mfst.Layers))
+
+	hasLabel := func(dgst digest.Digest) bool {
+		for k, v := range info.Labels {
+			if strings.HasPrefix(k, "containerd.io/gc.ref.content.") && v == dgst.String() {
+				return true
+			}
+		}
+		return false
+	}
+
+	// check that labels are set on all layers and config
+	for _, l := range mfst.Layers {
+		require.True(t, hasLabel(l.Digest))
+	}
+	require.True(t, hasLabel(mfst.Config.Digest))
+
+	err = c.Prune(sb.Context(), nil, PruneAll)
+	require.NoError(t, err)
+
+	// layer should not be deleted
+	_, err = store.Info(ctx, mfst.Layers[1].Digest)
+	require.NoError(t, err)
+
+	err = client.ImageService().Delete(ctx, target, images.SynchronousDelete())
+	require.NoError(t, err)
+
+	// layers should be deleted
+	_, err = store.Info(ctx, mfst.Layers[1].Digest)
+	require.Error(t, err)
+	require.True(t, errors.Is(err, ctderrdefs.ErrNotFound))
+
+	// config should be deleted
+	_, err = store.Info(ctx, mfst.Config.Digest)
+	require.Error(t, err)
+	require.True(t, errors.Is(err, ctderrdefs.ErrNotFound))
+
+	// buildkit contentstore still has the layer because it is multi-ns
+	bkstore := proxy.NewContentStore(c.ContentClient())
+
+	// layer should be deleted as not kept by history
+	_, err = bkstore.Info(ctx, mfst.Layers[1].Digest)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "not found")
+
+	// config should still be there
+	_, err = bkstore.Info(ctx, img.Metadata().Target.Digest)
+	require.NoError(t, err)
+
+	_, err = bkstore.Info(ctx, mfst.Config.Digest)
+	require.NoError(t, err)
+
+	cl, err := c.ControlClient().ListenBuildHistory(sb.Context(), &controlapi.BuildHistoryRequest{
+		EarlyExit: true,
+	})
+	require.NoError(t, err)
+
+	for {
+		resp, err := cl.Recv()
+		if err == io.EOF {
+			break
+		}
+		require.NoError(t, err)
+		_, err = c.ControlClient().UpdateBuildHistory(sb.Context(), &controlapi.UpdateBuildHistoryRequest{
+			Ref:    resp.Record.Ref,
+			Delete: true,
+		})
+		require.NoError(t, err)
+	}
+
+	// now everything should be deleted
+	_, err = bkstore.Info(ctx, img.Metadata().Target.Digest)
+	require.Error(t, err)
+
+	_, err = bkstore.Info(ctx, mfst.Config.Digest)
+	require.Error(t, err)
+}
+
 // #877
 func testExportBusyboxLocal(t *testing.T, sb integration.Sandbox) {
 	c, err := New(sb.Context(), sb.Address())

exporter/containerimage/writer.go

@@ -376,9 +376,10 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, opts *Ima
 		"containerd.io/gc.ref.content.0": configDigest.String(),
 	}
 
-	for _, desc := range remote.Descriptors {
+	for i, desc := range remote.Descriptors {
 		desc.Annotations = RemoveInternalLayerAnnotations(desc.Annotations, opts.OCITypes)
 		mfst.Layers = append(mfst.Layers, desc)
+		labels[fmt.Sprintf("containerd.io/gc.ref.content.%d", i+1)] = desc.Digest.String()
 	}
 
 	mfstJSON, err := json.MarshalIndent(mfst, "", "  ")

solver/llbsolver/history.go

@@ -4,10 +4,12 @@ import (
 	"bufio"
 	"context"
 	"encoding/binary"
+	"encoding/json"
 	"io"
 	"os"
 	"sort"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -142,7 +144,7 @@ func (h *HistoryQueue) migrateV2() error {
 			recs2 := make([]leases.Resource, 0, len(recs))
 			for _, r := range recs {
 				if r.Type == "content" {
-					if ok, err := h.migrateBlobV2(ctx, r.ID); err != nil {
+					if ok, err := h.migrateBlobV2(ctx, r.ID, false); err != nil {
 						return err
 					} else if ok {
 						recs2 = append(recs2, r)
@@ -185,11 +187,56 @@ func (h *HistoryQueue) migrateV2() error {
 	return nil
 }
 
-func (h *HistoryQueue) migrateBlobV2(ctx context.Context, id string) (bool, error) {
+func (h *HistoryQueue) blobRefs(ctx context.Context, dgst digest.Digest, detectSkipLayer bool) ([]digest.Digest, error) {
+	info, err := h.opt.ContentStore.Info(ctx, dgst)
+	if err != nil {
+		return nil, err // allow missing blobs
+	}
+	var out []digest.Digest
+	layers := map[digest.Digest]struct{}{}
+	if detectSkipLayer {
+		dt, err := content.ReadBlob(ctx, h.opt.ContentStore, ocispecs.Descriptor{
+			Digest: dgst,
+		})
+		if err != nil {
+			return nil, err
+		}
+		var mfst ocispecs.Manifest
+		if err := json.Unmarshal(dt, &mfst); err != nil {
+			return nil, err
+		}
+		for _, l := range mfst.Layers {
+			layers[l.Digest] = struct{}{}
+		}
+	}
+	for k, v := range info.Labels {
+		if !strings.HasPrefix(k, "containerd.io/gc.ref.content.") {
+			continue
+		}
+		dgst, err := digest.Parse(v)
+		if err != nil {
+			continue
+		}
+		if _, ok := layers[dgst]; ok {
+			continue
+		}
+		out = append(out, dgst)
+	}
+	return out, nil
+}
+
+func (h *HistoryQueue) migrateBlobV2(ctx context.Context, id string, detectSkipLayers bool) (bool, error) {
 	dgst, err := digest.Parse(id)
 	if err != nil {
 		return false, err
 	}
+
+	refs, _ := h.blobRefs(ctx, dgst, detectSkipLayers) // allow missing blobs
+	labels := map[string]string{}
+	for i, r := range refs {
+		labels["containerd.io/gc.ref.content."+strconv.Itoa(i)] = r.String()
+	}
+
 	w, err := content.OpenWriter(ctx, h.hContentStore, content.WithDescriptor(ocispecs.Descriptor{
 		Digest: dgst,
 	}), content.WithRef("history-migrate-"+id))
@@ -207,9 +254,14 @@ func (h *HistoryQueue) migrateBlobV2(ctx context.Context, id string) (bool, erro
 		return false, nil // allow skipping
 	}
 	defer ra.Close()
-	if err := content.Copy(ctx, w, &reader{ReaderAt: ra}, 0, dgst); err != nil {
+	if err := content.Copy(ctx, w, &reader{ReaderAt: ra}, 0, dgst, content.WithLabels(labels)); err != nil {
 		return false, err
 	}
+
+	for _, refs := range refs {
+		h.migrateBlobV2(ctx, refs.String(), detectSkipLayers) // allow missing blobs
+	}
+
 	return true, nil
 }
 
@@ -307,7 +359,7 @@ func (h *HistoryQueue) leaseID(id string) string {
 	return "ref_" + id
 }
 
-func (h *HistoryQueue) addResource(ctx context.Context, l leases.Lease, desc *controlapi.Descriptor) error {
+func (h *HistoryQueue) addResource(ctx context.Context, l leases.Lease, desc *controlapi.Descriptor, detectSkipLayers bool) error {
 	if desc == nil {
 		return nil
 	}
@@ -318,7 +370,7 @@ func (h *HistoryQueue) addResource(ctx context.Context, l leases.Lease, desc *co
 				return err
 			}
 			defer release(ctx)
-			ok, err := h.migrateBlobV2(ctx, string(desc.Digest))
+			ok, err := h.migrateBlobV2(ctx, string(desc.Digest), detectSkipLayers)
 			if err != nil {
 				return err
 			}
@@ -467,28 +519,28 @@ func (h *HistoryQueue) update(ctx context.Context, rec controlapi.BuildHistoryRe
 			}
 		}()
 
-		if err := h.addResource(ctx, l, rec.Logs); err != nil {
+		if err := h.addResource(ctx, l, rec.Logs, false); err != nil {
 			return err
 		}
-		if err := h.addResource(ctx, l, rec.Trace); err != nil {
+		if err := h.addResource(ctx, l, rec.Trace, false); err != nil {
 			return err
 		}
 		if rec.Result != nil {
-			if err := h.addResource(ctx, l, rec.Result.Result); err != nil {
+			if err := h.addResource(ctx, l, rec.Result.Result, true); err != nil {
 				return err
 			}
 			for _, att := range rec.Result.Attestations {
-				if err := h.addResource(ctx, l, att); err != nil {
+				if err := h.addResource(ctx, l, att, false); err != nil {
 					return err
 				}
 			}
 		}
 		for _, r := range rec.Results {
-			if err := h.addResource(ctx, l, r.Result); err != nil {
+			if err := h.addResource(ctx, l, r.Result, true); err != nil {
 				return err
 			}
 			for _, att := range r.Attestations {
-				if err := h.addResource(ctx, l, att); err != nil {
+				if err := h.addResource(ctx, l, att, false); err != nil {
 					return err
 				}
 			}