Skip to content

Commit f0404e7

Browse files
stephanmestephanme
authored
fix: gc for untagged docker manifests (#3349)
- fixes #3347: removeUntaggedManifests() did not consider compatible manifest types - add AsDockerImage() to Image and MultiarchImage for testing - extend TestGarbageCollectAndRetentionMetaDB to test docker image and multiarch image Signed-off-by: Stephan Merker <stephan.merker@sap.com>
1 parent cb520aa commit f0404e7

6 files changed

Lines changed: 182 additions & 9 deletions

File tree

‎pkg/storage/gc/gc.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,8 @@ func (gc GarbageCollect) removeUntaggedManifests(repo string, index *ispec.Index
485485
}
486486

487487
// remove untagged images
488-
if desc.MediaType == ispec.MediaTypeImageManifest || desc.MediaType == ispec.MediaTypeImageIndex {
488+
if desc.MediaType == ispec.MediaTypeImageManifest || compat.IsCompatibleManifestMediaType(desc.MediaType) ||
489+
desc.MediaType == ispec.MediaTypeImageIndex || compat.IsCompatibleManifestListMediaType(desc.MediaType) {
489490
_, ok := getDescriptorTag(desc)
490491
if !ok {
491492
gced, err = gc.gcManifest(repo, index, desc, "", "", gc.opts.ImageRetention.Delay)

‎pkg/storage/gc/gc_test.go‎

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"gopkg.in/resty.v1"
2121

2222
"zotregistry.dev/zot/pkg/api/config"
23+
"zotregistry.dev/zot/pkg/compat"
2324
"zotregistry.dev/zot/pkg/extensions/monitoring"
2425
zlog "zotregistry.dev/zot/pkg/log"
2526
"zotregistry.dev/zot/pkg/meta"
@@ -71,6 +72,7 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
7172
var imgStore storageTypes.ImageStore
7273

7374
var metaDB mTypes.MetaDB
75+
compat := []compat.MediaCompatibility{compat.DockerManifestV2SchemaV2}
7476

7577
if testcase.storageType == storageConstants.S3StorageDriverName {
7678
tskip.SkipDynamo(t)
@@ -140,13 +142,13 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
140142
panic(err)
141143
}
142144

143-
imgStore = s3.NewImageStore(rootDir, cacheDir, true, false, log, metrics, nil, store, nil, nil, nil)
145+
imgStore = s3.NewImageStore(rootDir, cacheDir, true, false, log, metrics, nil, store, nil, compat, nil)
144146
} else {
145147
// Create temporary directory
146148
rootDir := t.TempDir()
147149

148150
// Create ImageStore
149-
imgStore = local.NewImageStore(rootDir, false, false, log, metrics, nil, nil, nil, nil)
151+
imgStore = local.NewImageStore(rootDir, false, false, log, metrics, nil, nil, compat, nil)
150152

151153
// init metaDB
152154
params := boltdb.DBParameters{
@@ -243,6 +245,15 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
243245
err = WriteMultiArchImageToFileSystem(gcUntagged4, "gc-test4", gcUntagged4.DigestStr(), storeController)
244246
So(err, ShouldBeNil)
245247

248+
// docker images
249+
gcDocker1 := CreateRandomImage().AsDockerImage()
250+
err = WriteImageToFileSystem(gcDocker1, "gc-docker1", "0.0.1", storeController)
251+
So(err, ShouldBeNil)
252+
253+
gcDocker2 := CreateRandomMultiarch().AsDockerImage()
254+
err = WriteMultiArchImageToFileSystem(gcDocker2, "gc-docker2", "0.0.1", storeController)
255+
So(err, ShouldBeNil)
256+
246257
// for image retention testing
247258
// old images
248259
gcOld1 := CreateRandomImage()
@@ -361,6 +372,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
361372
err = gc.CleanRepo(ctx, "gc-test4")
362373
So(err, ShouldBeNil)
363374

375+
err = gc.CleanRepo(ctx, "gc-docker1")
376+
So(err, ShouldBeNil)
377+
378+
err = gc.CleanRepo(ctx, "gc-docker2")
379+
So(err, ShouldBeNil)
380+
364381
err = gc.CleanRepo(ctx, "retention")
365382
So(err, ShouldBeNil)
366383

@@ -412,6 +429,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
412429
_, _, _, err = imgStore.GetImageManifest("gc-test4", refOfRef4.DigestStr())
413430
So(err, ShouldBeNil)
414431

432+
_, _, _, err = imgStore.GetImageManifest("gc-docker1", gcDocker1.DigestStr())
433+
So(err, ShouldBeNil)
434+
435+
_, _, _, err = imgStore.GetImageManifest("gc-docker2", gcDocker2.DigestStr())
436+
So(err, ShouldBeNil)
437+
415438
_, _, _, err = imgStore.GetImageManifest("retention", "0.0.1")
416439
So(err, ShouldBeNil)
417440

@@ -465,6 +488,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
465488
err = gc.CleanRepo(ctx, "gc-test4")
466489
So(err, ShouldBeNil)
467490

491+
err = gc.CleanRepo(ctx, "gc-docker1")
492+
So(err, ShouldBeNil)
493+
494+
err = gc.CleanRepo(ctx, "gc-docker2")
495+
So(err, ShouldBeNil)
496+
468497
err = gc.CleanRepo(ctx, "retention")
469498
So(err, ShouldBeNil)
470499

@@ -515,6 +544,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
515544

516545
_, _, _, err = imgStore.GetImageManifest("gc-test4", refOfRef4.DigestStr())
517546
So(err, ShouldBeNil)
547+
548+
_, _, _, err = imgStore.GetImageManifest("gc-docker1", gcDocker1.DigestStr())
549+
So(err, ShouldBeNil)
550+
551+
_, _, _, err = imgStore.GetImageManifest("gc-docker2", gcDocker2.DigestStr())
552+
So(err, ShouldBeNil)
518553
})
519554

520555
Convey("gc all tags, untagged, and afterwards referrers", func() {
@@ -560,6 +595,50 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
560595
So(repos, ShouldContain, "gc-test2")
561596
So(repos, ShouldContain, "gc-test3")
562597
So(repos, ShouldContain, "gc-test4")
598+
So(repos, ShouldContain, "gc-docker1")
599+
So(repos, ShouldContain, "gc-docker2")
600+
So(repos, ShouldContain, "retention")
601+
})
602+
603+
Convey("gc all tags for docker repo", func() {
604+
gc := gc.NewGarbageCollect(imgStore, metaDB, gc.Options{
605+
Delay: 1 * time.Millisecond,
606+
ImageRetention: config.ImageRetention{
607+
Delay: 1 * time.Millisecond,
608+
Policies: []config.RetentionPolicy{
609+
{
610+
Repositories: []string{"gc-docker*"},
611+
DeleteReferrers: true,
612+
DeleteUntagged: &trueVal,
613+
KeepTags: []config.KeepTagsPolicy{
614+
{
615+
Patterns: []string{"v1"}, // should not match any tag
616+
},
617+
},
618+
},
619+
},
620+
},
621+
}, audit, log)
622+
623+
err := gc.CleanRepo(ctx, "gc-docker1")
624+
So(err, ShouldBeNil)
625+
err = gc.CleanRepo(ctx, "gc-docker2")
626+
So(err, ShouldBeNil)
627+
628+
_, _, _, err = imgStore.GetImageManifest("gc-docker1", gcDocker1.DigestStr())
629+
So(err, ShouldNotBeNil)
630+
_, _, _, err = imgStore.GetImageManifest("gc-docker2", gcDocker2.DigestStr())
631+
So(err, ShouldNotBeNil)
632+
633+
// now repo should get gc'ed
634+
repos, err := imgStore.GetRepositories()
635+
So(err, ShouldBeNil)
636+
So(repos, ShouldContain, "gc-test1")
637+
So(repos, ShouldContain, "gc-test2")
638+
So(repos, ShouldContain, "gc-test3")
639+
So(repos, ShouldContain, "gc-test4")
640+
So(repos, ShouldNotContain, "gc-docker1")
641+
So(repos, ShouldNotContain, "gc-docker2")
563642
So(repos, ShouldContain, "retention")
564643
})
565644

@@ -584,17 +663,17 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
584663
}, audit, log)
585664

586665
processedRepos := make(map[string]struct{})
587-
expectedRepos := []string{"gc-test1", "gc-test2", "gc-test3", "gc-test4", "retention"}
666+
expectedRepos := []string{"gc-docker1", "gc-docker2", "gc-test1", "gc-test2", "gc-test3", "gc-test4", "retention"}
588667

589-
for i := range 10 {
668+
for i := range 2 * len(expectedRepos) {
590669
t.Logf("index %d, processed repos %v", i, processedRepos)
591670

592671
// we need to check if GetNextRepository returns each repository just once, and empty string afterwards
593672
repo, err := imgStore.GetNextRepository(processedRepos)
594673
t.Logf("index %d, repo '%s'", i, repo)
595674
So(err, ShouldBeNil)
596675

597-
if i >= 5 {
676+
if i >= len(expectedRepos) {
598677
So(repo, ShouldEqual, "")
599678

600679
continue
@@ -627,6 +706,8 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
627706
So(repos, ShouldContain, "gc-test2")
628707
So(repos, ShouldNotContain, "gc-test3")
629708
So(repos, ShouldContain, "gc-test4")
709+
So(repos, ShouldContain, "gc-docker1")
710+
So(repos, ShouldContain, "gc-docker2")
630711
So(repos, ShouldContain, "retention")
631712
})
632713

@@ -670,6 +751,8 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
670751
So(repos, ShouldContain, "gc-test2")
671752
So(repos, ShouldContain, "gc-test3")
672753
So(repos, ShouldContain, "gc-test4")
754+
So(repos, ShouldContain, "gc-docker1")
755+
So(repos, ShouldContain, "gc-docker2")
673756
So(repos, ShouldContain, "retention")
674757

675758
tags, err := imgStore.GetImageTags("gc-test1")
@@ -713,6 +796,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
713796
_, _, _, err = imgStore.GetImageManifest("gc-test3", "0.0.1")
714797
So(err, ShouldBeNil)
715798

799+
_, _, _, err = imgStore.GetImageManifest("gc-docker1", "0.0.1")
800+
So(err, ShouldBeNil)
801+
802+
_, _, _, err = imgStore.GetImageManifest("gc-docker2", "0.0.1")
803+
So(err, ShouldBeNil)
804+
716805
_, _, _, err = imgStore.GetImageManifest("retention", "0.0.1")
717806
So(err, ShouldBeNil)
718807

@@ -768,6 +857,12 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
768857
_, _, _, err = imgStore.GetImageManifest("gc-test3", "0.0.1")
769858
So(err, ShouldBeNil)
770859

860+
_, _, _, err = imgStore.GetImageManifest("gc-docker1", "0.0.1")
861+
So(err, ShouldBeNil)
862+
863+
_, _, _, err = imgStore.GetImageManifest("gc-docker2", "0.0.1")
864+
So(err, ShouldBeNil)
865+
771866
_, _, _, err = imgStore.GetImageManifest("retention", "0.0.1")
772867
So(err, ShouldBeNil)
773868

@@ -1026,6 +1121,8 @@ func TestGarbageCollectAndRetentionMetaDB(t *testing.T) {
10261121
So(repos, ShouldContain, "gc-test1")
10271122
So(repos, ShouldContain, "gc-test2")
10281123
So(repos, ShouldContain, "gc-test3")
1124+
So(repos, ShouldContain, "gc-docker1")
1125+
So(repos, ShouldContain, "gc-docker2")
10291126
So(repos, ShouldContain, "retention")
10301127
})
10311128

‎pkg/test/image-utils/images.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strconv"
88
"time"
99

10+
docker "github.com/distribution/distribution/v3/manifest/schema2"
1011
godigest "github.com/opencontainers/go-digest"
1112
"github.com/opencontainers/image-spec/specs-go"
1213
ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -173,6 +174,35 @@ func (img Image) AsImageMeta() mTypes.ImageMeta {
173174
}
174175
}
175176

177+
func (img Image) AsDockerImage() Image {
178+
// Convert manifest media type
179+
img.Manifest.MediaType = docker.MediaTypeManifest
180+
181+
// Convert config media type
182+
img.Manifest.Config.MediaType = docker.MediaTypeImageConfig
183+
img.ConfigDescriptor.MediaType = docker.MediaTypeImageConfig
184+
185+
// Convert layer media types
186+
for i := range img.Manifest.Layers {
187+
img.Manifest.Layers[i].MediaType = docker.MediaTypeLayer
188+
}
189+
190+
manifestBlob, err := json.Marshal(img.Manifest)
191+
if err != nil {
192+
panic("unreachable: ispec.Manifest should always be marshable")
193+
}
194+
195+
img.ManifestDescriptor = ispec.Descriptor{
196+
MediaType: docker.MediaTypeImageConfig,
197+
Digest: img.digestAlgorithm.FromBytes(manifestBlob),
198+
Size: int64(len(manifestBlob)),
199+
Data: manifestBlob,
200+
Platform: img.ConfigDescriptor.Platform,
201+
}
202+
203+
return img
204+
}
205+
176206
type Layer struct {
177207
Blob []byte
178208
MediaType string

‎pkg/test/image-utils/images_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"testing"
66

7+
docker "github.com/distribution/distribution/v3/manifest/schema2"
78
godigest "github.com/opencontainers/go-digest"
89
ispec "github.com/opencontainers/image-spec/specs-go/v1"
910
. "github.com/smartystreets/goconvey/convey"
@@ -197,3 +198,19 @@ func TestImageMethods(t *testing.T) {
197198
})
198199
})
199200
}
201+
202+
func TestConvertImageToDocker(t *testing.T) {
203+
Convey("AsDockerImage", t, func() {
204+
Convey("DefaultImage", func() {
205+
img := CreateDefaultImage()
206+
dockerImage := img.AsDockerImage()
207+
208+
So(dockerImage.Manifest.MediaType, ShouldEqual, docker.MediaTypeManifest)
209+
So(dockerImage.ConfigDescriptor.MediaType, ShouldEqual, docker.MediaTypeImageConfig)
210+
211+
for _, layer := range dockerImage.Manifest.Layers {
212+
So(layer.MediaType, ShouldEqual, docker.MediaTypeLayer)
213+
}
214+
})
215+
})
216+
}

‎pkg/test/image-utils/multiarch.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package image
33
import (
44
"encoding/json"
55

6+
dockerList "github.com/distribution/distribution/v3/manifest/manifestlist"
67
godigest "github.com/opencontainers/go-digest"
78
"github.com/opencontainers/image-spec/specs-go"
89
ispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -75,6 +76,34 @@ func (mi MultiarchImage) AsImageMeta() mTypes.ImageMeta {
7576
}
7677
}
7778

79+
func (mi MultiarchImage) AsDockerImage() MultiarchImage {
80+
for i := range mi.Images {
81+
mi.Images[i] = mi.Images[i].AsDockerImage()
82+
}
83+
84+
mi.Index.MediaType = dockerList.MediaTypeManifestList
85+
for i := range mi.Index.Manifests {
86+
mi.Index.Manifests[i].Digest = mi.Images[i].ManifestDescriptor.Digest
87+
mi.Index.Manifests[i].Size = mi.Images[i].ManifestDescriptor.Size
88+
mi.Index.Manifests[i].MediaType = mi.Images[i].ManifestDescriptor.MediaType
89+
}
90+
91+
indexBlob, err := json.Marshal(mi.Index)
92+
if err != nil {
93+
panic("unreachable: ispec.Index should always be marshable")
94+
}
95+
96+
mi.IndexDescriptor.MediaType = dockerList.MediaTypeManifestList
97+
mi.IndexDescriptor = ispec.Descriptor{
98+
MediaType: dockerList.MediaTypeManifestList,
99+
Digest: mi.digestAlgorithm.FromBytes(indexBlob),
100+
Size: int64(len(indexBlob)),
101+
Data: indexBlob,
102+
}
103+
104+
return mi
105+
}
106+
78107
type ImagesBuilder interface {
79108
Images(images []Image) MultiarchBuilder
80109
RandomImages(count int) MultiarchBuilder

‎pkg/test/image-utils/write.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"encoding/json"
66

77
godigest "github.com/opencontainers/go-digest"
8-
ispec "github.com/opencontainers/image-spec/specs-go/v1"
98

109
stypes "zotregistry.dev/zot/pkg/storage/types"
1110
)
@@ -52,7 +51,7 @@ func WriteImageToFileSystem(image Image, repoName, ref string, storeController s
5251
return err
5352
}
5453

55-
_, _, err = store.PutImageManifest(repoName, ref, ispec.MediaTypeImageManifest, manifestBlob)
54+
_, _, err = store.PutImageManifest(repoName, ref, image.Manifest.MediaType, manifestBlob)
5655
if err != nil {
5756
return err
5857
}
@@ -82,7 +81,7 @@ func WriteMultiArchImageToFileSystem(multiarchImage MultiarchImage, repoName, re
8281
return err
8382
}
8483

85-
_, _, err = store.PutImageManifest(repoName, ref, ispec.MediaTypeImageIndex,
84+
_, _, err = store.PutImageManifest(repoName, ref, multiarchImage.Index.MediaType,
8685
indexBlob)
8786

8887
return err

0 commit comments

Comments
 (0)