Skip to content

Commit 610affa

Browse files
tonistiigitonistiigi
committed
exec: fix pruning cache mounts with parent ref on no-cache
On a build with no-cache, cache mounts were not pruned correctly if the mount was on top of another ref. This also appeared in Dockerfile when mode/uid/gid was set because implicit parent ref is created in these cases in order to change the permissions of a subdir that is used as a cache mount base. Because it is not possible to know ahead of time what ref will become the parent of cache mount during build, all cache mounts matching the ID that have a parent will be pruned. Signed-off-by: Tonis Tiigi <[email protected]>
1 parent 3a9ce1d commit 610affa

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)