Skip to content

Conversation

@ktock
Copy link
Collaborator

@ktock ktock commented Jul 18, 2025

Found in #6092

https://github.com/moby/buildkit/actions/runs/16344069969/job/46175402958?pr=6092#step:8:4328

 === FAIL: cache TestGetRemotes (1.05s)
    manager_test.go:1783: 
        	Error Trace:	/src/cache/manager_test.go:1783
        	            				/src/vendor/golang.org/x/sync/errgroup/errgroup.go:130
        	            				/usr/local/go/src/runtime/asm_amd64.s:1700
        	Error:      	An error is expected but got nil.
        	Test:       	TestGetRemotes

This is caused by the following part of test in TestGetRemotes isn't thread safe.

buildkit/cache/manager_test.go

Lines 1773 to 1788 in f7607f3

r := refChain[i]
isLazy, err := r.isLazy(egctx)
require.NoError(t, err)
needs, err := compressionType.NeedsConversion(ctx, co.cs, desc)
require.NoError(t, err)
if needs {
require.False(t, isLazy, "layer %q requires conversion so it must be unlazied", desc.Digest)
}
bDesc, err := r.getBlobWithCompression(egctx, compressionType)
if isLazy {
require.Error(t, err)
} else {
require.NoError(t, err)
checkDescriptor(ctx, t, co.cs, bDesc, compressionType)
require.Equal(t, desc.Digest, bDesc.Digest)
}

Concretely, the following sequence can occur.

  1. A goroutine calls r.isLazy()
  2. The laziness is changed by GetRemotes() of another goroutine, because that function can unlazy the blob.
  3. The first goroutine checks whether the laziness returned by r.getBlobWithCompression() matches to the earlier r.isLazy() call. But the laziness was changed by other goroutine so the test fails.

This isn't a bug of the cache but is a bug in the test. this PR fixes this flakiness by adding a mutex to protect the part of the test.

Although the failure rate wasn't high (about 1/200 when I run the test during debugging #6092), this can be reproduced by forcing the timing shown in the above steps (c.f. ktock@1ddcea9), which can be fixed by the same change as this PR (c.f. ktock@522d82a).

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
@tonistiigi tonistiigi merged commit 34a91d2 into moby:master Jul 18, 2025
140 checks passed
@ktock ktock deleted the fix-getremotes branch July 19, 2025 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants