From 01ae9525dd7d3e1fac7bd1b05f1d3286d853ff3a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 22 Dec 2021 17:21:27 +0100 Subject: [PATCH] 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 --- api/server/router/image/backend.go | 2 +- api/server/router/image/image_routes.go | 6 +- api/swagger.yaml | 17 ++++- daemon/images/image_import.go | 23 +++--- docs/api/version-history.md | 6 ++ integration/image/import_test.go | 98 +++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 17 deletions(-) diff --git a/api/server/router/image/backend.go b/api/server/router/image/backend.go index 624b6c7363..d54d2c0e6c 100644 --- a/api/server/router/image/backend.go +++ b/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 } diff --git a/api/server/router/image/image_routes.go b/api/server/router/image/image_routes.go index 59722fe861..71b0c14f64 100644 --- a/api/server/router/image/image_routes.go +++ b/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() { diff --git a/api/swagger.yaml b/api/swagger.yaml index 3b1ab5e404..46505d06c0 100644 --- a/api/swagger.yaml +++ b/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"] diff --git a/daemon/images/image_import.go b/daemon/images/image_import.go index bd0f60c674..bec67f68a4 100644 --- a/daemon/images/image_import.go +++ b/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, }, diff --git a/docs/api/version-history.md b/docs/api/version-history.md index e782fa6782..d6e44c4bbc 100644 --- a/docs/api/version-history.md +++ b/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 diff --git a/integration/image/import_test.go b/integration/image/import_test.go index 2880df8d32..110ab87a5f 100644 --- a/integration/image/import_test.go +++ b/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) + } + }) + } +}