Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ require (
unikraft.com/x/colors v0.0.0-20260304162956-523940cab1de
unikraft.com/x/fingerprint v0.0.0-20260126094137-ab6e717e5679
unikraft.com/x/guesstermwidth v0.0.0-20260304162956-523940cab1de
unikraft.com/x/image-spec v0.0.0-20260304162956-523940cab1de
unikraft.com/x/image-spec v0.0.0-20260320164959-32db9e2896d2
unikraft.com/x/joinerrgroup v0.0.0-20260304162956-523940cab1de
unikraft.com/x/kingkong v0.0.0-20260304162956-523940cab1de
unikraft.com/x/kraftfile v0.0.0-20260318103446-c2c548a69fc0
Expand Down Expand Up @@ -77,7 +77,7 @@ require (
github.com/containerd/console v1.0.5 // indirect
github.com/containerd/containerd/api v1.10.0 // indirect
github.com/containerd/continuity v0.4.5 // indirect
github.com/containerd/errdefs v1.0.0 // indirect
github.com/containerd/errdefs v1.0.0
github.com/containerd/errdefs/pkg v0.3.0 // indirect
github.com/containerd/ttrpc v1.2.7 // indirect
github.com/containerd/typeurl/v2 v2.2.3 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ unikraft.com/x/fingerprint v0.0.0-20260126094137-ab6e717e5679 h1:zdvJjNkjsriS8RM
unikraft.com/x/fingerprint v0.0.0-20260126094137-ab6e717e5679/go.mod h1:FP7uOxux/W5PKqSRQsR4tyjNuLq4Cfio7mc5QVH1kW8=
unikraft.com/x/guesstermwidth v0.0.0-20260304162956-523940cab1de h1:1xafSiBA1yfMvhnM3q1baUliqkV6wkE1AXxiOSUkJSA=
unikraft.com/x/guesstermwidth v0.0.0-20260304162956-523940cab1de/go.mod h1:q3EH6bLqLAJ1PqZgHlCJPpvvP6scnjJJspBMyGQtMt4=
unikraft.com/x/image-spec v0.0.0-20260304162956-523940cab1de h1:D0yMS95zf5Vgi4791JlxQDli3BRPZXLOjQmjVrqFjVI=
unikraft.com/x/image-spec v0.0.0-20260304162956-523940cab1de/go.mod h1:l0+tgd72OA6DzrX+V7z9XYjd80PrMJMCPnXNTODBjrc=
unikraft.com/x/image-spec v0.0.0-20260320164959-32db9e2896d2 h1:qV/HvP7q5Ckp3lXUr/K5gy1aTXRQcVlG4alhtnWnh+0=
unikraft.com/x/image-spec v0.0.0-20260320164959-32db9e2896d2/go.mod h1:l0+tgd72OA6DzrX+V7z9XYjd80PrMJMCPnXNTODBjrc=
unikraft.com/x/joinerrgroup v0.0.0-20260304162956-523940cab1de h1:cm4FnPvnahRIK0derbI+T4ds1LsD5CFeyyAvIqcOCek=
unikraft.com/x/joinerrgroup v0.0.0-20260304162956-523940cab1de/go.mod h1:XND1VvLxwqKFGrmdwUTWps4WEpMm7HTHPQg9HWQtrxg=
unikraft.com/x/kingkong v0.0.0-20260304162956-523940cab1de h1:jrsspHGxv0kXZG/9aZRQNwJidV/ZmI2ywkw2Zh4ZbeE=
Expand Down
5 changes: 3 additions & 2 deletions internal/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
imagespec "unikraft.com/x/image-spec"

ximagespec "unikraft.com/cli/internal/x/imagespec"
"unikraft.com/x/kraftfile"
)

Expand Down Expand Up @@ -100,8 +101,8 @@ func Build(ctx context.Context, opts BuildOpts, c *client.Client) ([]*imagespec.
}

images = append(images, imagespec.NewImage(
imagespec.WithKernel(kernel.Kernel),
imagespec.WithInitrd(root.Initrd),
imagespec.WithKernel(ximagespec.WrapCached(ctx, kernel.Kernel)),
imagespec.WithInitrd(ximagespec.WrapCached(ctx, root.Initrd)),
imagespec.WithImageConfig(root.Image.Config),
imagespec.WithImageMetadata(meta),
imagespec.WithPlatform(root.Image.Platform),
Expand Down
132 changes: 132 additions & 0 deletions internal/x/imagespec/cache.go
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay to have no automatic cache cleanup? Probably not.

Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// SPDX-License-Identifier: BSD-3-Clause
// Copyright (c) 2026, Unikraft GmbH and The Unikraft CLI Authors.
// Licensed under the BSD-3-Clause License (the "License").
// You may not use this file except in compliance with the License.

package imagespec

import (
"context"
"os"
"path/filepath"
"sync"

"github.com/containerd/containerd/v2/core/content"
"github.com/containerd/containerd/v2/plugins/content/local"
"github.com/containerd/errdefs"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
spec "unikraft.com/x/image-spec"
"unikraft.com/x/log"
)

// WrapCached wraps a File with caching support. If the file has a backing
// provider and descriptor, it returns a new ContentStoreFile that uses a
// pull-through cache at ~/.cache/unikraft. Otherwise, returns the original file.
func WrapCached(ctx context.Context, file spec.File) spec.File {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, we should try and only cache if it's fetched from a remote registry. If it's a local content store, then probably not.

if file == nil {
return nil
}

desc, provider := file.Source()
if desc.Digest == "" || provider == nil {
return file
}

store, err := cacheStore()
if err != nil {
log.G(ctx).Debug().Err(err).Msg("failed to initialize content cache")
return file
}

return spec.NewContentStoreFile(
pullThroughProvider{cache: store, upstream: provider},
desc,
file.Path(),
)
}

type pullThroughProvider struct {
cache content.Store
upstream content.Provider
}

func (p pullThroughProvider) ReaderAt(ctx context.Context, desc ocispec.Descriptor) (content.ReaderAt, error) {
if p.cache != nil {
ra, err := p.cache.ReaderAt(ctx, desc)
if err == nil {
log.G(ctx).Debug().
Str("digest", desc.Digest.String()).
Msg("content cache hit")
return ra, nil
}
if !errdefs.IsNotFound(err) {
log.G(ctx).Debug().
Err(err).
Str("digest", desc.Digest.String()).
Msg("content cache read failed")
}
}

ra, err := p.upstream.ReaderAt(ctx, desc)
if err != nil {
return nil, err
}

if p.cache != nil {
if err := cacheBlob(ctx, p.cache, ra, desc); err != nil {
log.G(ctx).Debug().
Err(err).
Str("digest", desc.Digest.String()).
Msg("failed to write content to cache")
return ra, nil
}
// Close the upstream reader and return from the cache instead
ra.Close()
return p.cache.ReaderAt(ctx, desc)
Comment on lines +83 to +85
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the cache-write success path, this closes the upstream reader and then blindly switches to p.cache.ReaderAt. If cacheBlob was a no-op (e.g., it returns nil when desc.Size <= 0), or if the cache read fails for any reason, this will return an error even though the upstream reader was valid (and has now been closed). Consider only closing upstream after successfully opening the cached reader, and falling back to the upstream reader when cache open fails; alternatively, make cacheBlob return an explicit "cached" boolean so the caller doesn’t assume the blob exists in the cache.

Suggested change
// Close the upstream reader and return from the cache instead
ra.Close()
return p.cache.ReaderAt(ctx, desc)
// Try to open a new reader from the cache after writing.
cachedRA, err := p.cache.ReaderAt(ctx, desc)
if err == nil {
// Successfully opened from cache: close upstream and return cached reader.
ra.Close()
log.G(ctx).Debug().
Str("digest", desc.Digest.String()).
Msg("content cache read after write")
return cachedRA, nil
}
// If the cache read after write fails for any reason, fall back to the
// still-open upstream reader instead of returning an error.
log.G(ctx).Debug().
Err(err).
Str("digest", desc.Digest.String()).
Msg("content cache read after write failed; using upstream reader")
return ra, nil

Copilot uses AI. Check for mistakes.
}

return ra, nil
}

func cacheBlob(ctx context.Context, store content.Store, ra content.ReaderAt, desc ocispec.Descriptor) error {
if desc.Digest == "" {
return nil
}
if desc.Size <= 0 {
desc.Size = ra.Size()
}
if desc.Size <= 0 {
return nil
}
Comment on lines +95 to +100
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cacheBlob returns nil even when it decides it cannot write anything (e.g., missing/unknown size). Callers currently interpret nil as "cached successfully". Consider returning a sentinel error or an additional boolean indicating whether a blob was actually written, so callers can safely decide whether to switch to the cache.

Copilot uses AI. Check for mistakes.
return content.WriteBlob(ctx, store, desc.Digest.String(), content.NewReader(ra), desc)
}

var (
cacheStoreOnce sync.Once
cacheStoreInst content.Store
cacheStoreErr error
)

func cacheStore() (content.Store, error) {
cacheStoreOnce.Do(func() {
root, err := cacheRoot()
if err != nil {
cacheStoreErr = err
return
}
if err := os.MkdirAll(root, 0o755); err != nil {
cacheStoreErr = err
return
}
cacheStoreInst, cacheStoreErr = local.NewStore(root)
})
return cacheStoreInst, cacheStoreErr
}

func cacheRoot() (string, error) {
dir, err := os.UserCacheDir()
if err != nil {
return "", err
}
return filepath.Join(dir, "unikraft"), nil
}
Loading