Skip to content

Commit 085bd8a

Browse files
ktockktock
committed
Fix cache cannot reuse lazy layers
Signed-off-by: Kohei Tokunaga <[email protected]>
1 parent d3b2492 commit 085bd8a

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
@@ -3796,18 +3796,19 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
37963796

37973797
// stargz layers should be lazy even for executing something on them
37983798
def, err = baseDef.
3799-
Run(llb.Args([]string{"/bin/touch", "/bar"})).
3799+
Run(llb.Args([]string{"sh", "-c", "cat /dev/urandom | head -c 100 | sha256sum > unique"})).
38003800
Marshal(sb.Context())
38013801
require.NoError(t, err)
38023802
target := registry + "/buildkit/testlazyimage:" + identity.NewID()
3803-
_, err = c.Solve(sb.Context(), def, SolveOpt{
3803+
resp, err := c.Solve(sb.Context(), def, SolveOpt{
38043804
Exports: []ExportEntry{
38053805
{
38063806
Type: ExporterImage,
38073807
Attrs: map[string]string{
38083808
"name": target,
38093809
"push": "true",
38103810
"store": "true",
3811+
"oci-mediatypes": "true",
38113812
"unsafe-internal-store-allow-incomplete": "true",
38123813
},
38133814
},
@@ -3820,9 +3821,25 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
38203821
},
38213822
},
38223823
},
3824+
CacheExports: []CacheOptionsEntry{
3825+
{
3826+
Type: "registry",
3827+
Attrs: map[string]string{
3828+
"ref": sgzCache,
3829+
"compression": "estargz",
3830+
"oci-mediatypes": "true",
3831+
},
3832+
},
3833+
},
38233834
}, nil)
38243835
require.NoError(t, err)
38253836

3837+
dgst, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
3838+
require.Equal(t, ok, true)
3839+
3840+
unique, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst, "/unique")
3841+
require.NoError(t, err)
3842+
38263843
img, err := imageService.Get(ctx, target)
38273844
require.NoError(t, err)
38283845

@@ -3843,6 +3860,40 @@ func testStargzLazyRegistryCacheImportExport(t *testing.T, sb integration.Sandbo
38433860
_, err = contentStore.Info(ctx, manifest.Layers[len(manifest.Layers)-1].Digest)
38443861
require.NoError(t, err)
38453862

3863+
// Run build again and check if cache is reused
3864+
resp, err = c.Solve(sb.Context(), def, SolveOpt{
3865+
Exports: []ExportEntry{
3866+
{
3867+
Type: ExporterImage,
3868+
Attrs: map[string]string{
3869+
"name": target,
3870+
"push": "true",
3871+
"store": "true",
3872+
"oci-mediatypes": "true",
3873+
"unsafe-internal-store-allow-incomplete": "true",
3874+
},
3875+
},
3876+
},
3877+
CacheImports: []CacheOptionsEntry{
3878+
{
3879+
Type: "registry",
3880+
Attrs: map[string]string{
3881+
"ref": sgzCache,
3882+
},
3883+
},
3884+
},
3885+
}, nil)
3886+
require.NoError(t, err)
3887+
3888+
dgst2, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
3889+
require.Equal(t, ok, true)
3890+
3891+
unique2, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst2, "/unique")
3892+
require.NoError(t, err)
3893+
3894+
require.Equal(t, dgst, dgst2)
3895+
require.EqualValues(t, unique, unique2)
3896+
38463897
// clear all local state out
38473898
err = imageService.Delete(ctx, img.Name, images.SynchronousDelete())
38483899
require.NoError(t, err)
@@ -4086,11 +4137,11 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
40864137

40874138
// stargz layers should be lazy even for executing something on them
40884139
def, err = llb.Image(sgzImage).
4089-
Run(llb.Args([]string{"/bin/touch", "/foo"})).
4140+
Run(llb.Args([]string{"sh", "-c", "cat /dev/urandom | head -c 100 | sha256sum > unique"})).
40904141
Marshal(sb.Context())
40914142
require.NoError(t, err)
40924143
target := registry + "/buildkit/testlazyimage:" + identity.NewID()
4093-
_, err = c.Solve(sb.Context(), def, SolveOpt{
4144+
resp, err := c.Solve(sb.Context(), def, SolveOpt{
40944145
Exports: []ExportEntry{
40954146
{
40964147
Type: ExporterImage,
@@ -4106,6 +4157,12 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
41064157
}, nil)
41074158
require.NoError(t, err)
41084159

4160+
dgst, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
4161+
require.Equal(t, ok, true)
4162+
4163+
unique, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst, "/unique")
4164+
require.NoError(t, err)
4165+
41094166
img, err := imageService.Get(ctx, target)
41104167
require.NoError(t, err)
41114168

@@ -4126,6 +4183,32 @@ func testStargzLazyPull(t *testing.T, sb integration.Sandbox) {
41264183
_, err = contentStore.Info(ctx, manifest.Layers[len(manifest.Layers)-1].Digest)
41274184
require.NoError(t, err)
41284185

4186+
// Run build again and check if cache is reused
4187+
resp, err = c.Solve(sb.Context(), def, SolveOpt{
4188+
Exports: []ExportEntry{
4189+
{
4190+
Type: ExporterImage,
4191+
Attrs: map[string]string{
4192+
"name": target,
4193+
"push": "true",
4194+
"store": "true",
4195+
"oci-mediatypes": "true",
4196+
"unsafe-internal-store-allow-incomplete": "true",
4197+
},
4198+
},
4199+
},
4200+
}, nil)
4201+
require.NoError(t, err)
4202+
4203+
dgst2, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
4204+
require.Equal(t, ok, true)
4205+
4206+
unique2, err := readFileInImage(sb.Context(), t, c, target+"@"+dgst2, "/unique")
4207+
require.NoError(t, err)
4208+
4209+
require.Equal(t, dgst, dgst2)
4210+
require.EqualValues(t, unique, unique2)
4211+
41294212
// clear all local state out
41304213
err = imageService.Delete(ctx, img.Name, images.SynchronousDelete())
41314214
require.NoError(t, err)

‎control/control.go

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

‎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)