From af30c06aff40dacc717f68677f156ad4a574fce2 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 29 Jun 2021 11:45:01 -0700 Subject: [PATCH] api: use blob cache path while making hard link previously mount blob will look for blob that is provided in http request and try to hard link that path but ideally we should look for path from our cache and do the hard link of that particular path. this commit does the same. --- pkg/api/controller_test.go | 51 +++++++++++++++++++++++++++++++++++-- pkg/api/routes.go | 10 ++++---- pkg/storage/storage.go | 40 +++++------------------------ pkg/storage/storage_test.go | 8 +++--- 4 files changed, 65 insertions(+), 44 deletions(-) diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 2e4139e6..cd9c132f 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -1801,6 +1801,7 @@ func TestCrossRepoMount(t *testing.T) { if err != nil { panic(err) } + defer os.RemoveAll(dir) c.Config.Storage.RootDirectory = dir @@ -1825,6 +1826,9 @@ func TestCrossRepoMount(t *testing.T) { params := make(map[string]string) digest := "sha256:63a795ca90aa6e7cca60941e826810a4cd0a2e73ea02bf458241df2a5c973e29" + + d := godigest.Digest(digest) + name := "zot-cve-test" params["mount"] = digest @@ -1836,6 +1840,7 @@ func TestCrossRepoMount(t *testing.T) { So(err, ShouldBeNil) So(headResponse.StatusCode(), ShouldEqual, 200) + // All invalid request of mount should return 202. params["mount"] = "sha:" postResponse, err := client.R(). @@ -1855,12 +1860,13 @@ func TestCrossRepoMount(t *testing.T) { So(postResponse.StatusCode(), ShouldEqual, 202) // Use correct request + // This is correct request but it will return 202 because blob is not present in cache. params["mount"] = digest postResponse, err = client.R(). SetBasicAuth(username, passphrase).SetQueryParams(params). Post(baseURL + "/v2/zot-c-test/blobs/uploads/") So(err, ShouldBeNil) - So(postResponse.StatusCode(), ShouldEqual, 201) + So(postResponse.StatusCode(), ShouldEqual, 202) // Send same request again postResponse, err = client.R(). @@ -1874,7 +1880,7 @@ func TestCrossRepoMount(t *testing.T) { SetBasicAuth(username, passphrase).SetQueryParams(params). Post(baseURL + "/v2/zot-d-test/blobs/uploads/") So(err, ShouldBeNil) - So(postResponse.StatusCode(), ShouldEqual, 201) + So(postResponse.StatusCode(), ShouldEqual, 202) headResponse, err = client.R().SetBasicAuth(username, passphrase). Head(fmt.Sprintf("%s/v2/zot-cv-test/blobs/%s", baseURL, digest)) @@ -1907,6 +1913,47 @@ func TestCrossRepoMount(t *testing.T) { So(err, ShouldBeNil) So(postResponse.StatusCode(), ShouldEqual, 201) + // We have uploaded a blob and since we have provided digest it should be full blob upload and there should be entry + // in cache, now try mount blob request status and it should be 201 because now blob is present in cache + // and it should do hard link. + + params["mount"] = digest + postResponse, err = client.R(). + SetBasicAuth(username, passphrase).SetQueryParams(params). + Post(baseURL + "/v2/zot-mount-test/blobs/uploads/") + So(err, ShouldBeNil) + So(postResponse.StatusCode(), ShouldEqual, 201) + + // Check os.SameFile here + cachePath := path.Join(c.Config.Storage.RootDirectory, "zot-d-test", "blobs/sha256", d.Hex()) + + cacheFi, err := os.Stat(cachePath) + So(err, ShouldBeNil) + + linkPath := path.Join(c.Config.Storage.RootDirectory, "zot-mount-test", "blobs/sha256", d.Hex()) + + linkFi, err := os.Stat(linkPath) + So(err, ShouldBeNil) + + So(os.SameFile(cacheFi, linkFi), ShouldEqual, true) + + // Now try another mount request and this time it should be from above uploaded repo i.e zot-mount-test + // mount request should pass and should return 201. + params["mount"] = digest + params["from"] = "zot-mount-test" + postResponse, err = client.R(). + SetBasicAuth(username, passphrase).SetQueryParams(params). + Post(baseURL + "/v2/zot-mount1-test/blobs/uploads/") + So(err, ShouldBeNil) + So(postResponse.StatusCode(), ShouldEqual, 201) + + linkPath = path.Join(c.Config.Storage.RootDirectory, "zot-mount1-test", "blobs/sha256", d.Hex()) + + linkFi, err = os.Stat(linkPath) + So(err, ShouldBeNil) + + So(os.SameFile(cacheFi, linkFi), ShouldEqual, true) + headResponse, err = client.R().SetBasicAuth(username, passphrase). Head(fmt.Sprintf("%s/v2/zot-cv-test/blobs/%s", baseURL, digest)) So(err, ShouldBeNil) diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 48b56731..f3803ea7 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -497,9 +497,7 @@ func (rh *RouteHandler) CheckBlob(w http.ResponseWriter, r *http.Request) { return } - mediaType := r.Header.Get("Accept") - - ok, blen, err := is.CheckBlob(name, digest, mediaType) + ok, blen, err := is.CheckBlob(name, digest) if err != nil { switch err { case errors.ErrBadBlobDigest: @@ -662,8 +660,10 @@ func (rh *RouteHandler) CreateBlobUpload(w http.ResponseWriter, r *http.Request) return } - // zot does not support cross mounting directly and do a workaround by copying blob using hard link - err := is.MountBlob(name, from[0], mountDigests[0]) + // zot does not support cross mounting directly and do a workaround creating using hard link. + // check blob looks for actual path (name+mountDigests[0]) first then look for cache and + // if found in cache, will do hard link and if fails we will start new upload. + _, _, err := is.CheckBlob(name, mountDigests[0]) if err != nil { u, err := is.NewBlobUpload(name) if err != nil { diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index e4c5ff2b..aa4f42c0 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -1024,40 +1024,8 @@ func (is *ImageStore) BlobPath(repo string, digest godigest.Digest) string { return path.Join(is.rootDir, repo, "blobs", digest.Algorithm().String(), digest.Encoded()) } -func (is *ImageStore) MountBlob(repo string, mountRepo string, digest string) error { - d, err := godigest.Parse(digest) - if err != nil { - is.log.Error().Err(err).Str("digest", digest).Msg("failed to parse digest") - - return errors.ErrBadBlobDigest - } - - mountBlobPath := is.BlobPath(mountRepo, d) - is.log.Debug().Str("mount path", mountBlobPath) - - blobPath := is.BlobPath(repo, d) - is.log.Debug().Str("repo path", blobPath) - - _, err = os.Stat(mountBlobPath) - if err != nil { - is.log.Error().Err(err).Msg("mount: blob path not found") - - return errors.ErrBlobNotFound - } - - _, err = is.copyBlob(repo, blobPath, mountBlobPath) - if err != nil { - is.log.Error().Err(err).Msg("cache: error copying blobs from cache location") - - return err - } - - return nil -} - // CheckBlob verifies a blob and returns true if the blob is correct. -func (is *ImageStore) CheckBlob(repo string, digest string, - mediaType string) (bool, int64, error) { +func (is *ImageStore) CheckBlob(repo string, digest string) (bool, int64, error) { d, err := godigest.Parse(digest) if err != nil { is.log.Error().Err(err).Str("digest", digest).Msg("failed to parse digest") @@ -1097,6 +1065,12 @@ func (is *ImageStore) CheckBlob(repo string, digest string, return false, -1, errors.ErrBlobNotFound } + if err := is.cache.PutBlob(digest, blobPath); err != nil { + is.log.Error().Err(err).Str("blobPath", blobPath).Msg("dedupe: unable to insert blob record") + + return false, -1, err + } + return true, blobSize, nil } diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 7afcf9b3..b48de1cd 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -102,7 +102,7 @@ func TestAPIs(t *testing.T) { So(err, ShouldBeNil) So(b, ShouldEqual, l) - _, _, err = il.CheckBlob("test", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") + _, _, err = il.CheckBlob("test", d.String()) So(err, ShouldBeNil) _, _, err = il.GetBlob("test", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") @@ -201,7 +201,7 @@ func TestAPIs(t *testing.T) { So(err, ShouldBeNil) So(b, ShouldEqual, l) - _, _, err = il.CheckBlob("test", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") + _, _, err = il.CheckBlob("test", d.String()) So(err, ShouldBeNil) _, _, err = il.GetBlob("test", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") @@ -363,7 +363,7 @@ func TestAPIs(t *testing.T) { So(err, ShouldBeNil) So(b, ShouldEqual, l) - _, _, err = il.CheckBlob("dedupe1", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") + _, _, err = il.CheckBlob("dedupe1", d.String()) So(err, ShouldBeNil) _, _, err = il.GetBlob("dedupe1", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") @@ -412,7 +412,7 @@ func TestAPIs(t *testing.T) { So(err, ShouldBeNil) So(b, ShouldEqual, l) - _, _, err = il.CheckBlob("dedupe2", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip") + _, _, err = il.CheckBlob("dedupe2", d.String()) So(err, ShouldBeNil) _, _, err = il.GetBlob("dedupe2", d.String(), "application/vnd.oci.image.layer.v1.tar+gzip")