Skip to content

Commit

Permalink
mountmanager: handle lazy blob ref loading in cache mounts
Browse files Browse the repository at this point in the history
Before this change, if a cache mount had base layers from a ref and
those layers were lazy, you could hit missing blob errors when trying to
reload an existing mutable ref for the cache mount.

It's possible to have lazy refs in the base layers when blobs get
deduped by chainID.

The fix is just to handle the lazy blob error and reload with
descHandlers set.

Signed-off-by: Erik Sipsma <[email protected]>
  • Loading branch information
sipsma committed Dec 2, 2024
1 parent e77465f commit 239a894
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 4 deletions.
104 changes: 101 additions & 3 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
testLayerLimitOnMounts,
testFrontendVerifyPlatforms,
testRunValidExitCodes,
testSameChainIDWithLazyBlobs,
testSameChainIDWithLazyBlobsCacheExport,
testSameChainIDWithLazyBlobsCacheMountBase,
}

func TestIntegration(t *testing.T) {
Expand Down Expand Up @@ -10897,7 +10898,7 @@ func testRunValidExitCodes(t *testing.T, sb integration.Sandbox) {
require.ErrorContains(t, err, "exit code: 0")
}

func testSameChainIDWithLazyBlobs(t *testing.T, sb integration.Sandbox) {
func testSameChainIDWithLazyBlobsCacheExport(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb,
workers.FeatureCacheExport,
workers.FeatureCacheImport,
Expand Down Expand Up @@ -10936,7 +10937,7 @@ func testSameChainIDWithLazyBlobs(t *testing.T, sb integration.Sandbox) {
require.NoError(t, err)

Check failure on line 10937 in client/client_test.go

View workflow job for this annotation

GitHub Actions / test-windows-amd64 (containerd, ./client)

Failed: client/TestIntegration/TestSameChainIDWithLazyBlobsCacheExport/worker=containerd

=== RUN TestIntegration/TestSameChainIDWithLazyBlobsCacheExport/worker=containerd client_test.go:10937: Error Trace: D:/a/buildkit/buildkit/client/client_test.go:10937 D:/a/buildkit/buildkit/util/testutil/integration/run.go:97 D:/a/buildkit/buildkit/util/testutil/integration/run.go:211 Error: Received unexpected error: failed to load cache key: no match for platform in manifest: not found github.com/moby/buildkit/util/stack.Enable D:/a/buildkit/buildkit/util/stack/stack.go:82 github.com/moby/buildkit/util/grpcerrors.FromGRPC D:/a/buildkit/buildkit/util/grpcerrors/grpcerrors.go:204 github.com/moby/buildkit/util/grpcerrors.UnaryClientInterceptor D:/a/buildkit/buildkit/util/grpcerrors/intercept.go:41 google.golang.org/grpc.(*ClientConn).Invoke D:/a/buildkit/buildkit/vendor/google.golang.org/grpc/call.go:35 github.com/moby/buildkit/api/services/control.(*controlClient).Solve D:/a/buildkit/buildkit/api/services/control/control_grpc.pb.go:88 github.com/moby/buildkit/client.(*Client).solve.func2 D:/a/buildkit/buildkit/client/solve.go:269 golang.org/x/sync/errgroup.(*Group).Go.func1 D:/a/buildkit/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:78 runtime.goexit C:/hostedtoolcache/windows/go/1.23.3/x64/src/runtime/asm_amd64.s:1700 failed to solve github.com/moby/buildkit/client.(*Client).solve.func2 D:/a/buildkit/buildkit/client/solve.go:285 golang.org/x/sync/errgroup.(*Group).Go.func1 D:/a/buildkit/buildkit/vendor/golang.org/x/sync/errgroup/errgroup.go:78 runtime.goexit C:/hostedtoolcache/windows/go/1.23.3/x64/src/runtime/asm_amd64.s:1700 Test: TestIntegration/TestSameChainIDWithLazyBlobsCacheExport/worker=containerd sandbox.go:135: stdout: D:\a\buildkit\buildkit\bin\containerd.exe --config C:\Users\RUNNER~1\AppData\Local\Temp\bktest_containerd1289084452\config.toml sandbox.go:135: stderr: D:\a\buildkit\buildkit\bin\containerd.exe --config C:\Users\RUNNER~1\AppData\Local\Temp\bktest_containerd1289084452\config.toml sandbox.go:138: > StartCmd 2024-12-02 19:54:10.8919557 +0000 UTC m=+97.873084701 D:\a\buildkit\buildkit\bin\containerd.exe --config C:\Users\RUNNER~1\AppData\Local\Temp\bktest_containerd1289084452\config.toml sandbox.go:138: time="2024-12-02T19:54:10.913006400Z" level=debug msg="Stackdump - waiting signal at Global\\stackdump-2756" sandbox.go:138: time="2024-12-02T19:54:10.913006400Z" level=info msg="starting containerd" revision= version=2.0.0+unknown sandbox.go:138: time="2024-12-02T19:54:10.937763600Z" level=info msg="loading plugin" id=io.containerd.internal.v1.opt type=io.containerd.internal.v1 sandbox.go:138: time="2024-12-02T19:54:10.943440100Z" level=info msg="loading plugin" id=io.containerd.content.v1.content type=io.containerd.content.v1 sandbox.go:138: time="2024-12-02T19:54:10.943440100Z" level=info msg="loading plugin" id=io.containerd.image-verifier.v1.bindir type=io.containerd.image-verifier.v1 sandbox.go:138: time="2024-12-02T19:54:10.943440100Z" level=info msg="loading plugin" id=io.containerd.warning.v1.deprecations type=io.containerd.warning.v1 sandbox.go:138: time="2024-12-02T19:54:10.943440100Z" level=info msg="loading plugin" id=io.containerd.event.v1.exchange type=io.containerd.event.v1 sandbox.go:138: time="2024-12-02T19:54:10.943440100Z" level=info msg="loading plugin" id=io.containerd.snapshotter.v1.windows-lcow type=io.containerd.snapshotter.v1 sandbox.go:138: time="2024-12-02T19:54:10.944192200Z" level=info msg=

// push the base busybox image plus an extra layer, ensuring it uses zstd
// the extra layer allows us to avoid edge merge later
// the extra layer allows us to avoid edge-merge/cache-load later
def, err = llb.Image("busybox:latest").
Run(llb.Shlex(`touch /foo`)).Root().
Marshal(sb.Context())
Expand Down Expand Up @@ -11037,3 +11038,100 @@ func testSameChainIDWithLazyBlobs(t *testing.T, sb integration.Sandbox) {

require.Equal(t, string(rand1), string(rand2))
}

func testSameChainIDWithLazyBlobsCacheMountBase(t *testing.T, sb integration.Sandbox) {
workers.CheckFeatureCompat(t, sb,
workers.FeatureCacheExport,
workers.FeatureCacheImport,
workers.FeatureCacheBackendRegistry,
)

c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

// push the base busybox image, ensuring it uses gzip

def, err := llb.Image("busybox:latest").
Marshal(sb.Context())
require.NoError(t, err)
busyboxGzipRef := registry + "/buildkit/busyboxgzip:latest"
_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterImage,
Attrs: map[string]string{
"name": busyboxGzipRef,
"push": "true",
"compression": "gzip",
"force-compression": "true",
},
},
},
}, nil)
require.NoError(t, err)

// push the base busybox image plus an extra layer, ensuring it uses zstd
// the extra layer allows us to avoid edge-merge/cache-load later
def, err = llb.Image("busybox:latest").
Run(llb.Shlex(`touch /foo`)).Root().
Marshal(sb.Context())
require.NoError(t, err)
busyboxZstdRef := registry + "/buildkit/busyboxzstd:latest"
_, err = c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterImage,
Attrs: map[string]string{
"name": busyboxZstdRef,
"push": "true",
"compression": "zstd",
"force-compression": "true",
},
},
},
}, nil)
require.NoError(t, err)

ensurePruneAll(t, c, sb)

// create non-lazy cache refs for the zstd image
def, err = llb.Image(busyboxZstdRef).
Run(llb.Shlex(`true`)).Root().
Marshal(sb.Context())
require.NoError(t, err)
_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.NoError(t, err)

// use the gzip image as a cache mount base, the cache ref will be deduped by
// chainID with the zstd layers made in the previous solve
def, err = llb.Image(busyboxZstdRef).Run(
llb.Shlex(`touch /mnt/bar`),
llb.AddMount("/mnt",
llb.Image(busyboxGzipRef),
llb.AsPersistentCacheDir("idc", llb.CacheMountShared),
),
).Root().Marshal(sb.Context())
require.NoError(t, err)
_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.NoError(t, err)

// try to re-use the cache mount from before, ensure we successfully get
// the same one writen to in previous step
def, err = llb.Image(busyboxZstdRef).Run(
llb.Shlex(`stat /mnt/bar`),
llb.AddMount("/mnt",
llb.Image(busyboxGzipRef),
llb.AsPersistentCacheDir("idc", llb.CacheMountShared),
),
).Root().Marshal(sb.Context())
require.NoError(t, err)
_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.NoError(t, err)
}
14 changes: 13 additions & 1 deletion solver/llbsolver/mounts/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/moby/buildkit/util/grpcerrors"
"github.com/moby/locker"
"github.com/moby/sys/userns"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
"google.golang.org/grpc/codes"
)
Expand Down Expand Up @@ -123,7 +124,18 @@ func (g *cacheRefGetter) getRefCacheDirNoCache(ctx context.Context, key string,
}
locked := false
for _, si := range sis {
if mRef, err := g.cm.GetMutable(ctx, si.ID()); err == nil {
mRef, err := g.cm.GetMutable(ctx, si.ID())
var needsRemoteProviders cache.NeedsRemoteProviderError
if errors.As(err, &needsRemoteProviders) && ref != nil {
descHandlers := cache.DescHandlers(make(map[digest.Digest]*cache.DescHandler))
for _, dgst := range needsRemoteProviders {
if handler := ref.DescHandler(dgst); handler != nil {
descHandlers[dgst] = handler
}
}
mRef, err = g.cm.GetMutable(ctx, si.ID(), descHandlers)
}
if err == nil {
bklog.G(ctx).Debugf("reusing ref for cache dir %q: %s", id, mRef.ID())
return mRef, nil
} else if errors.Is(err, cache.ErrLocked) {
Expand Down

0 comments on commit 239a894

Please sign in to comment.