From b1f882e1b8f3ed938ced922eef4cf3b27eada04f Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani Date: Mon, 6 Apr 2020 18:17:24 -0700 Subject: [PATCH] conformance: align with upstream conformance tests Upstream conformance tests are being updated, so we need to align along with our internal GC and dedupe features. Add a new example config file which plays nice with conformance tests. DeleteImageManifest() updated to deal with the case where the same manifest can be created with multiple tags and deleted with the same digest - so all entries must be deleted. DeleteBlob() delete the digest key (bucket) when last reference is dropped --- examples/config-conformance.json | 15 ++++++++++ pkg/api/routes.go | 7 ++++- pkg/compliance/v1_0_0/check.go | 6 +++- pkg/storage/cache.go | 11 +++++++ pkg/storage/storage.go | 51 ++++++++++++++++++-------------- pkg/storage/storage_test.go | 10 +++++++ 6 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 examples/config-conformance.json diff --git a/examples/config-conformance.json b/examples/config-conformance.json new file mode 100644 index 00000000..c44c4ff9 --- /dev/null +++ b/examples/config-conformance.json @@ -0,0 +1,15 @@ +{ + "version":"0.1.0-dev", + "storage":{ + "rootDirectory":"/tmp/zot", + "gc": false, + "dedupe": false + }, + "http": { + "address":"127.0.0.1", + "port":"8080" + }, + "log":{ + "level":"debug" + } +} diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 3f79607c..6ec6574c 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -222,7 +222,12 @@ func (rh *RouteHandler) ListTags(w http.ResponseWriter, r *http.Request) { pTags.Tags = tags[i+1 : i+1+n] } - last = pTags.Tags[len(pTags.Tags)-1] + if len(pTags.Tags) == 0 { + last = "" + } else { + last = pTags.Tags[len(pTags.Tags)-1] + } + w.Header().Set("Link", fmt.Sprintf("/v2/%s/tags/list?n=%d&last=%s; rel=\"next\"", name, n, last)) WriteJSON(w, http.StatusOK, pTags) diff --git a/pkg/compliance/v1_0_0/check.go b/pkg/compliance/v1_0_0/check.go index 3b1df400..47e125a7 100644 --- a/pkg/compliance/v1_0_0/check.go +++ b/pkg/compliance/v1_0_0/check.go @@ -565,7 +565,7 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { // delete manifest by digest resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) So(err, ShouldBeNil) - So(resp.StatusCode(), ShouldEqual, 202) + So(resp.StatusCode(), ShouldEqual, 404) // delete again should fail resp, err = resty.R().Delete(baseURL + "/v2/repo7/manifests/" + digest.String()) So(err, ShouldBeNil) @@ -656,6 +656,10 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) { So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 200) + resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=0") + So(err, ShouldBeNil) + So(resp.StatusCode(), ShouldEqual, 200) + resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=3") So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 200) diff --git a/pkg/storage/cache.go b/pkg/storage/cache.go index 5c7a0758..1c77f76b 100644 --- a/pkg/storage/cache.go +++ b/pkg/storage/cache.go @@ -154,6 +154,17 @@ func (c *Cache) DeleteBlob(digest string, path string) error { return err } + cur := b.Cursor() + k, _ := cur.First() + + if k == nil { + c.log.Debug().Str("digest", digest).Str("path", path).Msg("deleting empty bucket") + if err := root.DeleteBucket([]byte(digest)); err != nil { + c.log.Error().Err(err).Str("digest", digest).Str("path", path).Msg("unable to delete") + return err + } + } + return nil }); err != nil { return err diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 6f64da95..53733489 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -488,7 +488,7 @@ func (is *ImageStore) DeleteImageManifest(repo string, reference string) error { } // as per spec "reference" can only be a digest and not a tag - _, err := godigest.Parse(reference) + digest, err := godigest.Parse(reference) if err != nil { is.log.Error().Err(err).Msg("invalid reference") return errors.ErrBadManifest @@ -509,33 +509,29 @@ func (is *ImageStore) DeleteImageManifest(repo string, reference string) error { found := false - var digest godigest.Digest - - var i int - var m ispec.Descriptor - for i, m = range index.Manifests { - if reference == m.Digest.String() { - digest = m.Digest - found = true + // we are deleting, so keep only those manifests that don't match + outIndex := index + outIndex.Manifests = []ispec.Descriptor{} - break + for _, m = range index.Manifests { + if reference == m.Digest.String() { + found = true + continue } + + outIndex.Manifests = append(outIndex.Manifests, m) } if !found { return errors.ErrManifestNotFound } - // remove the manifest entry, not preserving order - index.Manifests[i] = index.Manifests[len(index.Manifests)-1] - index.Manifests = index.Manifests[:len(index.Manifests)-1] - // now update "index.json" dir = path.Join(is.rootDir, repo) file := path.Join(dir, "index.json") - buf, err = json.Marshal(index) + buf, err = json.Marshal(outIndex) if err != nil { return err @@ -809,44 +805,53 @@ func (is *ImageStore) FullBlobUpload(repo string, body io.Reader, digest string) // nolint (interfacer) func (is *ImageStore) DedupeBlob(src string, dstDigest godigest.Digest, dst string) error { +retry: + is.log.Debug().Str("src", src).Str("dstDigest", dstDigest.String()).Str("dst", dst).Msg("dedupe: ENTER") dstRecord, err := is.cache.GetBlob(dstDigest.String()) if err != nil && err != errors.ErrCacheMiss { - is.log.Error().Err(err).Str("blobPath", dst).Msg("unable to lookup blob record") + is.log.Error().Err(err).Str("blobPath", dst).Msg("dedupe: unable to lookup blob record") return err } if dstRecord == "" { if err := is.cache.PutBlob(dstDigest.String(), dst); err != nil { - is.log.Error().Err(err).Str("blobPath", dst).Msg("unable to insert blob record") + is.log.Error().Err(err).Str("blobPath", dst).Msg("dedupe: unable to insert blob record") return err } // move the blob from uploads to final dest if err := os.Rename(src, dst); err != nil { - is.log.Error().Err(err).Str("src", src).Str("dst", dst).Msg("unable to rename blob") + is.log.Error().Err(err).Str("src", src).Str("dst", dst).Msg("dedupe: unable to rename blob") return err } + is.log.Debug().Str("src", src).Str("dst", dst).Msg("dedupe: rename") } else { dstRecordFi, err := os.Stat(dstRecord) if err != nil { - is.log.Error().Err(err).Str("blobPath", dstRecord).Msg("unable to stat") - return err + is.log.Error().Err(err).Str("blobPath", dstRecord).Msg("dedupe: unable to stat") + // the actual blob on disk may have been removed by GC, so sync the cache + if err := is.cache.DeleteBlob(dstDigest.String(), dst); err != nil { + is.log.Error().Err(err).Str("dstDigest", dstDigest.String()).Str("dst", dst).Msg("dedupe: unable to delete blob record") + return err + } + goto retry } dstFi, err := os.Stat(dst) if err != nil && !os.IsNotExist(err) { - is.log.Error().Err(err).Str("blobPath", dstRecord).Msg("unable to stat") + is.log.Error().Err(err).Str("blobPath", dstRecord).Msg("dedupe: unable to stat") return err } if !os.SameFile(dstFi, dstRecordFi) { if err := os.Link(dstRecord, dst); err != nil { - is.log.Error().Err(err).Str("blobPath", dst).Str("link", dstRecord).Msg("unable to hard link") + is.log.Error().Err(err).Str("blobPath", dst).Str("link", dstRecord).Msg("dedupe: unable to hard link") return err } } if err := os.Remove(src); err != nil { - is.log.Error().Err(err).Str("src", src).Msg("uname to remove blob") + is.log.Error().Err(err).Str("src", src).Msg("dedupe: uname to remove blob") return err } + is.log.Debug().Str("src", src).Msg("dedupe: remove") } return nil diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 4fcb5453..6fd62dae 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -113,6 +113,12 @@ func TestAPIs(t *testing.T) { mb, _ := json.Marshal(m) Convey("Bad image manifest", func() { + _, err = il.PutImageManifest("test", d.String(), "application/json", mb) + So(err, ShouldNotBeNil) + + _, err = il.PutImageManifest("test", d.String(), ispec.MediaTypeImageManifest, []byte{}) + So(err, ShouldNotBeNil) + _, err = il.PutImageManifest("test", d.String(), ispec.MediaTypeImageManifest, mb) So(err, ShouldNotBeNil) @@ -133,6 +139,7 @@ func TestAPIs(t *testing.T) { Size: int64(l), }, }, + Annotations: map[string]string{ispec.AnnotationRefName: "1.0"}, } m.SchemaVersion = 2 mb, _ = json.Marshal(m) @@ -140,6 +147,9 @@ func TestAPIs(t *testing.T) { _, err = il.PutImageManifest("test", d.String(), ispec.MediaTypeImageManifest, mb) So(err, ShouldBeNil) + _, err = il.GetImageTags("test") + So(err, ShouldBeNil) + _, _, _, err = il.GetImageManifest("test", d.String()) So(err, ShouldBeNil)