Skip to content

Commit 416706e

Browse files
hiroTamadahiroTamada
andauthored
fix: delete orphaned digest directories when last tag is removed (#111)
* 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 * 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 * 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 * 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 --------- Co-authored-by: hiroTamada <hiro@kernel.sh>
1 parent e9afa23 commit 416706e

7 files changed

Lines changed: 125 additions & 6 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ AIR ?= $(BIN_DIR)/air
1414
WIRE ?= $(BIN_DIR)/wire
1515
XCADDY ?= $(BIN_DIR)/xcaddy
1616

17-
# Install oapi-codegen
17+
# Install oapi-codegen (pinned to match committed generated code)
1818
$(OAPI_CODEGEN): | $(BIN_DIR)
1919
GOBIN=$(BIN_DIR) go install github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen@$(OAPI_CODEGEN_VERSION)
2020

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ require (
3030
github.com/miekg/dns v1.1.68
3131
github.com/nrednav/cuid2 v1.1.0
3232
github.com/oapi-codegen/nethttp-middleware v1.1.2
33-
github.com/oapi-codegen/runtime v1.1.2
33+
github.com/oapi-codegen/runtime v1.2.0
3434
github.com/opencontainers/go-digest v1.0.0
3535
github.com/opencontainers/image-spec v1.1.1
3636
github.com/opencontainers/runtime-spec v1.2.1

go.sum

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ github.com/oapi-codegen/nethttp-middleware v1.1.2 h1:TQwEU3WM6ifc7ObBEtiJgbRPaCe
191191
github.com/oapi-codegen/nethttp-middleware v1.1.2/go.mod h1:5qzjxMSiI8HjLljiOEjvs4RdrWyMPKnExeFS2kr8om4=
192192
github.com/oapi-codegen/runtime v1.1.2 h1:P2+CubHq8fO4Q6fV1tqDBZHCwpVpvPg7oKiYzQgXIyI=
193193
github.com/oapi-codegen/runtime v1.1.2/go.mod h1:SK9X900oXmPWilYR5/WKPzt3Kqxn/uS/+lbpREv+eCg=
194+
github.com/oapi-codegen/runtime v1.2.0 h1:RvKc1CVS1QeKSNzO97FBQbSMZyQ8s6rZd+LpmzwHMP4=
195+
github.com/oapi-codegen/runtime v1.2.0/go.mod h1:Y7ZhmmlE8ikZOmuHRRndiIm7nf3xcVv+YMweKgG1DT0=
194196
github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 h1:G7ERwszslrBzRxj//JalHPu/3yz+De2J+4aLtSRlHiY=
195197
github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037/go.mod h1:2bpvgLBZEtENV5scfDFEtB/5+1M4hkQhDQrccEJ/qGw=
196198
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
250252
github.com/u-root/uio v0.0.0-20240224005618-d2acac8f3701/go.mod h1:P3a5rG4X7tI17Nn3aOIAYr5HbIMukwXG0urG0WuL8OA=
251253
github.com/ugorji/go/codec v1.2.11 h1:BMaWp1Bb6fHwEtbplGBGJ498wD+LKlNSl25MjdZY4dU=
252254
github.com/ugorji/go/codec v1.2.11/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg=
255+
github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE=
253256
github.com/vbatts/go-mtree v0.6.1-0.20250911112631-8307d76bc1b9 h1:R6l9BtUe83abUGu1YKGkfa17wMMFLt6mhHVQ8MxpfRE=
254257
github.com/vbatts/go-mtree v0.6.1-0.20250911112631-8307d76bc1b9/go.mod h1:W7bcG9PCn6lFY+ljGlZxx9DONkxL3v8a7HyN+PrSrjA=
255258
github.com/vbatts/tar-split v0.12.1 h1:CqKoORW7BUWBe7UL/iqTVvkTBOF8UvOMKOIZykxnnbo=

lib/images/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ Content-addressable storage with tag symlinks (similar to Docker/Unikraft):
9090
- Pulling same tag twice updates the symlink if digest changed
9191
- OCI cache uses digest hex as layout tag for true content-addressable caching
9292
- Shared blob storage enables automatic layer deduplication across all images
93-
- Old digests remain until explicitly garbage collected
93+
- Orphaned digests are automatically deleted when the last tag referencing them is removed
9494
- Symlinks only created after successful build (status: ready)
9595

9696
## Reference Handling (reference.go)

lib/images/manager.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,39 @@ func (m *manager) DeleteImage(ctx context.Context, name string) error {
421421
repository := ref.Repository()
422422
tag := ref.Tag()
423423

424-
return deleteTag(m.paths, repository, tag)
424+
// Resolve the tag to get the digest before deleting
425+
digestHex, err := resolveTag(m.paths, repository, tag)
426+
if err != nil {
427+
return err
428+
}
429+
430+
// Delete the tag symlink
431+
if err := deleteTag(m.paths, repository, tag); err != nil {
432+
return err
433+
}
434+
435+
// Hold createMu during orphan check and delete to prevent race with CreateImage.
436+
// Without this lock, a concurrent CreateImage could create a new tag pointing to
437+
// the same digest between our count check and delete, leaving a dangling symlink.
438+
m.createMu.Lock()
439+
defer m.createMu.Unlock()
440+
441+
// Check if the digest is now orphaned (no other tags reference it)
442+
count, err := countTagsForDigest(m.paths, repository, digestHex)
443+
if err != nil {
444+
fmt.Fprintf(os.Stderr, "Warning: failed to count tags for digest %s: %v\n", digestHex, err)
445+
return nil
446+
}
447+
448+
if count == 0 {
449+
// Digest is orphaned, delete it
450+
if err := deleteDigest(m.paths, repository, digestHex); err != nil {
451+
fmt.Fprintf(os.Stderr, "Warning: failed to delete orphaned digest %s: %v\n", digestHex, err)
452+
return nil
453+
}
454+
}
455+
456+
return nil
425457
}
426458

427459
// TotalImageBytes returns the total size of all ready images on disk.

lib/images/manager_test.go

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/google/go-containerregistry/pkg/v1/empty"
1212
"github.com/google/go-containerregistry/pkg/v1/layout"
1313
"github.com/kernel/hypeman/lib/paths"
14+
"github.com/stretchr/testify/assert"
1415
"github.com/stretchr/testify/require"
1516
)
1617

@@ -228,10 +229,10 @@ func TestDeleteImage(t *testing.T) {
228229
_, err = mgr.GetImage(ctx, created.Name)
229230
require.ErrorIs(t, err, ErrNotFound)
230231

231-
// But digest directory should still exist
232+
// Digest directory should also be deleted (no orphaned digests)
232233
digestDir := digestPath(paths.New(dataDir), ref.Repository(), digestHex)
233234
_, err = os.Stat(digestDir)
234-
require.NoError(t, err)
235+
require.True(t, os.IsNotExist(err), "digest directory should be deleted when orphaned")
235236
}
236237

237238
func TestDeleteImageNotFound(t *testing.T) {
@@ -245,6 +246,60 @@ func TestDeleteImageNotFound(t *testing.T) {
245246
require.ErrorIs(t, err, ErrNotFound)
246247
}
247248

249+
func TestDeleteImagePreservesSharedDigest(t *testing.T) {
250+
dataDir := t.TempDir()
251+
p := paths.New(dataDir)
252+
mgr, err := NewManager(p, 1, nil)
253+
require.NoError(t, err)
254+
255+
ctx := context.Background()
256+
req := CreateImageRequest{
257+
Name: "docker.io/library/alpine:latest",
258+
}
259+
260+
created, err := mgr.CreateImage(ctx, req)
261+
require.NoError(t, err)
262+
263+
waitForReady(t, mgr, ctx, created.Name)
264+
265+
// Get the digest
266+
img, err := mgr.GetImage(ctx, created.Name)
267+
require.NoError(t, err)
268+
ref, err := ParseNormalizedRef(img.Name)
269+
require.NoError(t, err)
270+
digestHex := strings.SplitN(img.Digest, ":", 2)[1]
271+
272+
// Create a second tag pointing to the same digest
273+
err = createTagSymlink(p, ref.Repository(), "v1.0", digestHex)
274+
require.NoError(t, err)
275+
276+
// Delete the first tag
277+
err = mgr.DeleteImage(ctx, created.Name)
278+
require.NoError(t, err)
279+
280+
// First tag should be gone
281+
_, err = mgr.GetImage(ctx, created.Name)
282+
require.ErrorIs(t, err, ErrNotFound)
283+
284+
// Digest directory should still exist (shared by v1.0 tag)
285+
digestDir := digestPath(p, ref.Repository(), digestHex)
286+
_, err = os.Stat(digestDir)
287+
require.NoError(t, err, "digest directory should be preserved when other tags reference it")
288+
289+
// Second tag should still work
290+
img2, err := mgr.GetImage(ctx, "docker.io/library/alpine:v1.0")
291+
require.NoError(t, err)
292+
assert.Equal(t, img.Digest, img2.Digest)
293+
294+
// Now delete the second tag
295+
err = mgr.DeleteImage(ctx, "docker.io/library/alpine:v1.0")
296+
require.NoError(t, err)
297+
298+
// Now the digest directory should be deleted
299+
_, err = os.Stat(digestDir)
300+
require.True(t, os.IsNotExist(err), "digest directory should be deleted when last tag is removed")
301+
}
302+
248303
func TestNormalizedRefParsing(t *testing.T) {
249304
tests := []struct {
250305
input string

lib/images/storage.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,3 +288,32 @@ func deleteTag(p *paths.Paths, repository, tag string) error {
288288

289289
return nil
290290
}
291+
292+
// countTagsForDigest counts how many tags in a repository point to a given digest
293+
func countTagsForDigest(p *paths.Paths, repository, digestHex string) (int, error) {
294+
tags, err := listTags(p, repository)
295+
if err != nil {
296+
return 0, err
297+
}
298+
299+
count := 0
300+
for _, tag := range tags {
301+
target, err := resolveTag(p, repository, tag)
302+
if err != nil {
303+
continue
304+
}
305+
if target == digestHex {
306+
count++
307+
}
308+
}
309+
return count, nil
310+
}
311+
312+
// deleteDigest removes a digest directory and all its contents
313+
func deleteDigest(p *paths.Paths, repository, digestHex string) error {
314+
dir := digestDir(p, repository, digestHex)
315+
if err := os.RemoveAll(dir); err != nil {
316+
return fmt.Errorf("remove digest directory: %w", err)
317+
}
318+
return nil
319+
}

0 commit comments

Comments
 (0)