Move cpu variant checks into platform matcher
Wrap platforms.Only and fallback to our ignore mismatches due to empty CPU variants. This just cleans things up and makes the logic re-usable in other places. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
This commit is contained in:
parent
4be5453215
commit
50f39e7247
|
@ -12,6 +12,7 @@ import (
|
||||||
containertypes "github.com/docker/docker/api/types/container"
|
containertypes "github.com/docker/docker/api/types/container"
|
||||||
networktypes "github.com/docker/docker/api/types/network"
|
networktypes "github.com/docker/docker/api/types/network"
|
||||||
"github.com/docker/docker/container"
|
"github.com/docker/docker/container"
|
||||||
|
"github.com/docker/docker/daemon/images"
|
||||||
"github.com/docker/docker/errdefs"
|
"github.com/docker/docker/errdefs"
|
||||||
"github.com/docker/docker/image"
|
"github.com/docker/docker/image"
|
||||||
"github.com/docker/docker/pkg/idtools"
|
"github.com/docker/docker/pkg/idtools"
|
||||||
|
@ -89,7 +90,7 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
|
||||||
Variant: img.Variant,
|
Variant: img.Variant,
|
||||||
}
|
}
|
||||||
|
|
||||||
if !platforms.Only(p).Match(imgPlat) {
|
if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
|
||||||
warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
|
warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -55,6 +55,8 @@ func (i *ImageService) manifestMatchesPlatform(img *image.Image, platform specs.
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Note we are comparing against manifest lists here, which we expect to always have a CPU variant set (where applicable).
|
||||||
|
// So there is no need for the fallback matcher here.
|
||||||
comparer := platforms.Only(platform)
|
comparer := platforms.Only(platform)
|
||||||
|
|
||||||
var (
|
var (
|
||||||
|
@ -161,31 +163,24 @@ func (i *ImageService) GetImage(refOrID string, platform *specs.Platform) (retIm
|
||||||
p := *platform
|
p := *platform
|
||||||
// Note that `platforms.Only` will fuzzy match this for us
|
// Note that `platforms.Only` will fuzzy match this for us
|
||||||
// For example: an armv6 image will run just fine an an armv7 CPU, without emulation or anything.
|
// For example: an armv6 image will run just fine an an armv7 CPU, without emulation or anything.
|
||||||
if !platforms.Only(p).Match(imgPlat) {
|
if OnlyPlatformWithFallback(p).Match(imgPlat) {
|
||||||
// Sometimes image variant is not populated due to legacy builders
|
|
||||||
// We still should support falling back here.
|
|
||||||
if imgPlat.OS == platform.OS && imgPlat.Architecture == platform.Architecture && imgPlat.Variant == "" {
|
|
||||||
logrus.WithField("image", refOrID).WithField("platform", platforms.Format(p)).Debug("Image platform cpu variant is not populated, but otherwise it matches what was requested")
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
// In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly)
|
|
||||||
// So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image.
|
|
||||||
if i.manifestMatchesPlatform(retImg, p) {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
// This allows us to tell clients that we don't have the image they asked for
|
|
||||||
// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
|
|
||||||
// An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
|
|
||||||
// The image store does not store the manifest list and image tags are assigned to architecture specific images.
|
|
||||||
// So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
|
|
||||||
// This may be confusing.
|
|
||||||
// The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
|
|
||||||
// able to automatically tell what causes the conflict.
|
|
||||||
retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// In some cases the image config can actually be wrong (e.g. classic `docker build` may not handle `--platform` correctly)
|
||||||
|
// So we'll look up the manifest list that coresponds to this imaage to check if at least the manifest list says it is the correct image.
|
||||||
|
if i.manifestMatchesPlatform(retImg, p) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// This allows us to tell clients that we don't have the image they asked for
|
||||||
|
// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
|
||||||
|
// An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
|
||||||
|
// The image store does not store the manifest list and image tags are assigned to architecture specific images.
|
||||||
|
// So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
|
||||||
|
// This may be confusing.
|
||||||
|
// The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
|
||||||
|
// able to automatically tell what causes the conflict.
|
||||||
|
retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform: wanted %s, actual: %s", refOrID, platforms.Format(p), platforms.Format(imgPlat)))
|
||||||
}()
|
}()
|
||||||
ref, err := reference.ParseAnyReference(refOrID)
|
ref, err := reference.ParseAnyReference(refOrID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
@ -223,3 +218,34 @@ func (i *ImageService) GetImage(refOrID string, platform *specs.Platform) (retIm
|
||||||
|
|
||||||
return nil, ErrImageDoesNotExist{ref}
|
return nil, ErrImageDoesNotExist{ref}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// OnlyPlatformWithFallback uses `platforms.Only` with a fallback to handle the case where the platform
|
||||||
|
// being matched does not have a CPU variant.
|
||||||
|
//
|
||||||
|
// The reason for this is that CPU variant is not even if the official image config spec as of this writing.
|
||||||
|
// See: https://github.com/opencontainers/image-spec/pull/809
|
||||||
|
// Since Docker tends to compare platforms from the image config, we need to handle this case.
|
||||||
|
func OnlyPlatformWithFallback(p specs.Platform) platforms.Matcher {
|
||||||
|
return &onlyFallbackMatcher{only: platforms.Only(p), p: platforms.Normalize(p)}
|
||||||
|
}
|
||||||
|
|
||||||
|
type onlyFallbackMatcher struct {
|
||||||
|
only platforms.Matcher
|
||||||
|
p specs.Platform
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *onlyFallbackMatcher) Match(other specs.Platform) bool {
|
||||||
|
if m.only.Match(other) {
|
||||||
|
// It matches, no reason to fallback
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if other.Variant != "" {
|
||||||
|
// If there is a variant then this fallback does not apply, and there is no match
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
otherN := platforms.Normalize(other)
|
||||||
|
otherN.Variant = "" // normalization adds a default variant... which is the whole problem with `platforms.Only`
|
||||||
|
|
||||||
|
return m.p.OS == otherN.OS &&
|
||||||
|
m.p.Architecture == otherN.Architecture
|
||||||
|
}
|
||||||
|
|
|
@ -0,0 +1,33 @@
|
||||||
|
package images
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
specs "github.com/opencontainers/image-spec/specs-go/v1"
|
||||||
|
"gotest.tools/v3/assert"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestOnlyPlatformWithFallback(t *testing.T) {
|
||||||
|
p := specs.Platform{
|
||||||
|
OS: "linux",
|
||||||
|
Architecture: "arm",
|
||||||
|
Variant: "v8",
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check no variant
|
||||||
|
assert.Assert(t, OnlyPlatformWithFallback(p).Match(specs.Platform{
|
||||||
|
OS: p.OS,
|
||||||
|
Architecture: p.Architecture,
|
||||||
|
}))
|
||||||
|
// check with variant
|
||||||
|
assert.Assert(t, OnlyPlatformWithFallback(p).Match(specs.Platform{
|
||||||
|
OS: p.OS,
|
||||||
|
Architecture: p.Architecture,
|
||||||
|
Variant: p.Variant,
|
||||||
|
}))
|
||||||
|
// Make sure non-matches are false.
|
||||||
|
assert.Assert(t, !OnlyPlatformWithFallback(p).Match(specs.Platform{
|
||||||
|
OS: p.OS,
|
||||||
|
Architecture: "amd64",
|
||||||
|
}))
|
||||||
|
}
|
Loading…
Reference in New Issue