diff --git a/cache/metadata.go b/cache/metadata.go index b223024dcae3..8de8efd9bcef 100644 --- a/cache/metadata.go +++ b/cache/metadata.go @@ -44,7 +44,7 @@ const blobchainIndex = "blobchainid:" const chainIndex = "chainid:" type MetadataStore interface { - Search(context.Context, string) ([]RefMetadata, error) + Search(context.Context, string, bool) ([]RefMetadata, error) } type RefMetadata interface { @@ -71,6 +71,7 @@ type RefMetadata interface { // generic getters/setters for external packages GetString(string) string + Get(string) *metadata.Value SetString(key, val, index string) error GetExternal(string) ([]byte, error) @@ -79,15 +80,15 @@ type RefMetadata interface { ClearValueAndIndex(string, string) error } -func (cm *cacheManager) Search(ctx context.Context, idx string) ([]RefMetadata, error) { +func (cm *cacheManager) Search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) { cm.mu.Lock() defer cm.mu.Unlock() - return cm.search(ctx, idx) + return cm.search(ctx, idx, prefixOnly) } // callers must hold cm.mu lock -func (cm *cacheManager) search(ctx context.Context, idx string) ([]RefMetadata, error) { - sis, err := cm.MetadataStore.Search(ctx, idx) +func (cm *cacheManager) search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) { + sis, err := cm.MetadataStore.Search(ctx, idx, prefixOnly) if err != nil { return nil, err } @@ -119,12 +120,12 @@ func (cm *cacheManager) getMetadata(id string) (*cacheMetadata, bool) { // callers must hold cm.mu lock func (cm *cacheManager) searchBlobchain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) { - return cm.search(ctx, blobchainIndex+id.String()) + return cm.search(ctx, blobchainIndex+id.String(), false) } // callers must hold cm.mu lock func (cm *cacheManager) searchChain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) { - return cm.search(ctx, chainIndex+id.String()) + return cm.search(ctx, chainIndex+id.String(), false) } type cacheMetadata struct { @@ -486,6 +487,10 @@ func (md *cacheMetadata) GetString(key string) string { return str } +func (md *cacheMetadata) Get(key string) *metadata.Value { + return md.si.Get(key) +} + func (md *cacheMetadata) GetStringSlice(key string) []string { v := md.si.Get(key) if v == nil { diff --git a/cache/metadata/metadata.go b/cache/metadata/metadata.go index 72d66800b353..47a08b69d4a0 100644 --- a/cache/metadata/metadata.go +++ b/cache/metadata/metadata.go @@ -83,7 +83,7 @@ func (s *Store) Probe(index string) (bool, error) { return exists, errors.WithStack(err) } -func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error) { +func (s *Store) Search(ctx context.Context, index string, prefix bool) ([]*StorageItem, error) { var out []*StorageItem err := s.db.View(func(tx *bolt.Tx) error { b := tx.Bucket([]byte(indexBucket)) @@ -94,12 +94,18 @@ func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error if main == nil { return nil } - index = indexKey(index, "") + if !prefix { + index = indexKey(index, "") + } c := b.Cursor() k, _ := c.Seek([]byte(index)) for { if k != nil && strings.HasPrefix(string(k), index) { - itemID := strings.TrimPrefix(string(k), index) + idx := strings.LastIndex(string(k), "::") + if idx == -1 { + continue + } + itemID := string(k[idx+2:]) k, _ = c.Next() b := main.Bucket([]byte(itemID)) if b == nil { diff --git a/cache/metadata/metadata_test.go b/cache/metadata/metadata_test.go index 1038b578225c..c44cda1bc076 100644 --- a/cache/metadata/metadata_test.go +++ b/cache/metadata/metadata_test.go @@ -142,14 +142,14 @@ func TestIndexes(t *testing.T) { } ctx := context.Background() - sis, err := s.Search(ctx, "tag:baz") + sis, err := s.Search(ctx, "tag:baz", false) require.NoError(t, err) require.Equal(t, 2, len(sis)) require.Equal(t, "foo1", sis[0].ID()) require.Equal(t, "foo3", sis[1].ID()) - sis, err = s.Search(ctx, "tag:bax") + sis, err = s.Search(ctx, "tag:bax", false) require.NoError(t, err) require.Equal(t, 1, len(sis)) @@ -158,7 +158,7 @@ func TestIndexes(t *testing.T) { err = s.Clear("foo1") require.NoError(t, err) - sis, err = s.Search(ctx, "tag:baz") + sis, err = s.Search(ctx, "tag:baz", false) require.NoError(t, err) require.Equal(t, 1, len(sis)) diff --git a/frontend/dockerfile/dockerfile2llb/convert_runmount.go b/frontend/dockerfile/dockerfile2llb/convert_runmount.go index c222b522f759..cc9d858641d2 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_runmount.go +++ b/frontend/dockerfile/dockerfile2llb/convert_runmount.go @@ -57,7 +57,7 @@ func setCacheUIDGID(m *instructions.Mount, st llb.State) llb.State { if m.Mode != nil { mode = os.FileMode(*m.Mode) } - return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] settings cache mount permissions")) + return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] setting cache mount permissions")) } func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*dispatchState, opt dispatchOpt) ([]llb.RunOption, error) { diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index e12c0dd88286..42cf2a1bd08c 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -100,6 +100,7 @@ var allTests = integration.TestFuncs( testReproducibleIDs, testImportExportReproducibleIDs, testNoCache, + testCacheMountModeNoCache, testDockerfileFromHTTP, testBuiltinArgs, testPullScratch, @@ -5065,6 +5066,74 @@ COPY --from=s1 unique2 / require.NotEqual(t, string(unique2Dir1), string(unique2Dir3)) } +// moby/buildkit#5305 +func testCacheMountModeNoCache(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox AS base +ARG FOO=abc +RUN --mount=type=cache,target=/cache,mode=0773 touch /cache/$FOO && ls -l /cache | wc -l > /out + +FROM scratch +COPY --from=base /out / +`) + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + destDir := t.TempDir() + + opt := client.SolveOpt{ + FrontendAttrs: map[string]string{}, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + } + + _, err = f.Solve(sb.Context(), c, opt, nil) + require.NoError(t, err) + + opt.FrontendAttrs["no-cache"] = "" + + dt, err := os.ReadFile(filepath.Join(destDir, "out")) + require.NoError(t, err) + require.Equal(t, "2\n", string(dt)) + + opt.FrontendAttrs["build-arg:FOO"] = "def" + + _, err = f.Solve(sb.Context(), c, opt, nil) + require.NoError(t, err) + + dt, err = os.ReadFile(filepath.Join(destDir, "out")) + require.NoError(t, err) + require.Equal(t, "2\n", string(dt)) + + // safety check without no-cache + delete(opt.FrontendAttrs, "no-cache") + opt.FrontendAttrs["build-arg:FOO"] = "ghi" + + _, err = f.Solve(sb.Context(), c, opt, nil) + require.NoError(t, err) + + dt, err = os.ReadFile(filepath.Join(destDir, "out")) + require.NoError(t, err) + require.Equal(t, "3\n", string(dt)) +} + func testPlatformArgsImplicit(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") f := getFrontend(t, sb) diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 9c4382606354..fa00b65e127d 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -144,12 +144,8 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp } if len(dpc.ids) > 0 { - ids := make([]string, 0, len(dpc.ids)) - for id := range dpc.ids { - ids = append(ids, id) - } if err := b.eachWorker(func(w worker.Worker) error { - return w.PruneCacheMounts(ctx, ids) + return w.PruneCacheMounts(ctx, dpc.ids) }); err != nil { return nil, err } diff --git a/solver/llbsolver/mounts/mount.go b/solver/llbsolver/mounts/mount.go index 85bc4ffb8632..509c7e44add1 100644 --- a/solver/llbsolver/mounts/mount.go +++ b/solver/llbsolver/mounts/mount.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "sync" "time" @@ -116,7 +117,7 @@ func (g *cacheRefGetter) getRefCacheDirNoCache(ctx context.Context, key string, cacheRefsLocker.Lock(key) defer cacheRefsLocker.Unlock(key) for { - sis, err := SearchCacheDir(ctx, g.cm, key) + sis, err := SearchCacheDir(ctx, g.cm, key, false) if err != nil { return nil, err } @@ -542,13 +543,24 @@ func (r *cacheRef) Release(ctx context.Context) error { const keyCacheDir = "cache-dir" const cacheDirIndex = keyCacheDir + ":" -func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string) ([]CacheRefMetadata, error) { +func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string, withNested bool) ([]CacheRefMetadata, error) { var results []CacheRefMetadata - mds, err := store.Search(ctx, cacheDirIndex+id) + key := cacheDirIndex + id + if withNested { + key += ":" + } + mds, err := store.Search(ctx, key, withNested) if err != nil { return nil, err } for _, md := range mds { + if withNested { + v := md.Get(keyCacheDir) + // skip partial ids but allow id without ref ID + if v == nil || v.Index != key && !strings.HasPrefix(v.Index, key) { + continue + } + } results = append(results, CacheRefMetadata{md}) } return results, nil diff --git a/solver/llbsolver/vertex.go b/solver/llbsolver/vertex.go index fdefa06e8447..b828e7336993 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -125,7 +125,7 @@ func ValidateEntitlements(ent entitlements.Set) LoadOpt { } type detectPrunedCacheID struct { - ids map[string]struct{} + ids map[string]bool } func (dpc *detectPrunedCacheID) Load(op *pb.Op, md *pb.OpMetadata, opt *solver.VertexOptions) error { @@ -142,9 +142,10 @@ func (dpc *detectPrunedCacheID) Load(op *pb.Op, md *pb.OpMetadata, opt *solver.V id = m.Dest } if dpc.ids == nil { - dpc.ids = map[string]struct{}{} + dpc.ids = map[string]bool{} } - dpc.ids[id] = struct{}{} + // value shows in mount is on top of a ref + dpc.ids[id] = m.Input != -1 } } } diff --git a/source/git/source.go b/source/git/source.go index 1b757500d7a3..be14d3fb55f7 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -746,7 +746,7 @@ const gitSnapshotIndex = keyGitSnapshot + "::" func search(ctx context.Context, store cache.MetadataStore, key string, idx string) ([]cacheRefMetadata, error) { var results []cacheRefMetadata - mds, err := store.Search(ctx, idx+key) + mds, err := store.Search(ctx, idx+key, false) if err != nil { return nil, err } diff --git a/source/http/source.go b/source/http/source.go index 5369eac3a496..d6c6050199fb 100644 --- a/source/http/source.go +++ b/source/http/source.go @@ -480,7 +480,7 @@ func getFileName(urlStr, manualFilename string, resp *http.Response) string { func searchHTTPURLDigest(ctx context.Context, store cache.MetadataStore, dgst digest.Digest) ([]cacheRefMetadata, error) { var results []cacheRefMetadata - mds, err := store.Search(ctx, string(dgst)) + mds, err := store.Search(ctx, string(dgst), false) if err != nil { return nil, err } diff --git a/source/local/source.go b/source/local/source.go index db61b7a4735d..7c17117886ce 100644 --- a/source/local/source.go +++ b/source/local/source.go @@ -334,7 +334,7 @@ const sharedKeyIndex = keySharedKey + ":" func searchSharedKey(ctx context.Context, store cache.MetadataStore, k string) ([]cacheRefMetadata, error) { var results []cacheRefMetadata - mds, err := store.Search(ctx, sharedKeyIndex+k) + mds, err := store.Search(ctx, sharedKeyIndex+k, false) if err != nil { return nil, err } diff --git a/worker/base/worker.go b/worker/base/worker.go index 8121c74c6530..d57cf457618f 100644 --- a/worker/base/worker.go +++ b/worker/base/worker.go @@ -341,13 +341,13 @@ func (w *Worker) ResolveOp(v solver.Vertex, s frontend.FrontendLLBBridge, sm *se return nil, errors.Errorf("could not resolve %v", v) } -func (w *Worker) PruneCacheMounts(ctx context.Context, ids []string) error { +func (w *Worker) PruneCacheMounts(ctx context.Context, ids map[string]bool) error { mu := mounts.CacheMountsLocker() mu.Lock() defer mu.Unlock() - for _, id := range ids { - mds, err := mounts.SearchCacheDir(ctx, w.CacheMgr, id) + for id, nested := range ids { + mds, err := mounts.SearchCacheDir(ctx, w.CacheMgr, id, nested) if err != nil { return err } diff --git a/worker/worker.go b/worker/worker.go index dd336917a4c9..e5d6ef564c5d 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -35,7 +35,7 @@ type Worker interface { Exporter(name string, sm *session.Manager) (exporter.Exporter, error) Prune(ctx context.Context, ch chan client.UsageInfo, opt ...client.PruneInfo) error FromRemote(ctx context.Context, remote *solver.Remote) (cache.ImmutableRef, error) - PruneCacheMounts(ctx context.Context, ids []string) error + PruneCacheMounts(ctx context.Context, ids map[string]bool) error ContentStore() *containerdsnapshot.Store Executor() executor.Executor CacheManager() cache.Manager