Skip to content

Improve 'no matching manifest' error message#38574

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
StefanScherer:improve-no-matching-manifest-error
Feb 2, 2019
Merged

Improve 'no matching manifest' error message#38574
cpuguy83 merged 1 commit intomoby:masterfrom
StefanScherer:improve-no-matching-manifest-error

Conversation

@StefanScherer
Copy link
Contributor

@StefanScherer StefanScherer commented Jan 15, 2019 •
edited
Loading

fixes #38199

- What I did

I improved the error message when someone tries to pull an image, but the manifest list only has manifests that doesn't match the current platform. At the moment only

no matching manifest for unknown in the manifest list entries

appears. The current platform is shown as "unknown" which is not helpful to see if someone is running Docker Desktop on Windows 10 with Linux containers or if the Windows OS version does not match.

Even on a Linux system we can reproduce the problem when we try to pull the newest nanoserver image which is a manifest list:

$ docker pull mcr.microsoft.com/windows/nanoserver:1809
1809: Pulling from windows/nanoserver
no matching manifest for unknown in the manifest list entries

- How I did it

For Windows the error message now shows windows/amd64 and the OS version number instead of "unknown".
For other platforms the error message now shows OS with the current CPU architecture instead of "unknown".

- How to verify it

make test-unit

There are unit tests.

A new Docker engine on Windows now shows a much more detailed error message

$ docker pull ubuntu
Using default tag: latest
latest: Pulling from library/ubuntu
no matching manifest for windows/amd64 10.0.17763 in the manifest list entries

- Description for the changelog

Show the current platform and architecture when pulling from a manifest list doesn't find any match.

- A picture of a cute animal (not mandatory but encouraged)

koala

@StefanScherer StefanScherer mentioned this pull request Jan 15, 2019
Closed
@thaJeztah
Copy link
Member

thaJeztah commented Jan 15, 2019

Looks like you need to gofmt some files;

08:06:48 Congratulations!  All commits are properly signed with the DCO!
08:08:48 distribution/pull_v2_test.go:1::warning: file is not gofmted with -s (gofmt)
08:08:48 distribution/pull_v2_test.go:1::warning: file is not goimported (goimports)
08:08:48 distribution/pull_v2_windows.go:1::warning: file is not goimported (goimports)

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jan 15, 2019
@StefanScherer StefanScherer force-pushed the improve-no-matching-manifest-error branch from 9614103 to 363f1f6 Compare January 15, 2019 14:02
@StefanScherer
Copy link
Contributor Author

StefanScherer commented Jan 15, 2019

Oh I'll have to check the windows code.

@codecov
Copy link

codecov bot commented Jan 15, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@5ebb679). Click here to learn what that means.
The diff coverage is 80%.

@@            Coverage Diff            @@
##             master   #38574   +/-   ##
=========================================
  Coverage          ?   37.02%           
=========================================
  Files             ?      610           
  Lines             ?    45789           
  Branches          ?        0           
=========================================
  Hits              ?    16952           
  Misses            ?    26507           
  Partials          ?     2330

@StefanScherer
Copy link
Contributor Author

StefanScherer commented Jan 15, 2019

Wow, I thought if checking for "10.0." is a good idea for the Windows unit test, but I thought what could go wrong, "every" Windows has this version number for years. Now I've added printing out the actual result: 'windows/amd64 6.2.9200' 😂

@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Jan 15, 2019
@thaJeztah
Copy link
Member

thaJeztah commented Jan 15, 2019

Might want to squash the commits before merging 🤗

@StefanScherer
Copy link
Contributor Author

StefanScherer commented Jan 15, 2019

@thaJeztah I will do that definitely, now that all tests work 😅

@StefanScherer StefanScherer force-pushed the improve-no-matching-manifest-error branch from cfb42ad to 4b9db20 Compare January 15, 2019 17:24
cpuguy83
cpuguy83 approved these changes Feb 1, 2019
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

thaJeztah
thaJeztah approved these changes Feb 1, 2019
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit 50e63ad into moby:master Feb 2, 2019
@lowenna
Copy link
Member

lowenna commented Feb 2, 2019

Wow, I thought if checking for "10.0." is a good idea for the Windows unit test, but I thought what could go wrong, "every" Windows has this version number for years. Now I've added printing out the actual result: 'windows/amd64 6.2.9200' 😂

This is because the test isn't manifested. See GetVersionEx documentation.

@StefanScherer StefanScherer deleted the improve-no-matching-manifest-error branch February 2, 2019 07:11
StefanScherer added a commit to StefanScherer/docker that referenced this pull request Apr 17, 2019
@StefanScherer
Add regression tests for moby#38574
7f03e23 
Signed-off-by: Stefan Scherer <[email protected]>
This was referenced Apr 17, 2019
Closed
Merged
@thaJeztah thaJeztah mentioned this pull request Apr 17, 2019
Closed
@StefanScherer StefanScherer mentioned this pull request May 10, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@AkihiroSuda AkihiroSuda

Labels

area/distribution Image Distribution platform/windows status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows: unhelpful error message when pulling an up-level, multi-arch image

6 participants

@StefanScherer @thaJeztah @lowenna @cpuguy83 @AkihiroSuda @GordonTheTurtle