Parcourir la source

Add support for platform (os and architecture) on image import

Commit 0380fbff37922cadf294851b1546f4c212c7f364 added the ability to pass a
--platform flag on `docker import` when importing an archive. The intent
of that commit was to allow importing a Linux rootfs on a Windows daemon
(as part of the experimental LCOW feature).

A later commit (337ba71fc1124603302e28d94e2f08674e31a756) changed some
of this code to take both OS and Architecture into account (for `docker build`
and `docker pull`), but did not yet update the `docker image import`.

This patch updates the import endpoitn to allow passing both OS and
Architecture. Note that currently only matching OSes are accepted,
and an error will be produced when (e.g.) specifying `linux` on Windows
and vice-versa.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn il y a 3 ans
Parent
commit
01ae9525dd

+ 1 - 1
api/server/router/image/backend.go

@@ -30,7 +30,7 @@ type imageBackend interface {
 
 type importExportBackend interface {
 	LoadImage(inTar io.ReadCloser, outStream io.Writer, quiet bool) error
-	ImportImage(src string, repository, platform string, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error
+	ImportImage(src string, repository string, platform *specs.Platform, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error
 	ExportImage(names []string, outStream io.Writer) error
 }
 

+ 1 - 5
api/server/router/image/image_routes.go

@@ -73,11 +73,7 @@ func (s *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrite
 		progressErr = s.backend.PullImage(ctx, image, tag, platform, metaHeaders, authConfig, output)
 	} else { // import
 		src := r.Form.Get("fromSrc")
-		os := ""
-		if platform != nil {
-			os = platform.OS
-		}
-		progressErr = s.backend.ImportImage(src, repo, os, tag, message, r.Body, output, r.Form["changes"])
+		progressErr = s.backend.ImportImage(src, repo, platform, tag, message, r.Body, output, r.Form["changes"])
 	}
 	if progressErr != nil {
 		if !output.Flushed() {

+ 16 - 1
api/swagger.yaml

@@ -7678,7 +7678,22 @@ paths:
             type: "string"
         - name: "platform"
           in: "query"
-          description: "Platform in the format os[/arch[/variant]]"
+          description: |
+            Platform in the format os[/arch[/variant]].
+
+            When used in combination with the `fromImage` option, the daemon checks
+            if the given image is present in the local image cache with the given
+            OS and Architecture, and otherwise attempts to pull the image. If the
+            option is not set, the host's native OS and Architecture are used.
+            If the given image does not exist in the local image cache, the daemon
+            attempts to pull the image with the host's native OS and Architecture.
+            If the given image does exists in the local image cache, but its OS or
+            architecture does not match, a warning is produced.
+
+            When used with the `fromSrc` option to import an image from an archive,
+            this option sets the platform information for the imported image. If
+            the option is not set, the host's native OS and Architecture are used
+            for the imported image.
           type: "string"
           default: ""
       tags: ["Image"]

+ 13 - 10
daemon/images/image_import.go

@@ -5,10 +5,10 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"runtime"
 	"strings"
 	"time"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile"
@@ -20,6 +20,7 @@ import (
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
 
@@ -27,18 +28,13 @@ import (
 // inConfig (if src is "-"), or from a URI specified in src. Progress output is
 // written to outStream. Repository and tag names can optionally be given in
 // the repo and tag arguments, respectively.
-func (i *ImageService) ImportImage(src string, repository, os string, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error {
+func (i *ImageService) ImportImage(src string, repository string, platform *specs.Platform, tag string, msg string, inConfig io.ReadCloser, outStream io.Writer, changes []string) error {
 	var (
 		rc     io.ReadCloser
 		resp   *http.Response
 		newRef reference.Named
 	)
 
-	// Default the operating system if not supplied.
-	if os == "" {
-		os = runtime.GOOS
-	}
-
 	if repository != "" {
 		var err error
 		newRef, err = reference.ParseNormalizedNamed(repository)
@@ -57,7 +53,13 @@ func (i *ImageService) ImportImage(src string, repository, os string, tag string
 		}
 	}
 
-	config, err := dockerfile.BuildFromConfig(&container.Config{}, changes, os)
+	// Normalize platform - default to the operating system and architecture if not supplied.
+	if platform == nil {
+		p := platforms.DefaultSpec()
+		platform = &p
+	}
+
+	config, err := dockerfile.BuildFromConfig(&container.Config{}, changes, platform.OS)
 	if err != nil {
 		return err
 	}
@@ -102,8 +104,9 @@ func (i *ImageService) ImportImage(src string, repository, os string, tag string
 		V1Image: image.V1Image{
 			DockerVersion: dockerversion.Version,
 			Config:        config,
-			Architecture:  runtime.GOARCH,
-			OS:            os,
+			Architecture:  platform.Architecture,
+			Variant:       platform.Variant,
+			OS:            platform.OS,
 			Created:       created,
 			Comment:       msg,
 		},

+ 6 - 0
docs/api/version-history.md

@@ -34,6 +34,12 @@ keywords: "API, Docker, rcli, REST, documentation"
   (`a disk usage operation is already running`) would be returned in this
   situation. This change is not versioned, and affects all API versions if the
   daemon has this patch.
+* The `POST /images/create` now supports both the operating system and architecture
+  that is passed through the `platform` query parameter when using the `fromSrc`
+  option to import an image from an archive. Previously, only the operating system
+  was used and the architecture was ignored. If no `platform` option is set, the
+  host's operating system and architecture as used as default. This change is not
+  versioned, and affects all API versions if the daemon has this patch.
 
 ## v1.41 API changes
 

+ 98 - 0
integration/image/import_test.go

@@ -6,10 +6,12 @@ import (
 	"context"
 	"io"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/image"
 	"github.com/docker/docker/testutil"
 	"github.com/docker/docker/testutil/daemon"
 	"gotest.tools/v3/assert"
@@ -47,3 +49,99 @@ func TestImportExtremelyLargeImageWorks(t *testing.T) {
 		types.ImageImportOptions{})
 	assert.NilError(t, err)
 }
+
+func TestImportWithCustomPlatform(t *testing.T) {
+	skip.If(t, testEnv.OSType == "windows", "TODO enable on windows")
+
+	defer setupTest(t)()
+	client := testEnv.APIClient()
+	ctx := context.Background()
+
+	// Construct an empty tar archive.
+	var tarBuffer bytes.Buffer
+
+	tw := tar.NewWriter(&tarBuffer)
+	err := tw.Close()
+	assert.NilError(t, err)
+	imageRdr := io.MultiReader(&tarBuffer, io.LimitReader(testutil.DevZero, 0))
+
+	tests := []struct {
+		name        string
+		platform    string
+		expected    image.V1Image
+		expectedErr string
+	}{
+		{
+			platform: "",
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform:    "       ",
+			expectedErr: "is an invalid component",
+		},
+		{
+			platform:    "/",
+			expectedErr: "is an invalid component",
+		},
+		{
+			platform: runtime.GOOS,
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform: strings.ToUpper(runtime.GOOS),
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: runtime.GOARCH, // this may fail on armhf due to normalization?
+			},
+		},
+		{
+			platform: runtime.GOOS + "/sparc64",
+			expected: image.V1Image{
+				OS:           runtime.GOOS,
+				Architecture: "sparc64",
+			},
+		},
+		{
+			platform:    "macos",
+			expectedErr: "operating system is not supported",
+		},
+		{
+			platform:    "macos/arm64",
+			expectedErr: "operating system is not supported",
+		},
+		{
+			// TODO: platforms.Normalize() only validates os or arch if a single component is passed,
+			//       but ignores unknown os/arch in other cases. See:
+			//       https://github.com/containerd/containerd/blob/7d4891783aac5adf6cd83f657852574a71875631/platforms/platforms.go#L183-L209
+			platform:    "nintendo64",
+			expectedErr: "unknown operating system or architecture",
+		},
+	}
+
+	for i, tc := range tests {
+		tc := tc
+		t.Run(tc.platform, func(t *testing.T) {
+			reference := "import-with-platform:tc-" + strconv.Itoa(i)
+			_, err = client.ImageImport(context.Background(),
+				types.ImageImportSource{Source: imageRdr, SourceName: "-"},
+				reference,
+				types.ImageImportOptions{Platform: tc.platform})
+			if tc.expectedErr != "" {
+				assert.ErrorContains(t, err, tc.expectedErr)
+			} else {
+				assert.NilError(t, err)
+
+				inspect, _, err := client.ImageInspectWithRaw(ctx, reference)
+				assert.NilError(t, err)
+				assert.Equal(t, inspect.Os, tc.expected.OS)
+				assert.Equal(t, inspect.Architecture, tc.expected.Architecture)
+			}
+		})
+	}
+}