Skip to content

Commit

Permalink
Merge pull request #108 from sudo-bmitch/pr-gc-testing
Browse files Browse the repository at this point in the history 
Fix: Logging of delete by tag and copying descriptors
  • Loading branch information
@sudo-bmitch
sudo-bmitch committed May 28, 2024
2 parents a5189f6 + 5540dfc commit 0f285cf
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 19 deletions.
4 changes: 2 additions & 2 deletions manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (s *Server) manifestDelete(repoStr, arg string) http.HandlerFunc {
_ = types.ErrRespJSON(w, types.ErrInfoManifestUnknown("tag or digest was not found in repository"))
return
}
// if referrers is enabled, get the manifest to check for a subject
// if referrers is enabled, remove entry from the referrers list
if *s.conf.API.Referrer.Enabled {
// wrap in a func to allow a return from errors without breaking the actual delete
err = func() error {
Expand All @@ -81,7 +81,7 @@ func (s *Server) manifestDelete(repoStr, arg string) http.HandlerFunc {
return nil
}()
if err != nil {
s.log.Info("failed to delete referrer", "repo", repoStr, "arg", arg, "err", err)
s.log.Info("failed to delete entry from referrers response", "repo", repoStr, "arg", arg, "err", err)
}
}
// delete the digest or tag
Expand Down
89 changes: 77 additions & 12 deletions olareg_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func TestServer(t *testing.T) {
t.Errorf("failed to generate sample data: %v", err)
return
}
grace := time.Millisecond * 250
freq := time.Millisecond * 100
sleep := (freq * 2) + grace + (time.Millisecond * 100)
existingRepo := "testrepo"
existingTag := "v2"
existingReferrerCountAll := 2
Expand All @@ -54,6 +57,7 @@ func TestServer(t *testing.T) {
return
}
boolT := true
boolF := false
ttServer := []struct {
name string
conf config.Config
Expand All @@ -66,6 +70,14 @@ func TestServer(t *testing.T) {
conf: config.Config{
Storage: config.ConfigStorage{
StoreType: config.StoreMem,
GC: config.ConfigGC{
Frequency: freq,
GracePeriod: grace,
RepoUploadMax: 10,
Untagged: &boolT,
ReferrersDangling: &boolF,
ReferrersWithSubj: &boolT,
},
},
API: config.ConfigAPI{
DeleteEnabled: &boolT,
Expand All @@ -74,13 +86,22 @@ func TestServer(t *testing.T) {
},
},
},
testGC: true,
},
{
name: "Mem with Dir",
conf: config.Config{
Storage: config.ConfigStorage{
StoreType: config.StoreMem,
RootDir: "./testdata",
GC: config.ConfigGC{
Frequency: freq,
GracePeriod: grace,
RepoUploadMax: 10,
Untagged: &boolT,
ReferrersDangling: &boolF,
ReferrersWithSubj: &boolT,
},
},
API: config.ConfigAPI{
DeleteEnabled: &boolT,
Expand All @@ -90,6 +111,7 @@ func TestServer(t *testing.T) {
},
},
existing: true,
testGC: true,
},
{
name: "Dir",
Expand All @@ -98,8 +120,12 @@ func TestServer(t *testing.T) {
StoreType: config.StoreDir,
RootDir: tempDir,
GC: config.ConfigGC{
Frequency: time.Second * -1,
GracePeriod: time.Second * -1,
Frequency: freq,
GracePeriod: grace,
RepoUploadMax: 10,
Untagged: &boolT,
ReferrersDangling: &boolF,
ReferrersWithSubj: &boolT,
},
},
API: config.ConfigAPI{
Expand Down Expand Up @@ -395,36 +421,75 @@ func TestServer(t *testing.T) {
if !tcServer.testGC || tcServer.readOnly {
return
}
// this test is not parallel because the server is closed and restarted
// push two images
t.Parallel()
// push two images with three tags
if err := testSampleEntryPush(t, s, *sd["image-amd64"], "gc", "amd64"); err != nil {
return
}
if err := testSampleEntryPush(t, s, *sd["image-amd64"], "gc", "amd64-copy"); err != nil {
return
}
if err := testSampleEntryPush(t, s, *sd["image-arm64"], "gc", "arm64"); err != nil {
return
}
// delete one manifest
// delete one manifest by digest
if _, err := testAPIManifestRm(t, s, "gc", sd["image-arm64"].manifestList[0].String()); err != nil {
t.Fatalf("failed to remove manifest: %v", err)
}
// close server and recreate
if err := s.Close(); err != nil {
t.Errorf("failed to close server: %v", err)
// delete the other manifest by tag
if _, err := testAPIManifestRm(t, s, "gc", "amd64-copy"); err != nil {
t.Fatalf("failed to remove manifest: %v", err)
}
// delay for GC
for retry := 0; retry < 10; retry++ {
time.Sleep(sleep)
if _, err := testClientRun(t, s, "GET", "/v2/gc/blobs/"+string(sd["image-arm64"].manifest[sd["image-arm64"].manifestList[0]]), nil,
testClientRespStatus(http.StatusNotFound)); err != nil {
break
}
}
s = New(tcServer.conf)
// verify get
if err := testSampleEntryPull(t, s, *sd["image-amd64"], "gc", "amd64"); err != nil {
t.Errorf("failed to pull entry after recreating server: %v", err)
}
// verify GC
if _, err := testClientRun(t, s, "GET", "/v2/gc/manifest/amd64-copy", nil,
testClientReqHeader("Accept", types.MediaTypeOCI1Manifest),
testClientReqHeader("Accept", types.MediaTypeOCI1ManifestList),
testClientReqHeader("Accept", types.MediaTypeDocker2Manifest),
testClientReqHeader("Accept", types.MediaTypeDocker2ManifestList),
testClientRespStatus(http.StatusNotFound)); err != nil {
t.Errorf("pulled image after it should have been deleted and GC'd: %v", err)
} else {
t.Log("amd64-copy was garbage collected")
}
if _, err := testClientRun(t, s, "GET", "/v2/gc/manifest/arm64", nil,
testClientReqHeader("Accept", types.MediaTypeOCI1Manifest),
testClientReqHeader("Accept", types.MediaTypeOCI1ManifestList),
testClientReqHeader("Accept", types.MediaTypeDocker2Manifest),
testClientReqHeader("Accept", types.MediaTypeDocker2ManifestList),
testClientRespStatus(http.StatusNotFound)); err != nil {
t.Errorf("pulled image after it should have been deleted and GC'd: %v", err)
} else {
t.Log("arm64 was garbage collected")
}
for dig := range sd["image-arm64"].blob {
if _, ok := sd["image-amd64"].blob[dig]; ok {
continue // skip dup blobs
}
_, err := testClientRun(t, s, "GET", "/v2/gc/blobs/"+dig.String(), nil,
testClientRespStatus(http.StatusNotFound))
if err != nil {
if _, err := testClientRun(t, s, "GET", "/v2/gc/blobs/"+dig.String(), nil,
testClientRespStatus(http.StatusNotFound)); err != nil {
t.Errorf("did not receive a not-found error on a GC blob: %v", err)
} else {
t.Logf("blog GC verified: %s", dig.String())
}
}
for dig := range sd["image-amd64"].blob {
if _, err := testClientRun(t, s, "HEAD", "/v2/gc/blobs/"+dig.String(), nil,
testClientRespStatus(http.StatusOK)); err != nil {
t.Errorf("GC blob of image that should have been preserved: %v", err)
} else {
t.Logf("blog retention verified: %s", dig.String())
}
}
})
Expand Down
6 changes: 6 additions & 0 deletions types/descriptor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,11 @@ func (d Descriptor) Copy() Descriptor {
p := d.Platform.Copy()
d2.Platform = &p
}
if d.Annotations != nil {
d2.Annotations = make(map[string]string)
for k, v := range d.Annotations {
d2.Annotations[k] = v
}
}
return d2
}
10 changes: 5 additions & 5 deletions types/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,22 @@ func (i Index) Copy() Index {

// GetDesc returns a descriptor for a tag or digest, including child descriptors.
func (i Index) GetDesc(arg string) (Descriptor, error) {
var dRet Descriptor
var dZero Descriptor
if len(i.Manifests) == 0 {
return dRet, ErrNotFound
return dZero, ErrNotFound
}
if RefTagRE.MatchString(arg) {
// search for tag
for _, d := range i.Manifests {
if d.Annotations != nil && d.Annotations[AnnotRefName] == arg {
return d, nil
return d.Copy(), nil
}
}
} else {
// else, attempt to parse digest
dig, err := digest.Parse(arg)
if err != nil {
return dRet, err
return dZero, err
}
// return a matching descriptor, but stripped of any annotations to avoid mixing with tags
for _, d := range i.Manifests {
Expand All @@ -142,7 +142,7 @@ func (i Index) GetDesc(arg string) (Descriptor, error) {
}
}
}
return dRet, ErrNotFound
return dZero, ErrNotFound
}

// GetByAnnotation finds an entry with a matching annotation.
Expand Down

0 comments on commit 0f285cf

Please sign in to comment.