Browse code

Don't set default platform on container create

This fixes a regression based on expectations of the runtime:

```
docker pull arm32v7/alpine
docker run arm32v7/alpine
```

Without this change, the `docker run` will fail due to platform
matching on non-arm32v7 systems, even though the image could run
(assuming the system is setup correctly).

This also emits a warning to make sure that the user is aware that a
platform that does not match the default platform of the system is being
run, for the cases like:

```
docker pull --platform armhf busybox
docker run busybox
```

Not typically an issue if the requests are done together like that, but
if the image was already there and someone did `docker run` without an
explicit `--platform`, they may very well be expecting to run a native
version of the image instead of the armhf one.

This warning does add some extra noise in the case of platform specific
images being run, such as `arm32v7/alpine`, but this can be supressed by
explicitly setting the platform.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2020/10/16 08:01:17
Showing 5 changed files
... ...
@@ -96,7 +96,8 @@ RUN /download-frozen-image-v2.sh /build \
96 96
         busybox:latest@sha256:95cf004f559831017cdf4628aaf1bb30133677be8702a8c5f2994629f637a209 \
97 97
         busybox:glibc@sha256:1f81263701cddf6402afe9f33fca0266d9fff379e59b1748f33d3072da71ee85 \
98 98
         debian:buster@sha256:46d659005ca1151087efa997f1039ae45a7bf7a2cbbe2d17d3dcbda632a3ee9a \
99
-        hello-world:latest@sha256:d58e752213a51785838f9eed2b7a498ffa1cb3aa7f946dda11af39286c3db9a9
99
+        hello-world:latest@sha256:d58e752213a51785838f9eed2b7a498ffa1cb3aa7f946dda11af39286c3db9a9 \
100
+        arm32v7/hello-world:latest@sha256:50b8560ad574c779908da71f7ce370c0a2471c098d44d1c8f6b513c5a55eeeb1
100 101
 # See also ensureFrozenImagesLinux() in "integration-cli/fixtures_linux_daemon_test.go" (which needs to be updated when adding images to this list)
101 102
 
102 103
 FROM base AS cross-false
... ...
@@ -510,17 +510,6 @@ func (s *containerRouter) postContainersCreate(ctx context.Context, w http.Respo
510 510
 			}
511 511
 			platform = &p
512 512
 		}
513
-		defaultPlatform := platforms.DefaultSpec()
514
-		if platform == nil {
515
-			platform = &defaultPlatform
516
-		}
517
-		if platform.OS == "" {
518
-			platform.OS = defaultPlatform.OS
519
-		}
520
-		if platform.Architecture == "" {
521
-			platform.Architecture = defaultPlatform.Architecture
522
-			platform.Variant = defaultPlatform.Variant
523
-		}
524 513
 	}
525 514
 
526 515
 	if hostConfig != nil && hostConfig.PidsLimit != nil && *hostConfig.PidsLimit <= 0 {
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"strings"
8 8
 	"time"
9 9
 
10
+	"github.com/containerd/containerd/platforms"
10 11
 	"github.com/docker/docker/api/types"
11 12
 	containertypes "github.com/docker/docker/api/types/container"
12 13
 	networktypes "github.com/docker/docker/api/types/network"
... ...
@@ -16,6 +17,7 @@ import (
16 16
 	"github.com/docker/docker/pkg/idtools"
17 17
 	"github.com/docker/docker/pkg/system"
18 18
 	"github.com/docker/docker/runconfig"
19
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
19 20
 	"github.com/opencontainers/selinux/go-selinux"
20 21
 	"github.com/pkg/errors"
21 22
 	"github.com/sirupsen/logrus"
... ...
@@ -59,8 +61,10 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
59 59
 	}
60 60
 
61 61
 	os := runtime.GOOS
62
+	var img *image.Image
62 63
 	if opts.params.Config.Image != "" {
63
-		img, err := daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform)
64
+		var err error
65
+		img, err = daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform)
64 66
 		if err == nil {
65 67
 			os = img.OS
66 68
 		}
... ...
@@ -77,6 +81,19 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
77 77
 		return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, errdefs.InvalidParameter(err)
78 78
 	}
79 79
 
80
+	if img != nil && opts.params.Platform == nil {
81
+		p := platforms.DefaultSpec()
82
+		imgPlat := v1.Platform{
83
+			OS:           img.OS,
84
+			Architecture: img.Architecture,
85
+			Variant:      img.Variant,
86
+		}
87
+
88
+		if !platforms.Only(p).Match(imgPlat) {
89
+			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)))
90
+		}
91
+	}
92
+
80 93
 	err = verifyNetworkingConfig(opts.params.NetworkingConfig)
81 94
 	if err != nil {
82 95
 		return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, errdefs.InvalidParameter(err)
... ...
@@ -3,6 +3,7 @@ package images // import "github.com/docker/docker/daemon/images"
3 3
 import (
4 4
 	"fmt"
5 5
 
6
+	"github.com/containerd/containerd/platforms"
6 7
 	"github.com/pkg/errors"
7 8
 
8 9
 	"github.com/docker/distribution/reference"
... ...
@@ -34,30 +35,24 @@ func (i *ImageService) GetImage(refOrID string, platform *specs.Platform) (retIm
34 34
 			return
35 35
 		}
36 36
 
37
-		// This allows us to tell clients that we don't have the image they asked for
38
-		// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
39
-		//   An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
40
-		//   The image store does not store the manifest list and image tags are assigned to architecture specific images.
41
-		//   So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
42
-		//   This may be confusing.
43
-		//   The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
44
-		//   able to automatically tell what causes the conflict.
45
-		if retImg.OS != platform.OS {
46
-			retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified OS platform: wanted: %s, actual: %s", refOrID, platform.OS, retImg.OS))
47
-			retImg = nil
48
-			return
49
-		}
50
-		if retImg.Architecture != platform.Architecture {
51
-			retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform cpu architecture: wanted: %s, actual: %s", refOrID, platform.Architecture, retImg.Architecture))
52
-			retImg = nil
53
-			return
37
+		imgPlat := specs.Platform{
38
+			OS:           retImg.OS,
39
+			Architecture: retImg.Architecture,
40
+			Variant:      retImg.Variant,
54 41
 		}
55
-
56
-		// Only validate variant if retImg has a variant set.
57
-		// The image variant may not be set since it's a newer field.
58
-		if platform.Variant != "" && retImg.Variant != "" && retImg.Variant != platform.Variant {
59
-			retErr = errdefs.NotFound(errors.Errorf("image with reference %s was found but does not match the specified platform cpu architecture variant: wanted: %s, actual: %s", refOrID, platform.Variant, retImg.Variant))
60
-			retImg = nil
42
+		p := *platform
43
+		// Note that `platforms.Only` will fuzzy match this for us
44
+		// For example: an armv6 image will run just fine an an armv7 CPU, without emulation or anything.
45
+		if !platforms.Only(p).Match(imgPlat) {
46
+			// This allows us to tell clients that we don't have the image they asked for
47
+			// Where this gets hairy is the image store does not currently support multi-arch images, e.g.:
48
+			//   An image `foo` may have a multi-arch manifest, but the image store only fetches the image for a specific platform
49
+			//   The image store does not store the manifest list and image tags are assigned to architecture specific images.
50
+			//   So we can have a `foo` image that is amd64 but the user requested armv7. If the user looks at the list of images.
51
+			//   This may be confusing.
52
+			//   The alternative to this is to return a errdefs.Conflict error with a helpful message, but clients will not be
53
+			//   able to automatically tell what causes the conflict.
54
+			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)))
61 55
 			return
62 56
 		}
63 57
 	}()
... ...
@@ -508,3 +508,23 @@ func TestCreateVolumesFromNonExistingContainer(t *testing.T) {
508 508
 	)
509 509
 	assert.Check(t, errdefs.IsInvalidParameter(err))
510 510
 }
511
+
512
+// Test that we can create a container from an image that is for a different platform even if a platform was not specified
513
+// This is for the regression detailed here: https://github.com/moby/moby/issues/41552
514
+func TestCreatePlatformSpecificImageNoPlatform(t *testing.T) {
515
+	defer setupTest(t)()
516
+
517
+	skip.If(t, testEnv.DaemonInfo.Architecture == "arm", "test only makes sense to run on non-arm systems")
518
+	skip.If(t, testEnv.OSType != "linux", "test image is only available on linux")
519
+	cli := testEnv.APIClient()
520
+
521
+	_, err := cli.ContainerCreate(
522
+		context.Background(),
523
+		&container.Config{Image: "arm32v7/hello-world"},
524
+		&container.HostConfig{},
525
+		nil,
526
+		nil,
527
+		"",
528
+	)
529
+	assert.NilError(t, err)
530
+}