Skip to content

Commit a1993e8

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#5306 from tonistiigi/cache-mount-mode-prune
exec: fix pruning cache mounts with parent ref on no-cache
2 parents 85668ff + 610affa commit a1993e8

File tree

13 files changed

+121
-32
lines changed

13 files changed

+121
-32
lines changed

‎cache/metadata.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const blobchainIndex = "blobchainid:"
4444
const chainIndex = "chainid:"
4545

4646
type MetadataStore interface {
47-
Search(context.Context, string) ([]RefMetadata, error)
47+
Search(context.Context, string, bool) ([]RefMetadata, error)
4848
}
4949

5050
type RefMetadata interface {
@@ -71,6 +71,7 @@ type RefMetadata interface {
7171

7272
// generic getters/setters for external packages
7373
GetString(string) string
74+
Get(string) *metadata.Value
7475
SetString(key, val, index string) error
7576

7677
GetExternal(string) ([]byte, error)
@@ -79,15 +80,15 @@ type RefMetadata interface {
7980
ClearValueAndIndex(string, string) error
8081
}
8182

82-
func (cm *cacheManager) Search(ctx context.Context, idx string) ([]RefMetadata, error) {
83+
func (cm *cacheManager) Search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) {
8384
cm.mu.Lock()
8485
defer cm.mu.Unlock()
85-
return cm.search(ctx, idx)
86+
return cm.search(ctx, idx, prefixOnly)
8687
}
8788

8889
// callers must hold cm.mu lock
89-
func (cm *cacheManager) search(ctx context.Context, idx string) ([]RefMetadata, error) {
90-
sis, err := cm.MetadataStore.Search(ctx, idx)
90+
func (cm *cacheManager) search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) {
91+
sis, err := cm.MetadataStore.Search(ctx, idx, prefixOnly)
9192
if err != nil {
9293
return nil, err
9394
}
@@ -119,12 +120,12 @@ func (cm *cacheManager) getMetadata(id string) (*cacheMetadata, bool) {
119120

120121
// callers must hold cm.mu lock
121122
func (cm *cacheManager) searchBlobchain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) {
122-
return cm.search(ctx, blobchainIndex+id.String())
123+
return cm.search(ctx, blobchainIndex+id.String(), false)
123124
}
124125

125126
// callers must hold cm.mu lock
126127
func (cm *cacheManager) searchChain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) {
127-
return cm.search(ctx, chainIndex+id.String())
128+
return cm.search(ctx, chainIndex+id.String(), false)
128129
}
129130

130131
type cacheMetadata struct {
@@ -486,6 +487,10 @@ func (md *cacheMetadata) GetString(key string) string {
486487
return str
487488
}
488489

490+
func (md *cacheMetadata) Get(key string) *metadata.Value {
491+
return md.si.Get(key)
492+
}
493+
489494
func (md *cacheMetadata) GetStringSlice(key string) []string {
490495
v := md.si.Get(key)
491496
if v == nil {

‎cache/metadata/metadata.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func (s *Store) Probe(index string) (bool, error) {
8383
return exists, errors.WithStack(err)
8484
}
8585

86-
func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error) {
86+
func (s *Store) Search(ctx context.Context, index string, prefix bool) ([]*StorageItem, error) {
8787
var out []*StorageItem
8888
err := s.db.View(func(tx *bolt.Tx) error {
8989
b := tx.Bucket([]byte(indexBucket))
@@ -94,12 +94,18 @@ func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error
9494
if main == nil {
9595
return nil
9696
}
97-
index = indexKey(index, "")
97+
if !prefix {
98+
index = indexKey(index, "")
99+
}
98100
c := b.Cursor()
99101
k, _ := c.Seek([]byte(index))
100102
for {
101103
if k != nil && strings.HasPrefix(string(k), index) {
102-
itemID := strings.TrimPrefix(string(k), index)
104+
idx := strings.LastIndex(string(k), "::")
105+
if idx == -1 {
106+
continue
107+
}
108+
itemID := string(k[idx+2:])
103109
k, _ = c.Next()
104110
b := main.Bucket([]byte(itemID))
105111
if b == nil {

‎cache/metadata/metadata_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,14 +142,14 @@ func TestIndexes(t *testing.T) {
142142
}
143143

144144
ctx := context.Background()
145-
sis, err := s.Search(ctx, "tag:baz")
145+
sis, err := s.Search(ctx, "tag:baz", false)
146146
require.NoError(t, err)
147147
require.Equal(t, 2, len(sis))
148148

149149
require.Equal(t, "foo1", sis[0].ID())
150150
require.Equal(t, "foo3", sis[1].ID())
151151

152-
sis, err = s.Search(ctx, "tag:bax")
152+
sis, err = s.Search(ctx, "tag:bax", false)
153153
require.NoError(t, err)
154154
require.Equal(t, 1, len(sis))
155155

@@ -158,7 +158,7 @@ func TestIndexes(t *testing.T) {
158158
err = s.Clear("foo1")
159159
require.NoError(t, err)
160160

161-
sis, err = s.Search(ctx, "tag:baz")
161+
sis, err = s.Search(ctx, "tag:baz", false)
162162
require.NoError(t, err)
163163
require.Equal(t, 1, len(sis))
164164

‎frontend/dockerfile/dockerfile2llb/convert_runmount.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func setCacheUIDGID(m *instructions.Mount, st llb.State) llb.State {
5757
if m.Mode != nil {
5858
mode = os.FileMode(*m.Mode)
5959
}
60-
return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] settings cache mount permissions"))
60+
return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] setting cache mount permissions"))
6161
}
6262

6363
func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*dispatchState, opt dispatchOpt) ([]llb.RunOption, error) {

‎frontend/dockerfile/dockerfile_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ var allTests = integration.TestFuncs(
100100
testReproducibleIDs,
101101
testImportExportReproducibleIDs,
102102
testNoCache,
103+
testCacheMountModeNoCache,
103104
testDockerfileFromHTTP,
104105
testBuiltinArgs,
105106
testPullScratch,
@@ -5065,6 +5066,74 @@ COPY --from=s1 unique2 /
50655066
require.NotEqual(t, string(unique2Dir1), string(unique2Dir3))
50665067
}
50675068

5069+
// moby/buildkit#5305
5070+
func testCacheMountModeNoCache(t *testing.T, sb integration.Sandbox) {
5071+
integration.SkipOnPlatform(t, "windows")
5072+
f := getFrontend(t, sb)
5073+
5074+
dockerfile := []byte(`
5075+
FROM busybox AS base
5076+
ARG FOO=abc
5077+
RUN --mount=type=cache,target=/cache,mode=0773 touch /cache/$FOO && ls -l /cache | wc -l > /out
5078+
5079+
FROM scratch
5080+
COPY --from=base /out /
5081+
`)
5082+
dir := integration.Tmpdir(
5083+
t,
5084+
fstest.CreateFile("Dockerfile", dockerfile, 0600),
5085+
)
5086+
5087+
c, err := client.New(sb.Context(), sb.Address())
5088+
require.NoError(t, err)
5089+
defer c.Close()
5090+
5091+
destDir := t.TempDir()
5092+
5093+
opt := client.SolveOpt{
5094+
FrontendAttrs: map[string]string{},
5095+
Exports: []client.ExportEntry{
5096+
{
5097+
Type: client.ExporterLocal,
5098+
OutputDir: destDir,
5099+
},
5100+
},
5101+
LocalMounts: map[string]fsutil.FS{
5102+
dockerui.DefaultLocalNameDockerfile: dir,
5103+
dockerui.DefaultLocalNameContext: dir,
5104+
},
5105+
}
5106+
5107+
_, err = f.Solve(sb.Context(), c, opt, nil)
5108+
require.NoError(t, err)
5109+
5110+
opt.FrontendAttrs["no-cache"] = ""
5111+
5112+
dt, err := os.ReadFile(filepath.Join(destDir, "out"))
5113+
require.NoError(t, err)
5114+
require.Equal(t, "2\n", string(dt))
5115+
5116+
opt.FrontendAttrs["build-arg:FOO"] = "def"
5117+
5118+
_, err = f.Solve(sb.Context(), c, opt, nil)
5119+
require.NoError(t, err)
5120+
5121+
dt, err = os.ReadFile(filepath.Join(destDir, "out"))
5122+
require.NoError(t, err)
5123+
require.Equal(t, "2\n", string(dt))
5124+
5125+
// safety check without no-cache
5126+
delete(opt.FrontendAttrs, "no-cache")
5127+
opt.FrontendAttrs["build-arg:FOO"] = "ghi"
5128+
5129+
_, err = f.Solve(sb.Context(), c, opt, nil)
5130+
require.NoError(t, err)
5131+
5132+
dt, err = os.ReadFile(filepath.Join(destDir, "out"))
5133+
require.NoError(t, err)
5134+
require.Equal(t, "3\n", string(dt))
5135+
}
5136+
50685137
func testPlatformArgsImplicit(t *testing.T, sb integration.Sandbox) {
50695138
integration.SkipOnPlatform(t, "windows")
50705139
f := getFrontend(t, sb)

‎solver/llbsolver/bridge.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,8 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
144144
}
145145

146146
if len(dpc.ids) > 0 {
147-
ids := make([]string, 0, len(dpc.ids))
148-
for id := range dpc.ids {
149-
ids = append(ids, id)
150-
}
151147
if err := b.eachWorker(func(w worker.Worker) error {
152-
return w.PruneCacheMounts(ctx, ids)
148+
return w.PruneCacheMounts(ctx, dpc.ids)
153149
}); err != nil {
154150
return nil, err
155151
}

‎solver/llbsolver/mounts/mount.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"strings"
89
"sync"
910
"time"
1011

@@ -116,7 +117,7 @@ func (g *cacheRefGetter) getRefCacheDirNoCache(ctx context.Context, key string,
116117
cacheRefsLocker.Lock(key)
117118
defer cacheRefsLocker.Unlock(key)
118119
for {
119-
sis, err := SearchCacheDir(ctx, g.cm, key)
120+
sis, err := SearchCacheDir(ctx, g.cm, key, false)
120121
if err != nil {
121122
return nil, err
122123
}
@@ -542,13 +543,24 @@ func (r *cacheRef) Release(ctx context.Context) error {
542543
const keyCacheDir = "cache-dir"
543544
const cacheDirIndex = keyCacheDir + ":"
544545

545-
func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string) ([]CacheRefMetadata, error) {
546+
func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string, withNested bool) ([]CacheRefMetadata, error) {
546547
var results []CacheRefMetadata
547-
mds, err := store.Search(ctx, cacheDirIndex+id)
548+
key := cacheDirIndex + id
549+
if withNested {
550+
key += ":"
551+
}
552+
mds, err := store.Search(ctx, key, withNested)
548553
if err != nil {
549554
return nil, err
550555
}
551556
for _, md := range mds {
557+
if withNested {
558+
v := md.Get(keyCacheDir)
559+
// skip partial ids but allow id without ref ID
560+
if v == nil || v.Index != key && !strings.HasPrefix(v.Index, key) {
561+
continue
562+
}
563+
}
552564
results = append(results, CacheRefMetadata{md})
553565
}
554566
return results, nil

‎solver/llbsolver/vertex.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func ValidateEntitlements(ent entitlements.Set) LoadOpt {
125125
}
126126

127127
type detectPrunedCacheID struct {
128-
ids map[string]struct{}
128+
ids map[string]bool
129129
}
130130

131131
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
142142
id = m.Dest
143143
}
144144
if dpc.ids == nil {
145-
dpc.ids = map[string]struct{}{}
145+
dpc.ids = map[string]bool{}
146146
}
147-
dpc.ids[id] = struct{}{}
147+
// value shows in mount is on top of a ref
148+
dpc.ids[id] = m.Input != -1
148149
}
149150
}
150151
}

‎source/git/source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ const gitSnapshotIndex = keyGitSnapshot + "::"
746746

747747
func search(ctx context.Context, store cache.MetadataStore, key string, idx string) ([]cacheRefMetadata, error) {
748748
var results []cacheRefMetadata
749-
mds, err := store.Search(ctx, idx+key)
749+
mds, err := store.Search(ctx, idx+key, false)
750750
if err != nil {
751751
return nil, err
752752
}

‎source/http/source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ func getFileName(urlStr, manualFilename string, resp *http.Response) string {
480480

481481
func searchHTTPURLDigest(ctx context.Context, store cache.MetadataStore, dgst digest.Digest) ([]cacheRefMetadata, error) {
482482
var results []cacheRefMetadata
483-
mds, err := store.Search(ctx, string(dgst))
483+
mds, err := store.Search(ctx, string(dgst), false)
484484
if err != nil {
485485
return nil, err
486486
}

0 commit comments

Comments
 (0)