Skip to content

Replace ocispec.MediaTypeImageManifest with manifest.MediaType#3904

Merged
dmcgowan merged 3 commits intocontainerd:masterfrom
makllama:manifest
Dec 28, 2019
Merged

Replace ocispec.MediaTypeImageManifest with manifest.MediaType#3904
dmcgowan merged 3 commits intocontainerd:masterfrom
makllama:manifest

Conversation

@yeahdongcn
Copy link
Copy Markdown

@yeahdongcn yeahdongcn commented Dec 20, 2019

Signed-off-by: Xiaodong Ye [email protected]

Import a tar with manifest.json, ctr image ls returns incorrect TYPE.

REF                              TYPE                                       DIGEST                                                                  SIZE      PLATFORMS   LABELS 
docker.io/library/myimage:latest application/vnd.oci.image.manifest.v1+json sha256:663a45ec27ca42bd4ad37eab55a7ad96db3ee5d7ad5c6dc5bbec958b9e63ad53 745.9 KiB linux/amd64 -

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 20, 2019

Build succeeded.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 20, 2019

Build succeeded.

Remove an empty line
facedf8 
Signed-off-by: Xiaodong Ye <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 20, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 20, 2019 •
edited
Loading

Codecov Report

Merging #3904 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3904   +/-   ##
=======================================
  Coverage   42.47%   42.47%           
=======================================
  Files         130      130           
  Lines       14716    14716           
=======================================
  Hits         6251     6251           
  Misses       7544     7544           
  Partials      921      921
Flag Coverage Δ
#linux 45.86% <ø> (ø) ⬆️
#windows 38% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1809231...facedf8. Read the comment docs.

@yeahdongcn
Copy link
Copy Markdown
Author

yeahdongcn commented Dec 20, 2019

Import a DockerV2.1 image which contains manifest.json, then push the image back to a local Docker registry, the following error happens.

DEBU[0005] fetch response received                       digest="sha256:c1ca92aa72256a38278018b0786936af31df7b043c0f8d8285f30d0253789c3a" mediatype=application/vnd.oci.image.manifest.v1+json response.header.connection=keep-alive response.header.content-length=82 response.header.content-type="application/json; charset=utf-8" response.header.date="Wed, 18 Dec 2019 10:01:05 GMT" response.header.docker-distribution-api-version=registry/2.0 response.header.x-content-type-options=nosniff response.status="400 Bad Request" size=587 url="http://10.117.175.83:5000/v2/vctl/myimage/manifests/latest"
ctr: failed commit on ref "manifest-sha256:c1ca92aa72256a38278018b0786936af31df7b043c0f8d8285f30d0253789c3a": unexpected status: 400 Bad Request

I think this should be caused by the mediatype is mismatch with the mediatype in the file.
Update the mediatype then the pushing works as expected.

@crosbymichael I have closed #3900

@yeahdongcn yeahdongcn marked this pull request as ready for review December 20, 2019 09:53
@fuweid fuweid requested a review from dmcgowan December 20, 2019 12:47
@fuweid
Copy link
Copy Markdown
Member

fuweid commented Dec 20, 2019

The change is LGTM.

@yeahdongcn could you please provide the log in registry side? Thanks

@yeahdongcn
Copy link
Copy Markdown
Author

yeahdongcn commented Dec 20, 2019

The change is LGTM.

@yeahdongcn could you please provide the log in registry side? Thanks

time="2019-12-20T13:49:19.148656588Z" level=error msg="response completed with error" err.code="manifest unknown" err.detail="unknown tag=latest" err.message="manifest unknown" go.version=go1.11.2 http.request.host="localhost:5000" http.request.id=28eb4731-7417-469c-b1ec-0526b289f1ab http.request.method=HEAD http.request.remoteaddr="172.17.0.1:36682" http.request.uri="/v2/myimage/manifests/latest" http.request.useragent="containerd/v1.3.2-86-g370429a.m" http.response.contenttype="application/json; charset=utf-8" http.response.duration=1.08636ms http.response.status=404 http.response.written=96 vars.name="myimage" vars.reference=latest 
time="2019-12-20T13:49:19.151742947Z" level=error msg="response completed with error" err.code="manifest invalid" err.detail="if present, mediaType in manifest should be 'application/vnd.oci.image.manifest.v1+json' not 'application/vnd.docker.distribution.manifest.v2+json'" err.message="manifest invalid" go.version=go1.11.2 http.request.contenttype="application/vnd.oci.image.manifest.v1+json" http.request.host="localhost:5000" http.request.id=3ad180a3-1e92-4f5b-829d-f91501694afe http.request.method=PUT http.request.remoteaddr="172.17.0.1:36682" http.request.uri="/v2/myimage/manifests/latest" http.request.useragent="containerd/v1.3.2-86-g370429a.m" http.response.contenttype="application/json; charset=utf-8" http.response.duration=1.27431ms http.response.status=400 http.response.written=82 vars.name="myimage" vars.reference=latest

fuweid
fuweid approved these changes Dec 25, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fuweid
Copy link
Copy Markdown
Member

fuweid commented Dec 25, 2019

Checked docker/distribution and the registry server will use http content type header to choose the validator for manifest https://github.com/docker/distribution/blob/master/manifests.go#L106.

The content-type should be consistent with file.

dmcgowan
dmcgowan approved these changes Dec 28, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dmcgowan dmcgowan merged commit f823c37 into containerd:master Dec 28, 2019
@estesp estesp mentioned this pull request Jul 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmcgowan dmcgowan dmcgowan approved these changes

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yeahdongcn @codecov-io @fuweid @dmcgowan @estesp