From 35013437f3006502218589ef1daf0e500023b7f3 Mon Sep 17 00:00:00 2001 From: hiroTamada Date: Fri, 27 Feb 2026 11:45:11 -0500 Subject: [PATCH 1/4] fix: delete orphaned digest directories when last tag is removed Previously, DeleteImage only removed the tag symlink, leaving the digest directory (containing the erofs rootfs) on disk indefinitely. This caused orphaned digests to accumulate over time. Now DeleteImage checks if the digest is orphaned after removing the tag, and deletes the digest directory if no other tags reference it. This is eager GC - cleanup happens immediately at delete time. Made-with: Cursor --- lib/images/README.md | 2 +- lib/images/manager.go | 28 +++++++++++++++++- lib/images/manager_test.go | 59 ++++++++++++++++++++++++++++++++++++-- lib/images/storage.go | 29 +++++++++++++++++++ 4 files changed, 114 insertions(+), 4 deletions(-) diff --git a/lib/images/README.md b/lib/images/README.md index ad698294..eb2d422b 100644 --- a/lib/images/README.md +++ b/lib/images/README.md @@ -90,7 +90,7 @@ Content-addressable storage with tag symlinks (similar to Docker/Unikraft): - Pulling same tag twice updates the symlink if digest changed - OCI cache uses digest hex as layout tag for true content-addressable caching - Shared blob storage enables automatic layer deduplication across all images -- Old digests remain until explicitly garbage collected +- Orphaned digests are automatically deleted when the last tag referencing them is removed - Symlinks only created after successful build (status: ready) ## Reference Handling (reference.go) diff --git a/lib/images/manager.go b/lib/images/manager.go index f5e5468d..92e080e6 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -421,7 +421,33 @@ func (m *manager) DeleteImage(ctx context.Context, name string) error { repository := ref.Repository() tag := ref.Tag() - return deleteTag(m.paths, repository, tag) + // Resolve the tag to get the digest before deleting + digestHex, err := resolveTag(m.paths, repository, tag) + if err != nil { + return err + } + + // Delete the tag symlink + if err := deleteTag(m.paths, repository, tag); err != nil { + return err + } + + // Check if the digest is now orphaned (no other tags reference it) + count, err := countTagsForDigest(m.paths, repository, digestHex) + if err != nil { + // Log but don't fail - tag was already deleted successfully + return nil + } + + if count == 0 { + // Digest is orphaned, delete it + if err := deleteDigest(m.paths, repository, digestHex); err != nil { + // Log but don't fail - tag was already deleted successfully + return nil + } + } + + return nil } // TotalImageBytes returns the total size of all ready images on disk. diff --git a/lib/images/manager_test.go b/lib/images/manager_test.go index b3b74efd..90d3c560 100644 --- a/lib/images/manager_test.go +++ b/lib/images/manager_test.go @@ -11,6 +11,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -228,10 +229,10 @@ func TestDeleteImage(t *testing.T) { _, err = mgr.GetImage(ctx, created.Name) require.ErrorIs(t, err, ErrNotFound) - // But digest directory should still exist + // Digest directory should also be deleted (no orphaned digests) digestDir := digestPath(paths.New(dataDir), ref.Repository(), digestHex) _, err = os.Stat(digestDir) - require.NoError(t, err) + require.True(t, os.IsNotExist(err), "digest directory should be deleted when orphaned") } func TestDeleteImageNotFound(t *testing.T) { @@ -245,6 +246,60 @@ func TestDeleteImageNotFound(t *testing.T) { require.ErrorIs(t, err, ErrNotFound) } +func TestDeleteImagePreservesSharedDigest(t *testing.T) { + dataDir := t.TempDir() + p := paths.New(dataDir) + mgr, err := NewManager(p, 1, nil) + require.NoError(t, err) + + ctx := context.Background() + req := CreateImageRequest{ + Name: "docker.io/library/alpine:latest", + } + + created, err := mgr.CreateImage(ctx, req) + require.NoError(t, err) + + waitForReady(t, mgr, ctx, created.Name) + + // Get the digest + img, err := mgr.GetImage(ctx, created.Name) + require.NoError(t, err) + ref, err := ParseNormalizedRef(img.Name) + require.NoError(t, err) + digestHex := strings.SplitN(img.Digest, ":", 2)[1] + + // Create a second tag pointing to the same digest + err = createTagSymlink(p, ref.Repository(), "v1.0", digestHex) + require.NoError(t, err) + + // Delete the first tag + err = mgr.DeleteImage(ctx, created.Name) + require.NoError(t, err) + + // First tag should be gone + _, err = mgr.GetImage(ctx, created.Name) + require.ErrorIs(t, err, ErrNotFound) + + // Digest directory should still exist (shared by v1.0 tag) + digestDir := digestPath(p, ref.Repository(), digestHex) + _, err = os.Stat(digestDir) + require.NoError(t, err, "digest directory should be preserved when other tags reference it") + + // Second tag should still work + img2, err := mgr.GetImage(ctx, "docker.io/library/alpine:v1.0") + require.NoError(t, err) + assert.Equal(t, img.Digest, img2.Digest) + + // Now delete the second tag + err = mgr.DeleteImage(ctx, "docker.io/library/alpine:v1.0") + require.NoError(t, err) + + // Now the digest directory should be deleted + _, err = os.Stat(digestDir) + require.True(t, os.IsNotExist(err), "digest directory should be deleted when last tag is removed") +} + func TestNormalizedRefParsing(t *testing.T) { tests := []struct { input string diff --git a/lib/images/storage.go b/lib/images/storage.go index 63d730ae..26d73271 100644 --- a/lib/images/storage.go +++ b/lib/images/storage.go @@ -288,3 +288,32 @@ func deleteTag(p *paths.Paths, repository, tag string) error { return nil } + +// countTagsForDigest counts how many tags in a repository point to a given digest +func countTagsForDigest(p *paths.Paths, repository, digestHex string) (int, error) { + tags, err := listTags(p, repository) + if err != nil { + return 0, err + } + + count := 0 + for _, tag := range tags { + target, err := resolveTag(p, repository, tag) + if err != nil { + continue + } + if target == digestHex { + count++ + } + } + return count, nil +} + +// deleteDigest removes a digest directory and all its contents +func deleteDigest(p *paths.Paths, repository, digestHex string) error { + dir := digestDir(p, repository, digestHex) + if err := os.RemoveAll(dir); err != nil { + return fmt.Errorf("remove digest directory: %w", err) + } + return nil +} From f6492dec1d60bbbfb9b0d04d43af5bc21d1388a2 Mon Sep 17 00:00:00 2001 From: hiroTamada Date: Fri, 27 Feb 2026 11:51:47 -0500 Subject: [PATCH 2/4] fix: update oapi-codegen/runtime to v1.2.0 for CI compatibility The CI workflow runs `make oapi-generate` which uses `oapi-codegen@latest`. The latest generator outputs code using `runtime.StyleParamWithOptions` which was added in runtime v1.2.0, but go.mod had v1.1.2 pinned. This caused CI failures on main starting with commit 08958b8. Made-with: Cursor --- go.mod | 2 +- go.sum | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index bf2cbdd1..aea5119c 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/miekg/dns v1.1.68 github.com/nrednav/cuid2 v1.1.0 github.com/oapi-codegen/nethttp-middleware v1.1.2 - github.com/oapi-codegen/runtime v1.1.2 + github.com/oapi-codegen/runtime v1.2.0 github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 github.com/opencontainers/runtime-spec v1.2.1 diff --git a/go.sum b/go.sum index 7e44f095..d2e1ff06 100644 --- a/go.sum +++ b/go.sum @@ -191,6 +191,8 @@ github.com/oapi-codegen/nethttp-middleware v1.1.2 h1:TQwEU3WM6ifc7ObBEtiJgbRPaCe github.com/oapi-codegen/nethttp-middleware v1.1.2/go.mod h1:5qzjxMSiI8HjLljiOEjvs4RdrWyMPKnExeFS2kr8om4= github.com/oapi-codegen/runtime v1.1.2 h1:P2+CubHq8fO4Q6fV1tqDBZHCwpVpvPg7oKiYzQgXIyI= github.com/oapi-codegen/runtime v1.1.2/go.mod h1:SK9X900oXmPWilYR5/WKPzt3Kqxn/uS/+lbpREv+eCg= +github.com/oapi-codegen/runtime v1.2.0 h1:RvKc1CVS1QeKSNzO97FBQbSMZyQ8s6rZd+LpmzwHMP4= +github.com/oapi-codegen/runtime v1.2.0/go.mod h1:Y7ZhmmlE8ikZOmuHRRndiIm7nf3xcVv+YMweKgG1DT0= github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 h1:G7ERwszslrBzRxj//JalHPu/3yz+De2J+4aLtSRlHiY= github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037/go.mod h1:2bpvgLBZEtENV5scfDFEtB/5+1M4hkQhDQrccEJ/qGw= github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 h1:bQx3WeLcUWy+RletIKwUIt4x3t8n2SxavmoclizMb8c= @@ -250,6 +252,7 @@ github.com/u-root/uio v0.0.0-20240224005618-d2acac8f3701 h1:pyC9PaHYZFgEKFdlp3G8 github.com/u-root/uio v0.0.0-20240224005618-d2acac8f3701/go.mod h1:P3a5rG4X7tI17Nn3aOIAYr5HbIMukwXG0urG0WuL8OA= github.com/ugorji/go/codec v1.2.11 h1:BMaWp1Bb6fHwEtbplGBGJ498wD+LKlNSl25MjdZY4dU= github.com/ugorji/go/codec v1.2.11/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= github.com/vbatts/go-mtree v0.6.1-0.20250911112631-8307d76bc1b9 h1:R6l9BtUe83abUGu1YKGkfa17wMMFLt6mhHVQ8MxpfRE= github.com/vbatts/go-mtree v0.6.1-0.20250911112631-8307d76bc1b9/go.mod h1:W7bcG9PCn6lFY+ljGlZxx9DONkxL3v8a7HyN+PrSrjA= github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo= From 5c91a8b6919c48ce92e7f24357fba37101416d97 Mon Sep 17 00:00:00 2001 From: hiroTamada Date: Fri, 27 Feb 2026 11:54:29 -0500 Subject: [PATCH 3/4] fix: pin oapi-codegen to v2.5.1 to match committed generated code The CI workflow regenerates oapi.go using `oapi-codegen@latest`. When v2.6.0 was released, it changed struct tag generation (adding omitempty to some fields), causing type mismatches with code in instances.go that uses inline struct literals. Pinning to v2.5.1 ensures CI regenerates the same code that's committed. Made-with: Cursor --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index e2656e14..fdeb2f56 100644 --- a/Makefile +++ b/Makefile @@ -13,9 +13,9 @@ AIR ?= $(BIN_DIR)/air WIRE ?= $(BIN_DIR)/wire XCADDY ?= $(BIN_DIR)/xcaddy -# Install oapi-codegen +# Install oapi-codegen (pinned to match committed generated code) $(OAPI_CODEGEN): | $(BIN_DIR) - GOBIN=$(BIN_DIR) go install github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@latest + GOBIN=$(BIN_DIR) go install github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@v2.5.1 # Install air for hot reload $(AIR): | $(BIN_DIR) From 0f6b3081cf9c2e701f6aae66cec39f596744e301 Mon Sep 17 00:00:00 2001 From: hiroTamada Date: Fri, 27 Feb 2026 12:07:48 -0500 Subject: [PATCH 4/4] fix: address review feedback - race condition and silent errors 1. Race condition (medium severity): Hold createMu during the orphan check and delete sequence to prevent a concurrent CreateImage from creating a tag pointing to the same digest between count and delete. 2. Silent errors (low severity): Actually log errors when countTagsForDigest or deleteDigest fails, instead of silently swallowing them. Made-with: Cursor --- lib/images/manager.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/images/manager.go b/lib/images/manager.go index 92e080e6..ac170c0e 100644 --- a/lib/images/manager.go +++ b/lib/images/manager.go @@ -432,17 +432,23 @@ func (m *manager) DeleteImage(ctx context.Context, name string) error { return err } + // Hold createMu during orphan check and delete to prevent race with CreateImage. + // Without this lock, a concurrent CreateImage could create a new tag pointing to + // the same digest between our count check and delete, leaving a dangling symlink. + m.createMu.Lock() + defer m.createMu.Unlock() + // Check if the digest is now orphaned (no other tags reference it) count, err := countTagsForDigest(m.paths, repository, digestHex) if err != nil { - // Log but don't fail - tag was already deleted successfully + fmt.Fprintf(os.Stderr, "Warning: failed to count tags for digest %s: %v\n", digestHex, err) return nil } if count == 0 { // Digest is orphaned, delete it if err := deleteDigest(m.paths, repository, digestHex); err != nil { - // Log but don't fail - tag was already deleted successfully + fmt.Fprintf(os.Stderr, "Warning: failed to delete orphaned digest %s: %v\n", digestHex, err) return nil } }