Skip to content

Commit 0ad8d61

Browse files
authored
Merge pull request moby#3109 from ktock/reuseremotelayers
Fix cache cannot reuse lazy layers
2 parents 71a3a18 + 085bd8a commit 0ad8d61

File tree

13 files changed

+130
-36
lines changed

13 files changed

+130
-36
lines changed

cache/remotecache/v1/cachestorage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func (cs *cacheResultStorage) LoadRemotes(ctx context.Context, res solver.CacheR
291291
return nil, errors.WithStack(solver.ErrNotFound)
292292
}
293293

294-
func (cs *cacheResultStorage) Exists(id string) bool {
294+
func (cs *cacheResultStorage) Exists(ctx context.Context, id string) bool {
295295
return cs.byResultID(id) != nil
296296
}
297297

client/client_test.go

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3814,18 +3814,19 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
38143814

38153815
// stargz layers should be lazy even for executing something on them
38163816
def, err = baseDef.
3817-
Run(llb.Args([]string{"/bin/touch", "/bar"})).
3817+
Run(llb.Args([]string{"sh", "-c", "cat /dev/urandom | head -c 100 | sha256sum > unique"})).
38183818
Marshal(sb.Context())
38193819
require.NoError(t, err)
38203820
target := registry + "/buildkit/testlazyimage:" + identity.NewID()
3821-
_, err = c.Solve(sb.Context(), def, SolveOpt{
3821+
resp, err := c.Solve(sb.Context(), def, SolveOpt{
38223822
Exports: []ExportEntry{
38233823
{
38243824
Type: ExporterImage,
38253825
Attrs: map[string]string{
38263826
"name": target,
38273827
"push": "true",
38283828
"store": "true",
3829+
"oci-mediatypes": "true",
38293830
"unsafe-internal-store-allow-incomplete": "true",
38303831
},
38313832
},
@@ -3838,9 +3839,25 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
38383839
},
38393840
},
38403841
},
3842+
CacheExports: []CacheOptionsEntry{
3843+
{
3844+
Type: "registry",
3845+
Attrs: map[string]string{
3846+
"ref": sgzCache,
3847+
"compression": "estargz",
3848+
"oci-mediatypes": "true",
3849+
},
3850+
},
3851+
},
38413852
}, nil)
38423853
require.NoError(t, err)
38433854

3855+
dgst, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
3856+
require.Equal(t, ok, true)
3857+
3858+
unique, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst, "/unique")
3859+
require.NoError(t, err)
3860+
38443861
img, err := imageService.Get(ctx, target)
38453862
require.NoError(t, err)
38463863

@@ -3861,6 +3878,40 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
38613878
_, err = contentStore.Info(ctx, manifest.Layers[len(manifest.Layers)-1].Digest)
38623879
require.NoError(t, err)
38633880

3881+
// Run build again and check if cache is reused
3882+
resp, err = c.Solve(sb.Context(), def, SolveOpt{
3883+
Exports: []ExportEntry{
3884+
{
3885+
Type: ExporterImage,
3886+
Attrs: map[string]string{
3887+
"name": target,
3888+
"push": "true",
3889+
"store": "true",
3890+
"oci-mediatypes": "true",
3891+
"unsafe-internal-store-allow-incomplete": "true",
3892+
},
3893+
},
3894+
},
3895+
CacheImports: []CacheOptionsEntry{
3896+
{
3897+
Type: "registry",
3898+
Attrs: map[string]string{
3899+
"ref": sgzCache,
3900+
},
3901+
},
3902+
},
3903+
}, nil)
3904+
require.NoError(t, err)
3905+
3906+
dgst2, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
3907+
require.Equal(t, ok, true)
3908+
3909+
unique2, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst2, "/unique")
3910+
require.NoError(t, err)
3911+
3912+
require.Equal(t, dgst, dgst2)
3913+
require.EqualValues(t, unique, unique2)
3914+
38643915
// clear all local state out
38653916
err = imageService.Delete(ctx, img.Name, images.SynchronousDelete())
38663917
require.NoError(t, err)
@@ -4104,11 +4155,11 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
41044155

41054156
// stargz layers should be lazy even for executing something on them
41064157
def, err = llb.Image(sgzImage).
4107-
Run(llb.Args([]string{"/bin/touch", "/foo"})).
4158+
Run(llb.Args([]string{"sh", "-c", "cat /dev/urandom | head -c 100 | sha256sum > unique"})).
41084159
Marshal(sb.Context())
41094160
require.NoError(t, err)
41104161
target := registry + "/buildkit/testlazyimage:" + identity.NewID()
4111-
_, err = c.Solve(sb.Context(), def, SolveOpt{
4162+
resp, err := c.Solve(sb.Context(), def, SolveOpt{
41124163
Exports: []ExportEntry{
41134164
{
41144165
Type: ExporterImage,
@@ -4124,6 +4175,12 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
41244175
}, nil)
41254176
require.NoError(t, err)
41264177

4178+
dgst, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
4179+
require.Equal(t, ok, true)
4180+
4181+
unique, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst, "/unique")
4182+
require.NoError(t, err)
4183+
41274184
img, err := imageService.Get(ctx, target)
41284185
require.NoError(t, err)
41294186

@@ -4144,6 +4201,32 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
41444201
_, err = contentStore.Info(ctx, manifest.Layers[len(manifest.Layers)-1].Digest)
41454202
require.NoError(t, err)
41464203

4204+
// Run build again and check if cache is reused
4205+
resp, err = c.Solve(sb.Context(), def, SolveOpt{
4206+
Exports: []ExportEntry{
4207+
{
4208+
Type: ExporterImage,
4209+
Attrs: map[string]string{
4210+
"name": target,
4211+
"push": "true",
4212+
"store": "true",
4213+
"oci-mediatypes": "true",
4214+
"unsafe-internal-store-allow-incomplete": "true",
4215+
},
4216+
},
4217+
},
4218+
}, nil)
4219+
require.NoError(t, err)
4220+
4221+
dgst2, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
4222+
require.Equal(t, ok, true)
4223+
4224+
unique2, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst2, "/unique")
4225+
require.NoError(t, err)
4226+
4227+
require.Equal(t, dgst, dgst2)
4228+
require.EqualValues(t, unique, unique2)
4229+
41474230
// clear all local state out
41484231
err = imageService.Delete(ctx, img.Name, images.SynchronousDelete())
41494232
require.NoError(t, err)

control/control.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ func (c *Controller) Prune(req *controlapi.PruneRequest, stream controlapi.Contr
182182
defer func() {
183183
if didPrune {
184184
if c, ok := c.cache.(interface {
185-
ReleaseUnreferenced() error
185+
ReleaseUnreferenced(context.Context) error
186186
}); ok {
187-
if err := c.ReleaseUnreferenced(); err != nil {
187+
if err := c.ReleaseUnreferenced(ctx); err != nil {
188188
bklog.G(ctx).Errorf("failed to release cache metadata: %+v", err)
189189
}
190190
}

solver/cache_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestInMemoryCache(t *testing.T) {
4949
require.NoError(t, err)
5050
require.Equal(t, len(keys), 1)
5151

52-
matches, err := m.Records(keys[0])
52+
matches, err := m.Records(context.TODO(), keys[0])
5353
require.NoError(t, err)
5454
require.Equal(t, len(matches), 1)
5555

@@ -65,7 +65,7 @@ func TestInMemoryCache(t *testing.T) {
6565
require.NoError(t, err)
6666
require.Equal(t, len(keys), 1)
6767

68-
matches, err = m.Records(keys[0])
68+
matches, err = m.Records(context.TODO(), keys[0])
6969
require.NoError(t, err)
7070
require.Equal(t, len(matches), 1)
7171

@@ -99,7 +99,7 @@ func TestInMemoryCache(t *testing.T) {
9999
require.NoError(t, err)
100100
require.Equal(t, len(keys), 1)
101101

102-
matches, err = m.Records(keys[0])
102+
matches, err = m.Records(context.TODO(), keys[0])
103103
require.NoError(t, err)
104104
require.Equal(t, len(matches), 1)
105105

@@ -121,7 +121,7 @@ func TestInMemoryCache(t *testing.T) {
121121
require.NoError(t, err)
122122
require.Equal(t, len(keys), 1)
123123

124-
matches, err = m.Records(keys[0])
124+
matches, err = m.Records(context.TODO(), keys[0])
125125
require.NoError(t, err)
126126
require.Equal(t, len(matches), 2)
127127

@@ -175,7 +175,7 @@ func TestInMemoryCacheSelector(t *testing.T) {
175175
require.NoError(t, err)
176176
require.Equal(t, len(keys), 1)
177177

178-
matches, err := m.Records(keys[0])
178+
matches, err := m.Records(context.TODO(), keys[0])
179179
require.NoError(t, err)
180180
require.Equal(t, len(matches), 1)
181181

@@ -203,7 +203,7 @@ func TestInMemoryCacheSelectorNested(t *testing.T) {
203203
require.NoError(t, err)
204204
require.Equal(t, len(keys), 1)
205205

206-
matches, err := m.Records(keys[0])
206+
matches, err := m.Records(context.TODO(), keys[0])
207207
require.NoError(t, err)
208208
require.Equal(t, len(matches), 1)
209209

@@ -223,7 +223,7 @@ func TestInMemoryCacheSelectorNested(t *testing.T) {
223223
require.NoError(t, err)
224224
require.Equal(t, len(keys), 1)
225225

226-
matches, err = m.Records(keys[0])
226+
matches, err = m.Records(context.TODO(), keys[0])
227227
require.NoError(t, err)
228228
require.Equal(t, len(matches), 1)
229229

@@ -253,7 +253,7 @@ func TestInMemoryCacheReleaseParent(t *testing.T) {
253253
require.NoError(t, err)
254254
require.Equal(t, len(keys), 1)
255255

256-
matches, err := m.Records(keys[0])
256+
matches, err := m.Records(context.TODO(), keys[0])
257257
require.NoError(t, err)
258258
require.Equal(t, len(matches), 1)
259259

@@ -265,15 +265,15 @@ func TestInMemoryCacheReleaseParent(t *testing.T) {
265265
require.NoError(t, err)
266266
require.Equal(t, len(keys), 1)
267267

268-
matches, err = m.Records(keys[0])
268+
matches, err = m.Records(context.TODO(), keys[0])
269269
require.NoError(t, err)
270270
require.Equal(t, len(matches), 0)
271271

272272
keys, err = m.Query(depKeys(expKey(keys[0])), 0, dgst("bar"), 0)
273273
require.NoError(t, err)
274274
require.Equal(t, len(keys), 1)
275275

276-
matches, err = m.Records(keys[0])
276+
matches, err = m.Records(context.TODO(), keys[0])
277277
require.NoError(t, err)
278278
require.Equal(t, len(matches), 1)
279279

@@ -311,15 +311,15 @@ func TestInMemoryCacheRestoreOfflineDeletion(t *testing.T) {
311311
require.NoError(t, err)
312312
require.Equal(t, len(keys), 1)
313313

314-
matches, err := m.Records(keys[0])
314+
matches, err := m.Records(context.TODO(), keys[0])
315315
require.NoError(t, err)
316316
require.Equal(t, len(matches), 0)
317317

318318
keys, err = m.Query(depKeys(expKey(keys[0])), 0, dgst("bar"), 0)
319319
require.NoError(t, err)
320320
require.Equal(t, len(keys), 1)
321321

322-
matches, err = m.Records(keys[0])
322+
matches, err = m.Records(context.TODO(), keys[0])
323323
require.NoError(t, err)
324324
require.Equal(t, len(matches), 1)
325325
}

solver/cachemanager.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func NewCacheManager(ctx context.Context, id string, storage CacheKeyStorage, re
2525
results: results,
2626
}
2727

28-
if err := cm.ReleaseUnreferenced(); err != nil {
28+
if err := cm.ReleaseUnreferenced(ctx); err != nil {
2929
bklog.G(ctx).Errorf("failed to release unreferenced cache metadata: %+v", err)
3030
}
3131

@@ -40,10 +40,10 @@ type cacheManager struct {
4040
results CacheResultStorage
4141
}
4242

43-
func (c *cacheManager) ReleaseUnreferenced() error {
43+
func (c *cacheManager) ReleaseUnreferenced(ctx context.Context) error {
4444
return c.backend.Walk(func(id string) error {
4545
return c.backend.WalkResults(id, func(cr CacheResult) error {
46-
if !c.results.Exists(cr.ID) {
46+
if !c.results.Exists(ctx, cr.ID) {
4747
c.backend.Release(cr.ID)
4848
}
4949
return nil
@@ -112,10 +112,10 @@ func (c *cacheManager) Query(deps []CacheKeyWithSelector, input Index, dgst dige
112112
return keys, nil
113113
}
114114

115-
func (c *cacheManager) Records(ck *CacheKey) ([]*CacheRecord, error) {
115+
func (c *cacheManager) Records(ctx context.Context, ck *CacheKey) ([]*CacheRecord, error) {
116116
outs := make([]*CacheRecord, 0)
117117
if err := c.backend.WalkResults(c.getID(ck), func(r CacheResult) error {
118-
if c.results.Exists(r.ID) {
118+
if c.results.Exists(ctx, r.ID) {
119119
outs = append(outs, &CacheRecord{
120120
ID: r.ID,
121121
cacheManager: c,

solver/cachestorage.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,5 @@ type CacheResultStorage interface {
4949
Save(Result, time.Time) (CacheResult, error)
5050
Load(ctx context.Context, res CacheResult) (Result, error)
5151
LoadRemotes(ctx context.Context, res CacheResult, compression *compression.Config, s session.Group) ([]*Remote, error)
52-
Exists(id string) bool
52+
Exists(ctx context.Context, id string) bool
5353
}

solver/combinedcache.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (cm *combinedCacheManager) Save(key *CacheKey, s Result, createdAt time.Tim
100100
return cm.main.Save(key, s, createdAt)
101101
}
102102

103-
func (cm *combinedCacheManager) Records(ck *CacheKey) ([]*CacheRecord, error) {
103+
func (cm *combinedCacheManager) Records(ctx context.Context, ck *CacheKey) ([]*CacheRecord, error) {
104104
if len(ck.ids) == 0 {
105105
return nil, errors.Errorf("no results")
106106
}
@@ -112,7 +112,7 @@ func (cm *combinedCacheManager) Records(ck *CacheKey) ([]*CacheRecord, error) {
112112
for c := range ck.ids {
113113
func(c *cacheManager) {
114114
eg.Go(func() error {
115-
recs, err := c.Records(ck)
115+
recs, err := c.Records(ctx, ck)
116116
if err != nil {
117117
return err
118118
}

solver/edge.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func (e *edge) processUpdate(upt pipe.Receiver) (depChanged bool) {
405405
} else {
406406
for _, k := range keys {
407407
k.vtx = e.edge.Vertex.Digest()
408-
records, err := e.op.Cache().Records(k)
408+
records, err := e.op.Cache().Records(context.Background(), k)
409409
if err != nil {
410410
bklog.G(context.TODO()).Errorf("error receiving cache records: %v", err)
411411
continue
@@ -583,7 +583,7 @@ func (e *edge) recalcCurrentState() {
583583
}
584584
}
585585

586-
records, err := e.op.Cache().Records(mergedKey)
586+
records, err := e.op.Cache().Records(context.Background(), mergedKey)
587587
if err != nil {
588588
bklog.G(context.TODO()).Errorf("error receiving cache records: %v", err)
589589
continue

solver/jobs.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,18 @@ func (s *sharedOp) IgnoreCache() bool {
680680
}
681681

682682
func (s *sharedOp) Cache() CacheManager {
683-
return s.st.combinedCacheManager()
683+
return &cacheWithCacheOpts{s.st.combinedCacheManager(), s.st}
684+
}
685+
686+
type cacheWithCacheOpts struct {
687+
CacheManager
688+
st *state
689+
}
690+
691+
func (c cacheWithCacheOpts) Records(ctx context.Context, ck *CacheKey) ([]*CacheRecord, error) {
692+
// Allow Records accessing to cache opts through ctx. This enable to use remote provider
693+
// during checking the cache existence.
694+
return c.CacheManager.Records(withAncestorCacheOpts(ctx, c.st), ck)
684695
}
685696

686697
func (s *sharedOp) LoadCache(ctx context.Context, rec *CacheRecord) (Result, error) {

solver/llbsolver/bridge.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,12 @@ func (lcm *lazyCacheManager) Query(inp []solver.CacheKeyWithSelector, inputIndex
329329
}
330330
return lcm.main.Query(inp, inputIndex, dgst, outputIndex)
331331
}
332-
func (lcm *lazyCacheManager) Records(ck *solver.CacheKey) ([]*solver.CacheRecord, error) {
332+
func (lcm *lazyCacheManager) Records(ctx context.Context, ck *solver.CacheKey) ([]*solver.CacheRecord, error) {
333333
lcm.wait()
334334
if lcm.main == nil {
335335
return nil, nil
336336
}
337-
return lcm.main.Records(ck)
337+
return lcm.main.Records(ctx, ck)
338338
}
339339
func (lcm *lazyCacheManager) Load(ctx context.Context, rec *solver.CacheRecord) (solver.Result, error) {
340340
if err := lcm.wait(); err != nil {

0 commit comments

Comments
 (0)