Skip to content

Commit

Permalink
Merge pull request moby#4210 from tonistiigi/unlazy-missing-blob
Browse files Browse the repository at this point in the history
  • Loading branch information
@jedevc
jedevc authored Sep 26, 2023
2 parents b4471c6 + 5dccc0a commit 5e6497c
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cache/filelist.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (sr *immutableRef) FileList(ctx context.Context, s session.Group) ([]string
}

// lazy blobs need to be pulled first
if err := sr.Extract(ctx, s); err != nil {
if err := sr.ensureLocalContentBlob(ctx, s); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion cache/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (cm *cacheManager) GetByBlob(ctx context.Context, desc ocispecs.Descriptor,

ref := rec.ref(true, descHandlers, nil)
if s := unlazySessionOf(opts...); s != nil {
if err := ref.unlazy(ctx, ref.descHandlers, ref.progress, s, true); err != nil {
if err := ref.unlazy(ctx, ref.descHandlers, ref.progress, s, true, false); err != nil {
return nil, err
}
}
Expand Down
40 changes: 28 additions & 12 deletions cache/refs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,14 @@ func (sr *immutableRef) Mount(ctx context.Context, readonly bool, s session.Grou
return mnt, nil
}

func (sr *immutableRef) ensureLocalContentBlob(ctx context.Context, s session.Group) error {
if (sr.kind() == Layer || sr.kind() == BaseLayer) && !sr.getBlobOnly() {
return nil
}

return sr.unlazy(ctx, sr.descHandlers, sr.progress, s, true, true)
}

func (sr *immutableRef) Extract(ctx context.Context, s session.Group) (rerr error) {
if (sr.kind() == Layer || sr.kind() == BaseLayer) && !sr.getBlobOnly() {
return nil
Expand All @@ -1002,14 +1010,14 @@ func (sr *immutableRef) Extract(ctx context.Context, s session.Group) (rerr erro
if rerr = sr.prepareRemoteSnapshotsStargzMode(ctx, s); rerr != nil {
return
}
rerr = sr.unlazy(ctx, sr.descHandlers, sr.progress, s, true)
rerr = sr.unlazy(ctx, sr.descHandlers, sr.progress, s, true, false)
}); err != nil {
return err
}
return rerr
}

return sr.unlazy(ctx, sr.descHandlers, sr.progress, s, true)
return sr.unlazy(ctx, sr.descHandlers, sr.progress, s, true, false)
}

func (sr *immutableRef) withRemoteSnapshotLabelsStargzMode(ctx context.Context, s session.Group, f func()) error {
Expand Down Expand Up @@ -1149,25 +1157,33 @@ func makeTmpLabelsStargzMode(labels map[string]string, s session.Group) (fields
return
}

func (sr *immutableRef) unlazy(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group, topLevel bool) error {
func (sr *immutableRef) unlazy(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group, topLevel bool, ensureContentStore bool) error {
_, err := g.Do(ctx, sr.ID()+"-unlazy", func(ctx context.Context) (_ struct{}, rerr error) {
if _, err := sr.cm.Snapshotter.Stat(ctx, sr.getSnapshotID()); err == nil {
return struct{}{}, nil
if !ensureContentStore {
return struct{}{}, nil
}
if blob := sr.getBlob(); blob == "" {
return struct{}{}, nil
}
if _, err := sr.cm.ContentStore.Info(ctx, sr.getBlob()); err == nil {
return struct{}{}, nil
}
}

switch sr.kind() {
case Merge, Diff:
return struct{}{}, sr.unlazyDiffMerge(ctx, dhs, pg, s, topLevel)
return struct{}{}, sr.unlazyDiffMerge(ctx, dhs, pg, s, topLevel, ensureContentStore)
case Layer, BaseLayer:
return struct{}{}, sr.unlazyLayer(ctx, dhs, pg, s)
return struct{}{}, sr.unlazyLayer(ctx, dhs, pg, s, ensureContentStore)
}
return struct{}{}, nil
})
return err
}

// should be called within sizeG.Do call for this ref's ID
func (sr *immutableRef) unlazyDiffMerge(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group, topLevel bool) (rerr error) {
func (sr *immutableRef) unlazyDiffMerge(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group, topLevel bool, ensureContentStore bool) (rerr error) {
eg, egctx := errgroup.WithContext(ctx)
var diffs []snapshot.Diff
sr.layerWalk(func(sr *immutableRef) {
Expand All @@ -1177,13 +1193,13 @@ func (sr *immutableRef) unlazyDiffMerge(ctx context.Context, dhs DescHandlers, p
if sr.diffParents.lower != nil {
diff.Lower = sr.diffParents.lower.getSnapshotID()
eg.Go(func() error {
return sr.diffParents.lower.unlazy(egctx, dhs, pg, s, false)
return sr.diffParents.lower.unlazy(egctx, dhs, pg, s, false, ensureContentStore)
})
}
if sr.diffParents.upper != nil {
diff.Upper = sr.diffParents.upper.getSnapshotID()
eg.Go(func() error {
return sr.diffParents.upper.unlazy(egctx, dhs, pg, s, false)
return sr.diffParents.upper.unlazy(egctx, dhs, pg, s, false, ensureContentStore)
})
}
case Layer:
Expand All @@ -1192,7 +1208,7 @@ func (sr *immutableRef) unlazyDiffMerge(ctx context.Context, dhs DescHandlers, p
case BaseLayer:
diff.Upper = sr.getSnapshotID()
eg.Go(func() error {
return sr.unlazy(egctx, dhs, pg, s, false)
return sr.unlazy(egctx, dhs, pg, s, false, ensureContentStore)
})
}
diffs = append(diffs, diff)
Expand Down Expand Up @@ -1223,7 +1239,7 @@ func (sr *immutableRef) unlazyDiffMerge(ctx context.Context, dhs DescHandlers, p
}

// should be called within sizeG.Do call for this ref's ID
func (sr *immutableRef) unlazyLayer(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group) (rerr error) {
func (sr *immutableRef) unlazyLayer(ctx context.Context, dhs DescHandlers, pg progress.Controller, s session.Group, ensureContentStore bool) (rerr error) {
if !sr.getBlobOnly() {
return nil
}
Expand All @@ -1250,7 +1266,7 @@ func (sr *immutableRef) unlazyLayer(ctx context.Context, dhs DescHandlers, pg pr
parentID := ""
if sr.layerParent != nil {
eg.Go(func() error {
if err := sr.layerParent.unlazy(egctx, dhs, pg, s, false); err != nil {
if err := sr.layerParent.unlazy(egctx, dhs, pg, s, false, ensureContentStore); err != nil {
return err
}
parentID = sr.layerParent.getSnapshotID()
Expand Down
143 changes: 143 additions & 0 deletions client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ func TestIntegration(t *testing.T) {
testLLBMountPerformance,
testClientCustomGRPCOpts,
testMultipleRecordsWithSameLayersCacheImportExport,
testSnapshotWithMultipleBlobs,
testExportLocalNoPlatformSplit,
testExportLocalNoPlatformSplitOverwrite,
)
Expand Down Expand Up @@ -5494,6 +5495,148 @@ func testMultipleRecordsWithSameLayersCacheImportExport(t *testing.T, sb integra
ensurePruneAll(t, c, sb)
}

// moby/buildkit#3809
func testSnapshotWithMultipleBlobs(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter)
c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

// build two images with same layer but different compressed blobs

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

now := time.Now()

st := llb.Scratch().File(llb.Copy(llb.Image("alpine"), "/", "/alpine/", llb.WithCreatedTime(now)))

def, err := st.Marshal(sb.Context())
require.NoError(t, err)

name1 := registry + "/multiblobtest1/image:latest"

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: "image",
Attrs: map[string]string{
"name": name1,
"push": "true",
"compression-level": "0",
},
},
},
}, nil)
require.NoError(t, err)

ensurePruneAll(t, c, sb)

st = st.File(llb.Mkfile("test", 0600, []byte("test"))) // extra layer so we don't get a cache match based on image config rootfs only

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

name2 := registry + "/multiblobtest2/image:latest"

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: "image",
Attrs: map[string]string{
"name": name2,
"push": "true",
"compression-level": "9",
},
},
},
}, nil)
require.NoError(t, err)

ensurePruneAll(t, c, sb)

// first build with first image
destDir := t.TempDir()

out1 := filepath.Join(destDir, "out1.tar")
outW1, err := os.Create(out1)
require.NoError(t, err)

st = llb.Image(name1).File(llb.Mkfile("test", 0600, []byte("test1")))

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterOCI, // make sure to export so blobs need to be loaded
Output: fixedWriteCloser(outW1),
},
},
}, nil)
require.NoError(t, err)

// make sure second image does not cause any errors
out2 := filepath.Join(destDir, "out2.tar")
outW2, err := os.Create(out2)
require.NoError(t, err)

st = llb.Image(name2).File(llb.Mkfile("test", 0600, []byte("test2")))

def, err = st.Marshal(sb.Context())
require.NoError(t, err)

_, err = c.Solve(sb.Context(), def, SolveOpt{
FrontendAttrs: map[string]string{
"attest:sbom": "",
},
Exports: []ExportEntry{
{
Type: ExporterOCI, // make sure to export so blobs need to be loaded
Output: fixedWriteCloser(outW2),
},
},
}, nil)
require.NoError(t, err)

// extra validation that we did get different layer blobs
dt, err := os.ReadFile(out1)
require.NoError(t, err)

m, err := testutil.ReadTarToMap(dt, false)
require.NoError(t, err)

var index ocispecs.Index
err = json.Unmarshal(m["index.json"].Data, &index)
require.NoError(t, err)

var mfst ocispecs.Manifest
err = json.Unmarshal(m["blobs/sha256/"+index.Manifests[0].Digest.Hex()].Data, &mfst)
require.NoError(t, err)

dt, err = os.ReadFile(out2)
require.NoError(t, err)

m, err = testutil.ReadTarToMap(dt, false)
require.NoError(t, err)

err = json.Unmarshal(m["index.json"].Data, &index)
require.NoError(t, err)

err = json.Unmarshal(m["blobs/sha256/"+index.Manifests[0].Digest.Hex()].Data, &index)
require.NoError(t, err)

var mfst2 ocispecs.Manifest
err = json.Unmarshal(m["blobs/sha256/"+index.Manifests[0].Digest.Hex()].Data, &mfst2)
require.NoError(t, err)

require.NotEqual(t, mfst.Layers[0].Digest, mfst2.Layers[0].Digest)
}

func testExportLocalNoPlatformSplit(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureMultiPlatform)
c, err := New(sb.Context(), sb.Address())
Expand Down

0 comments on commit 5e6497c

Please sign in to comment.