c8d: test a backend dependent error on pull #46517
Conversation
moby and containerd have slightly different error messages when someone tries to pull an image that doesn't contain the current platform, instead of looking inside the error returned by containerd we match the errors in the test related to what image backend we are using Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
| @@ -285,5 +285,11 @@ func (s *DockerCLIPullSuite) TestPullLinuxImageFailsOnWindows(c *testing.T) { | |||
| func (s *DockerCLIPullSuite) TestPullWindowsImageFailsOnLinux(c *testing.T) { | |||
| testRequires(c, DaemonIsLinux, Network) | |||
| _, _, err := dockerCmdWithError("pull", "mcr.microsoft.com/windows/servercore:ltsc2022") | |||
| assert.ErrorContains(c, err, "no matching manifest for linux") | |||
|
|
|||
| errorMessage := "no matching manifest for linux" | |||
There was a problem hiding this comment.
We probably need to update the test above as well; it currently doesn't fail because we don't run the Windows tests with snapshotters 🤔
There was a problem hiding this comment.
Also thinking of some things to consider; honestly, the containerd error is less informative than the old one, because it doesn't show WHAT we weren't able to match (which could be obvious if a user explicitly passed a --platform, but less so if not, but could also be that user passed a partial platform, and we normalized it).
The "non-snapshotter" error looks to be this one;
Lines 939 to 945 in 0d9da73
For containerd, the error is produced here;
moby/vendor/github.com/containerd/containerd/images/handlers.go
Lines 310 to 312 in 0d9da73
I guess ideally the containerd error would be somewhat more informative (what were we trying to match, and couldn't find?), but it looks like that's less trivial, because that's fully up to the HandlerFunc that's passed (which does the filtering).
The alternative here would be to handle this on our side: when we get the result. The error returned is typed; it's a errdefs.ErrNotFound, so we can decorate the error (and perhaps we can decorate it regardless of what the reason was), i.e.;
if cerrdefs.IsNotFound(err) {
err = fmt.Errorf("no matching manifest for %s: %w", formatPlatform(e.platform), err)
} else {
err = fmt.Errorf("failed to find matching manifest for %s: %w", formatPlatform(e.platform), err)
}(I removed "in the manifest list entries" from the above, but maybe we should keep it, not sure if it's "too much")
There was a problem hiding this comment.
- opened c8d: improve pull errors (non-matching platform, etc) #46765 for the above, as it's not related to the test
- What I did
moby and containerd have slightly different error messages when someone tries to pull an image that doesn't contain the current platform, instead of looking inside the error returned by containerd we match the errors in the test related to what image backend we are using
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)