Skip to content

images: provide better error for manifest list match error#2410

Merged
estesp merged 1 commit intocontainerd:masterfrom
tonistiigi:mlist-error
Jun 21, 2018
Merged

images: provide better error for manifest list match error#2410
estesp merged 1 commit intocontainerd:masterfrom
tonistiigi:mlist-error

Conversation

@tonistiigi
Copy link
Member

Provide a better error if pull fails because there is no match for current platform in the manifest list.

Instead of returning early I remember manifest list in a bool to not break possible cases of nested manifest lists.

Error on pull:

Previously:

manifest sha256:32efb15cbb8a46f4b564857e534fd4c26a4a15813885f91b1a95b5758fea3fd5: not found

New:

no match for current platform linux/arm/v7 in manifest sha256:32efb15cbb8a46f4b564857e534fd4c26a4a15813885f91b1a95b5758fea3fd5: not found

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2410   +/-   ##
=======================================
  Coverage   44.99%   44.99%           
=======================================
  Files          92       92           
  Lines        9398     9398           
=======================================
  Hits         4229     4229           
  Misses       4486     4486           
  Partials      683      683
Flag Coverage Δ
#linux 49.23% <ø> (ø) ⬆️
#windows 41.26% <ø> (ø) ⬆️

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 c1e1f3d...53fe31d. Read the comment docs.

return ocispec.Manifest{}, errors.Wrapf(errdefs.ErrNotFound, "manifest %v", image.Digest)
err := errors.Wrapf(errdefs.ErrNotFound, "manifest %v", image.Digest)
if wasIndex {
err = errors.Wrapf(errdefs.ErrNotFound, "no match for current platform %s in manifest %v", platforms.Format(p), image.Digest)
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to just use platform here instead of reformatting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Parse() normalizes the platform so it becomes more clear.

Copy link
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

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 7ff2748 into containerd:master Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants