Skip to content

Commit

Permalink
update the size field when existing manifest entry is updated
Browse files Browse the repository at this point in the history
An existing manifest descriptor in index.json can be updated with
different manifest contents for the same/existing tag. We were updating
the digest but not the size field causing GC to report an error.

Add a unit test case to cover this.

Add logs.
  • Loading branch information
rchincha authored and rchamarthy committed Jun 18, 2020
1 parent c374f9d commit 3dc9885
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pkg/storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ func (is *ImageStore) PutImageManifest(repo string, reference string, mediaType
for _, l := range m.Layers {
digest := l.Digest
blobPath := is.BlobPath(repo, digest)
is.log.Info().Str("blobPath", blobPath).Str("reference", reference).Msg("manifest layers")

if _, err := os.Stat(blobPath); err != nil {
is.log.Error().Err(err).Str("blobPath", blobPath).Msg("unable to find blob")
Expand Down Expand Up @@ -434,8 +435,17 @@ func (is *ImageStore) PutImageManifest(repo string, reference string, mediaType

break
}
// manifest contents have changed for the same tag
// manifest contents have changed for the same tag,
// so update index.json descriptor
is.log.Info().
Int64("old size", desc.Size).
Int64("new size", int64(len(body))).
Str("old digest", desc.Digest.String()).
Str("new digest", mDigest.String()).
Msg("updating existing tag with new manifest contents")

desc = m
desc.Size = int64(len(body))
desc.Digest = mDigest

index.Manifests = append(index.Manifests[:i], index.Manifests[i+1:]...)
Expand Down
83 changes: 83 additions & 0 deletions pkg/storage/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,89 @@ func TestAPIs(t *testing.T) {
So(err, ShouldNotBeNil)
})

Convey("Modify manifest in-place", func() {
// original blob
v, err := il.NewBlobUpload("replace")
So(err, ShouldBeNil)
So(v, ShouldNotBeEmpty)

content := []byte("test-data-replace-1")
buf := bytes.NewBuffer(content)
l := buf.Len()
d := godigest.FromBytes(content)
b, err := il.PutBlobChunkStreamed("replace", v, buf)
So(err, ShouldBeNil)
So(b, ShouldEqual, l)
blobDigest1 := strings.Split(d.String(), ":")[1]
So(blobDigest1, ShouldNotBeEmpty)

err = il.FinishBlobUpload("replace", v, buf, d.String())
So(err, ShouldBeNil)
So(b, ShouldEqual, l)

m := ispec.Manifest{}
m.SchemaVersion = 2
m = ispec.Manifest{
Config: ispec.Descriptor{
Digest: d,
Size: int64(l),
},
Layers: []ispec.Descriptor{
{
MediaType: "application/vnd.oci.image.layer.v1.tar",
Digest: d,
Size: int64(l),
},
},
}
m.SchemaVersion = 2
mb, _ := json.Marshal(m)
d = godigest.FromBytes(mb)
_, err = il.PutImageManifest("replace", "1.0", ispec.MediaTypeImageManifest, mb)
So(err, ShouldBeNil)

_, _, _, err = il.GetImageManifest("replace", d.String())
So(err, ShouldBeNil)

// new blob to replace
v, err = il.NewBlobUpload("replace")
So(err, ShouldBeNil)
So(v, ShouldNotBeEmpty)

content = []byte("test-data-replace-2")
buf = bytes.NewBuffer(content)
l = buf.Len()
d = godigest.FromBytes(content)
b, err = il.PutBlobChunkStreamed("replace", v, buf)
So(err, ShouldBeNil)
So(b, ShouldEqual, l)
blobDigest2 := strings.Split(d.String(), ":")[1]
So(blobDigest2, ShouldNotBeEmpty)

err = il.FinishBlobUpload("replace", v, buf, d.String())
So(err, ShouldBeNil)
So(b, ShouldEqual, l)

m = ispec.Manifest{
Config: ispec.Descriptor{
Digest: d,
Size: int64(l),
},
Layers: []ispec.Descriptor{
{
MediaType: "application/vnd.oci.image.layer.v1.tar",
Digest: d,
Size: int64(l),
},
},
}
m.SchemaVersion = 2
mb, _ = json.Marshal(m)
_ = godigest.FromBytes(mb)
_, err = il.PutImageManifest("replace", "1.0", ispec.MediaTypeImageManifest, mb)
So(err, ShouldBeNil)
})

Convey("Dedupe", func() {
blobDigest1 := ""
blobDigest2 := ""
Expand Down

0 comments on commit 3dc9885

Please sign in to comment.