Skip to content

Commit dcbb752

Browse files
tonistiigitonistiigi
authored
Merge pull request #6292 from tonistiigi/http-release-fix
http: fix release race between cache and snapshot
2 parents 3f7b522 + 23a0dd7 commit dcbb752

File tree

18 files changed

+459
-86
lines changed

18 files changed

+459
-86
lines changed

‎client/client_test.go‎

Lines changed: 169 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import (
7676
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
7777
"github.com/pkg/errors"
7878
"github.com/spdx/tools-golang/spdx"
79+
"github.com/stretchr/testify/assert"
7980
"github.com/stretchr/testify/require"
8081
"github.com/tonistiigi/fsutil"
8182
fsutiltypes "github.com/tonistiigi/fsutil/types"
@@ -243,6 +244,8 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
243244
testMetadataOnlyLocal,
244245
testGitResolveSourceMetadata,
245246
testHTTPResolveSourceMetadata,
247+
testHTTPPruneAfterCacheKey,
248+
testHTTPPruneAfterResolveMeta,
246249
}
247250

248251
func TestIntegration(t *testing.T) {
@@ -3223,13 +3226,13 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) {
32233226

32243227
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
32253228

3226-
resp := httpserver.Response{
3229+
resp := &httpserver.Response{
32273230
Etag: identity.NewID(),
32283231
Content: []byte("content1"),
32293232
LastModified: &modTime,
32303233
}
32313234

3232-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3235+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
32333236
"/foo": resp,
32343237
})
32353238
defer server.Close()
@@ -3293,7 +3296,7 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) {
32933296
require.NoError(t, err)
32943297
require.NoError(t, gw.Close())
32953298
gzipBytes := buf.Bytes()
3296-
respGzip := httpserver.Response{
3299+
respGzip := &httpserver.Response{
32973300
Etag: identity.NewID(),
32983301
Content: gzipBytes,
32993302
LastModified: &modTime,
@@ -3373,18 +3376,18 @@ func testBuildHTTPSourceEtagScope(t *testing.T, sb integration.Sandbox) {
33733376
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
33743377

33753378
sharedEtag := identity.NewID()
3376-
resp := httpserver.Response{
3379+
resp := &httpserver.Response{
33773380
Etag: sharedEtag,
33783381
Content: []byte("content1"),
33793382
LastModified: &modTime,
33803383
}
3381-
resp2 := httpserver.Response{
3384+
resp2 := &httpserver.Response{
33823385
Etag: sharedEtag,
33833386
Content: []byte("another"),
33843387
LastModified: &modTime,
33853388
}
33863389

3387-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3390+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
33883391
"/one/foo": resp,
33893392
"/two/foo": resp2,
33903393
})
@@ -3468,13 +3471,13 @@ func testBuildHTTPSourceAuthHeaderSecret(t *testing.T, sb integration.Sandbox) {
34683471

34693472
modTime := time.Now().Add(-24 * time.Hour) // avoid false positive with current time
34703473

3471-
resp := httpserver.Response{
3474+
resp := &httpserver.Response{
34723475
Etag: identity.NewID(),
34733476
Content: []byte("content1"),
34743477
LastModified: &modTime,
34753478
}
34763479

3477-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3480+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
34783481
"/foo": resp,
34793482
})
34803483
defer server.Close()
@@ -3509,13 +3512,13 @@ func testBuildHTTPSourceHostTokenSecret(t *testing.T, sb integration.Sandbox) {
35093512

35103513
modTime := time.Now().Add(-24 * time.Hour) // avoid false positive with current time
35113514

3512-
resp := httpserver.Response{
3515+
resp := &httpserver.Response{
35133516
Etag: identity.NewID(),
35143517
Content: []byte("content1"),
35153518
LastModified: &modTime,
35163519
}
35173520

3518-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3521+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
35193522
"/foo": resp,
35203523
})
35213524
defer server.Close()
@@ -3550,13 +3553,13 @@ func testBuildHTTPSourceHeader(t *testing.T, sb integration.Sandbox) {
35503553

35513554
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
35523555

3553-
resp := httpserver.Response{
3556+
resp := &httpserver.Response{
35543557
Etag: identity.NewID(),
35553558
Content: []byte("content1"),
35563559
LastModified: &modTime,
35573560
}
35583561

3559-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3562+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
35603563
"/foo": resp,
35613564
})
35623565
defer server.Close()
@@ -12066,19 +12069,19 @@ func testHTTPResolveSourceMetadata(t *testing.T, sb integration.Sandbox) {
1206612069

1206712070
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
1206812071

12069-
resp := httpserver.Response{
12072+
resp := &httpserver.Response{
1207012073
Etag: identity.NewID(),
1207112074
Content: []byte("content1"),
1207212075
LastModified: &modTime,
1207312076
}
1207412077

12075-
resp2 := httpserver.Response{
12078+
resp2 := &httpserver.Response{
1207612079
Etag: identity.NewID(),
1207712080
Content: []byte("content2"),
1207812081
ContentDisposition: "attachment; filename=\"my img.jpg\"",
1207912082
}
1208012083

12081-
server := httpserver.NewTestServer(map[string]httpserver.Response{
12084+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
1208212085
"/foo": resp,
1208312086
"/bar": resp2,
1208412087
})
@@ -12116,6 +12119,157 @@ func testHTTPResolveSourceMetadata(t *testing.T, sb integration.Sandbox) {
1211612119
require.NoError(t, err)
1211712120
}
1211812121

12122+
func testHTTPPruneAfterCacheKey(t *testing.T, sb integration.Sandbox) {
12123+
// this test depends on hitting race condition in internal functions.
12124+
// If debugging and expecting failure you can add small sleep in beginning of source/http.Exec() to hit reliably
12125+
ctx := sb.Context()
12126+
c, err := New(ctx, sb.Address())
12127+
require.NoError(t, err)
12128+
defer c.Close()
12129+
12130+
resp := &httpserver.Response{
12131+
Etag: identity.NewID(),
12132+
Content: []byte("content1"),
12133+
}
12134+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
12135+
"/foo": resp,
12136+
})
12137+
defer server.Close()
12138+
12139+
done := make(chan struct{})
12140+
12141+
startScan := make(chan struct{})
12142+
stopScan := make(chan struct{})
12143+
pauseScan := make(chan struct{})
12144+
12145+
go func() {
12146+
// attempt to prune the HTTP record in between cachekey and snapshot
12147+
defer close(done)
12148+
for {
12149+
select {
12150+
case <-startScan:
12151+
scan:
12152+
for {
12153+
select {
12154+
case <-pauseScan:
12155+
break scan
12156+
default:
12157+
du, err := c.DiskUsage(ctx)
12158+
assert.NoError(t, err)
12159+
for _, entry := range du {
12160+
if entry.Description == "http url "+server.URL+"/foo" {
12161+
if !entry.InUse {
12162+
t.Logf("entry no longer in use, pruning")
12163+
err = c.Prune(ctx, nil)
12164+
assert.NoError(t, err)
12165+
12166+
resp.Etag = identity.NewID()
12167+
resp.Content = []byte("content2")
12168+
}
12169+
}
12170+
}
12171+
}
12172+
}
12173+
case <-stopScan:
12174+
return
12175+
}
12176+
}
12177+
}()
12178+
12179+
const iterations = 10
12180+
for range iterations {
12181+
startScan <- struct{}{}
12182+
resp.Etag = identity.NewID()
12183+
resp.Content = []byte("content1")
12184+
_, err = c.Build(ctx, SolveOpt{}, "test", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
12185+
st := llb.Scratch().File(llb.Copy(llb.HTTP(server.URL+"/foo"), "foo", "bar"))
12186+
def, err := st.Marshal(sb.Context())
12187+
if err != nil {
12188+
return nil, err
12189+
}
12190+
resp, err := c.Solve(ctx, gateway.SolveRequest{
12191+
Definition: def.ToPB(),
12192+
})
12193+
if err != nil {
12194+
return nil, err
12195+
}
12196+
12197+
return resp, nil
12198+
}, nil)
12199+
require.NoError(t, err)
12200+
12201+
pauseScan <- struct{}{}
12202+
12203+
err = c.Prune(ctx, nil)
12204+
require.NoError(t, err)
12205+
12206+
checkAllReleasable(t, c, sb, false)
12207+
}
12208+
close(stopScan)
12209+
<-done
12210+
}
12211+
12212+
// testHTTPPruneAfterResolveMeta ensures that pruning after ResolveSourceMetadata
12213+
// doesn't pull in new data for same build. Once URL has been resolved once for a specific
12214+
// build, the data should be considered immutable and remote changes don't affect ongoing build.
12215+
func testHTTPPruneAfterResolveMeta(t *testing.T, sb integration.Sandbox) {
12216+
ctx := sb.Context()
12217+
c, err := New(ctx, sb.Address())
12218+
require.NoError(t, err)
12219+
defer c.Close()
12220+
12221+
resp := &httpserver.Response{
12222+
Etag: identity.NewID(),
12223+
Content: []byte("content1"),
12224+
}
12225+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
12226+
"/foo": resp,
12227+
})
12228+
defer server.Close()
12229+
12230+
dest := t.TempDir()
12231+
12232+
_, err = c.Build(ctx, SolveOpt{
12233+
Exports: []ExportEntry{
12234+
{
12235+
Type: ExporterLocal,
12236+
OutputDir: dest,
12237+
},
12238+
},
12239+
}, "test", func(ctx context.Context, gc gateway.Client) (*gateway.Result, error) {
12240+
id := server.URL + "/foo"
12241+
md, err := gc.ResolveSourceMetadata(ctx, &pb.SourceOp{
12242+
Identifier: id,
12243+
}, sourceresolver.Opt{})
12244+
if err != nil {
12245+
return nil, err
12246+
}
12247+
require.NotNil(t, md.HTTP)
12248+
12249+
// prune all
12250+
err = c.Prune(ctx, nil)
12251+
require.NoError(t, err)
12252+
12253+
resp.Content = []byte("content2") // etag is same so should hit cache if record not pruned
12254+
12255+
st := llb.Scratch().File(llb.Copy(llb.HTTP(id), "foo", "bar"))
12256+
def, err := st.Marshal(sb.Context())
12257+
if err != nil {
12258+
return nil, err
12259+
}
12260+
return gc.Solve(ctx, gateway.SolveRequest{
12261+
Definition: def.ToPB(),
12262+
})
12263+
}, nil)
12264+
require.NoError(t, err)
12265+
12266+
dt, err := os.ReadFile(filepath.Join(dest, "bar"))
12267+
require.NoError(t, err)
12268+
require.Equal(t, "content1", string(dt))
12269+
12270+
checkAllReleasable(t, c, sb, false)
12271+
}
12272+
1211912273
func runInDir(dir string, cmds ...string) error {
1212012274
for _, args := range cmds {
1212112275
var cmd *exec.Cmd

‎frontend/dockerfile/dockerfile_addchecksum_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ func testAddChecksum(t *testing.T, sb integration.Sandbox) {
2828
f := getFrontend(t, sb)
2929
f.RequiresBuildctl(t)
3030

31-
resp := httpserver.Response{
31+
resp := &httpserver.Response{
3232
Etag: identity.NewID(),
3333
Content: []byte("content1"),
3434
}
35-
server := httpserver.NewTestServer(map[string]httpserver.Response{
35+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
3636
"/foo": resp,
3737
})
3838
defer server.Close()

‎frontend/dockerfile/dockerfile_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,12 +2509,12 @@ RUN echo foo-contents> /foo
25092509
err := os.WriteFile(filepath.Join(srcDir, "Dockerfile"), dockerfile, 0600)
25102510
require.NoError(t, err)
25112511

2512-
resp := httpserver.Response{
2512+
resp := &httpserver.Response{
25132513
Etag: identity.NewID(),
25142514
Content: dockerfile,
25152515
}
25162516

2517-
server := httpserver.NewTestServer(map[string]httpserver.Response{
2517+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
25182518
"/df": resp,
25192519
})
25202520
defer server.Close()
@@ -3061,18 +3061,18 @@ func testDockerfileADDFromURL(t *testing.T, sb integration.Sandbox) {
30613061

30623062
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
30633063

3064-
resp := httpserver.Response{
3064+
resp := &httpserver.Response{
30653065
Etag: identity.NewID(),
30663066
Content: []byte("content1"),
30673067
}
30683068

3069-
resp2 := httpserver.Response{
3069+
resp2 := &httpserver.Response{
30703070
Etag: identity.NewID(),
30713071
LastModified: &modTime,
30723072
Content: []byte("content2"),
30733073
}
30743074

3075-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3075+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
30763076
"/foo": resp,
30773077
"/": resp2,
30783078
})
@@ -3271,12 +3271,12 @@ COPY t.tar.gz /
32713271
require.Equal(t, buf2.Bytes(), dt)
32723272

32733273
// ADD from URL doesn't extract
3274-
resp := httpserver.Response{
3274+
resp := &httpserver.Response{
32753275
Etag: identity.NewID(),
32763276
Content: buf2.Bytes(),
32773277
}
32783278

3279-
server := httpserver.NewTestServer(map[string]httpserver.Response{
3279+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
32803280
"/t.tar.gz": resp,
32813281
})
32823282
defer server.Close()
@@ -4707,11 +4707,11 @@ func testAddURLChmod(t *testing.T, sb integration.Sandbox) {
47074707
f := getFrontend(t, sb)
47084708
f.RequiresBuildctl(t)
47094709

4710-
resp := httpserver.Response{
4710+
resp := &httpserver.Response{
47114711
Etag: identity.NewID(),
47124712
Content: []byte("content1"),
47134713
}
4714-
server := httpserver.NewTestServer(map[string]httpserver.Response{
4714+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
47154715
"/foo": resp,
47164716
})
47174717
defer server.Close()
@@ -4952,12 +4952,12 @@ COPY foo bar
49524952

49534953
require.NoError(t, w.Flush())
49544954

4955-
resp := httpserver.Response{
4955+
resp := &httpserver.Response{
49564956
Etag: identity.NewID(),
49574957
Content: buf.Bytes(),
49584958
}
49594959

4960-
server := httpserver.NewTestServer(map[string]httpserver.Response{
4960+
server := httpserver.NewTestServer(map[string]*httpserver.Response{
49614961
"/myurl": resp,
49624962
})
49634963
defer server.Close()

0 commit comments

Comments
 (0)