API: add Plaform (OS and Architecture) to /containers/json #42464

thaJeztah · 2021-06-04T12:56:02Z

fixes Expose platform in docker ps #47764

Very quick, 10 minute attempt at adding platform information to containers (for docker ps). This information is useful for (e.g.) Docker Desktop, where non-matching architectures can be run, using QEMU binfmt.

Lots of things to do still (besides "design"), also considering:

filter by architecture
perhaps a way to indicate that a container/image is non-native (useful to provide that information, so that clients don't have to replicate the "variants" such as arm/v5 running on arm64 (which is not "matching", but no emulation should be needed in that case); similarly, Windows OS versions (?)
check for consistency, e.g. docker inspect shows a Platform: linux, but does not contain Architecture.

docker run -d --name foo nginx:alpine

curl --unix-socket /var/run/docker.sock http://localhost/containers/json | jq

[
  {
    "Id": "cf30128fc519f22d74fff234757a1592a97c05a7e1c5e411eef394d8dad571b9",
    "Names": [
      "/foo"
    ],
    "Image": "nginx:alpine",
    "ImageID": "sha256:a6eb2a334a9fcf41788e0cff252f749df07427df29e50f0b141cc5ed1094006d",
    "Command": "/docker-entrypoint.sh nginx -g 'daemon off;'",
    "Created": 1622809767,
    "Ports": [
      {
        "PrivatePort": 80,
        "Type": "tcp"
      }
    ],
    "Labels": {
      "maintainer": "NGINX Docker Maintainers <docker-maint@nginx.com>"
    },
    "State": "running",
    "Status": "Up About a minute",
    "HostConfig": {
      "NetworkMode": "default"
    },
    "Platform": {
      "Architecture": "amd64",
      "OS": "linux"
    },
    "NetworkSettings": {
      "Networks": {
        "bridge": {
          "IPAMConfig": null,
          "Links": null,
          "Aliases": null,
          "NetworkID": "45a313a87b5237c5d0ddf5d5de01e44dd2940af95e74ba34ebd760ccb02acfb3",
          "EndpointID": "ad9557d54bb9bb63995760a911296e6ef2a6dd22c1ea65bcb58998caae6537b5",
          "Gateway": "172.18.0.1",
          "IPAddress": "172.18.0.2",
          "IPPrefixLen": 16,
          "IPv6Gateway": "",
          "GlobalIPv6Address": "",
          "GlobalIPv6PrefixLen": 0,
          "MacAddress": "02:42:ac:12:00:02",
          "DriverOpts": null
        }
      }
    },
    "Mounts": []
  }
]

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2021-06-30T12:55:21Z

Depends on #42063 (included)

thaJeztah · 2021-06-30T12:58:33Z

api/types/types.go

@@ -54,13 +54,37 @@ type ImageMetadata struct {
 	LastTagTime time.Time `json:",omitempty"`
 }

+// ImagePlatform describes the platform which the image in the manifest runs on.
+// This type is our equivalent of ocispec.Platform, with Capitalized JSON field names.


Question: the /distribution/{name}/json endpoint already returns platform information, but that's a direct use of the ocispec.Platform, which means that that endpoint does not use the same capitalisation.

Should we omit this type (and use a direct copy of the OCI-spec, or change the other endpoint?

In general, I prefer that we define our own types for the API (in either case), instead of types we don't (directly) control (to prevent inadvertently changing API responses)

Hmm, it seems strange IMO for us to be returning what's effectively an "OCI Platform" object without using the canonical capitalization for the fields, so I'd vote anywhere we return a full proper "platform object" that it be the lowercase OCI versions.

(I don't have a strong opinion one way or another on using the OCI structs directly.)

Same. We can use our own types if desired but it would be nice to use the industry standard field name format.

Oh! Thought I pushed 🤦

I updated to use the same casing as upstream; still using our own type (with the addition of a Stringer() interface), but otherwise same as the OCI type

thaJeztah · 2021-07-05T13:37:45Z

daemon/container.go

+		// TODO do we need a "fallback" here for images that have empty OS/Arch/etc (in which case, use runtime.GOARCH?)
+		//      would be nice if img had methods for this with that implemented (img.OS(), img.Architecture(), img.Platform())
+		base.Platform = ocispec.Platform{


Do we want to "normalize" this information here, or keep it "as-is" and handle it on the client side? https://github.com/containerd/containerd/blob/v1.5.2/platforms/platforms.go#L265-L278

I added containerd's Normalize code here, but perhaps this is something to be discussed.

IMO doing the normalize client-side makes sense (or doing the normalize before we ever store/use the data in the first place 😇)

I originally started with that (do the normalising on the client side), then noticed that "normalising" is doing more than just changing casing and (e.g.) x86 -> amd64. It's also filling in missing information based on the current platform it's running on (which obviously wouldn't work if the client is running on a different platform as the daemon.

From that, I was also wondering if the client should just present the information the daemon provides (in most situations i think that's the ideal), hence I moved it here.

or doing the normalize before we ever store/use the data in the first place

You mean "when we store the image" ? That felt a bit tricky; should we modify image data?

Arg, sorry, didn't realize this was straight from the image. In that case I'd be even more in favor of the API passing the data as-is all the way through and letting the client decide whether to normalize before presentation.

I don't think there are cases where containerd's Normalize will insert the current architecture's info, right? Doesn't amd64 always means linux/amd64, even on Windows, for example?

It does normalise the OS to be the "current" OS, so potentially show darwin or macOS on a Mac CLI; https://github.com/containerd/containerd/blob/36cc874494a56a253cd181a1a685b44b58a2e34a/platforms/database.go#L69-L80

func normalizeOS(os string) string { if os == "" { return runtime.GOOS } os = strings.ToLower(os) switch os { case "macos": os = "darwin" } return os }

Ahhh, that's unfortunate (I guess that's a quirk of part of these APIs being designed for data storage / display and part of them being designed for querying, like the fallback vector code...) 😞

Sounds like the best thing to do in this case then is just to display as-is -- if an image is missing the os value, we probably shouldn't inject/infer one for display, right?

thaJeztah · 2021-07-05T19:40:01Z

@tonistiigi @tianon @cpuguy83 PTAL. I think this is ready for review, but I left some comments above about things that may need input;

I also opened a draft PR for the CLI; docker/cli#3177 (TBD whether or not PLATFORM should be shown by default).

integration/container/ps_test.go

Very quick, 10 minute attempt at adding platform information to containers (for `docker ps`). This information is useful for (e.g.) Docker Desktop, where non-matching architectures can be run, using QEMU binfmt. Lots of things to do still (besides "design"), also considering: - filter by architecture - perhaps a way to indicate that a container/image is non-native (useful to provide that information, so that clients don't have to replicate the "variants" such as arm/v5 running on arm64 (which is not "matching", but no emulation should be needed in that case); similarly, Windows OS versions (?) - check for consistency, e.g. `docker inspect` shows a `Platform: linux`, but does not contain `Architecture`. ```bash docker run -d --name foo nginx:alpine curl --unix-socket /var/run/docker.sock http://localhost/containers/json | jq ``` ```json [ { "Id": "cf30128fc519f22d74fff234757a1592a97c05a7e1c5e411eef394d8dad571b9", "Names": [ "/foo" ], "Image": "nginx:alpine", "ImageID": "sha256:a6eb2a334a9fcf41788e0cff252f749df07427df29e50f0b141cc5ed1094006d", "Command": "/docker-entrypoint.sh nginx -g 'daemon off;'", "Created": 1622809767, "Ports": [ { "PrivatePort": 80, "Type": "tcp" } ], "Labels": { "maintainer": "NGINX Docker Maintainers <docker-maint@nginx.com>" }, "State": "running", "Status": "Up About a minute", "HostConfig": { "NetworkMode": "default" }, "Platform": { "architecture": "amd64", "os": "linux" }, "NetworkSettings": { "Networks": { "bridge": { "IPAMConfig": null, "Links": null, "Aliases": null, "NetworkID": "45a313a87b5237c5d0ddf5d5de01e44dd2940af95e74ba34ebd760ccb02acfb3", "EndpointID": "ad9557d54bb9bb63995760a911296e6ef2a6dd22c1ea65bcb58998caae6537b5", "Gateway": "172.18.0.1", "IPAddress": "172.18.0.2", "IPPrefixLen": 16, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "02:42:ac:12:00:02", "DriverOpts": null } } }, "Mounts": [] } ] ``` Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

With this change, on API v1.42: curl -s --unix-socket /var/run/docker.sock "http://localhost/v1.42/containers/json?all=1" | jq . [ { "Id": "7291db238bf9aaa02e42e09557aa5e52151a9f3b62f95049d6ce7448b5e2df78", "Names": [ "/stupefied_morse" ], "Image": "hello-world", "ImageID": "sha256:d1165f2212346b2bab48cb01c1e39ee8ad1be46b87873d9ca7a4e434980a7726", "Platform": { "Architecture": "amd64", "OS": "linux" }, "Command": "/hello", "Created": 1625405554, "Ports": [], "Labels": {}, "State": "created", "Status": "Created", "HostConfig": { "NetworkMode": "default" }, "NetworkSettings": { "Networks": { "bridge": { "IPAMConfig": null, "Links": null, "Aliases": null, "NetworkID": "", "EndpointID": "", "Gateway": "", "IPAddress": "", "IPPrefixLen": 0, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "", "DriverOpts": null } } }, "Mounts": [] } ] And on API v1.41 and before: docker create hello-world curl -s --unix-socket /var/run/docker.sock "http://localhost/v1.41/containers/json?all=1" | jq . [ { "Id": "7291db238bf9aaa02e42e09557aa5e52151a9f3b62f95049d6ce7448b5e2df78", "Names": [ "/stupefied_morse" ], "Image": "hello-world", "ImageID": "sha256:d1165f2212346b2bab48cb01c1e39ee8ad1be46b87873d9ca7a4e434980a7726", "Command": "/hello", "Created": 1625405554, "Ports": [], "Labels": {}, "State": "created", "Status": "Created", "HostConfig": { "NetworkMode": "default" }, "NetworkSettings": { "Networks": { "bridge": { "IPAMConfig": null, "Links": null, "Aliases": null, "NetworkID": "", "EndpointID": "", "Gateway": "", "IPAddress": "", "IPPrefixLen": 0, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "", "DriverOpts": null } } }, "Mounts": [] } ] Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

This allows the platform to be printed as a string. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2021-07-23T10:11:01Z

Updated the test; ptal

api/types/types.go

tianon

Dunno if this still makes sense in containerd-integration, but LGTM 😇

tianon · 2024-04-26T22:45:58Z

api/swagger.yaml

+    type: "object"
+    x-nullable: true
+    description: |
+      Platform information of the image that the container was created from.


Should this mention explicitly that this follows the OCI definition of the fields?

(Maybe we need a PR to the image-spec to add an anchor to https://github.com/opencontainers/image-spec/blob/v1.1.0/image-index.md#:~:text=generate%20an%20error.-,platform%20object,-This%20OPTIONAL%20property -- I find myself linking to it constantly with this janky browser-specific syntax 😂)

thaJeztah added area/api status/1-design-review impact/api impact/changelog impact/documentation labels Jun 4, 2021

thaJeztah force-pushed the add_platform_info branch from 8404686 to 1ea093e Compare June 4, 2021 15:42

This comment has been minimized.

Sign in to view

thaJeztah force-pushed the add_platform_info branch from 1ea093e to fb4a195 Compare June 10, 2021 10:56

thaJeztah force-pushed the add_platform_info branch from fb4a195 to 76c8a60 Compare June 30, 2021 12:54

thaJeztah commented Jun 30, 2021

View reviewed changes

thaJeztah force-pushed the add_platform_info branch from 76c8a60 to 6d4ebbe Compare July 3, 2021 16:13

thaJeztah changed the title ~~WIP: API: add Plaform (OS and Architecture) to /containers/json~~ API: add Plaform (OS and Architecture) to /containers/json Jul 3, 2021

thaJeztah force-pushed the add_platform_info branch 3 times, most recently from f43ca2b to 7753a5a Compare July 4, 2021 14:11

thaJeztah marked this pull request as ready for review July 4, 2021 17:50

thaJeztah commented Jul 5, 2021

View reviewed changes

This was referenced Jul 5, 2021

[WIP] Add "platform" column to docker ps docker/cli#3177

Draft

API: update API version to v1.42 #42063

Merged

thaJeztah force-pushed the add_platform_info branch 3 times, most recently from 42d695b to 5c00664 Compare July 7, 2021 16:34

thaJeztah mentioned this pull request Jul 7, 2021

[WIP, DNM] try using oci.Platform where possible #42606

Draft

thaJeztah force-pushed the add_platform_info branch from 5c00664 to 635ce3c Compare July 10, 2021 10:28

thaJeztah mentioned this pull request Jul 12, 2021

Remove containerd "platform" dependency from client #42623

Merged

cpuguy83 requested changes Jul 21, 2021

View reviewed changes

integration/container/ps_test.go Outdated Show resolved Hide resolved

thaJeztah force-pushed the add_platform_info branch from 635ce3c to 0c6ef8c Compare July 23, 2021 10:08

thaJeztah added 3 commits July 23, 2021 12:10

daemon/container: normalize Platform with containerd.platforms

9432f56

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/types.ImagePlatform: implement String()

95b43fc

This allows the platform to be printed as a string. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the add_platform_info branch from 0c6ef8c to 95b43fc Compare July 23, 2021 10:10

cpuguy83 reviewed Jul 28, 2021

View reviewed changes

api/types/types.go Show resolved Hide resolved

thaJeztah mentioned this pull request Dec 4, 2022

UX: improve / design UX for multi-arch images #44582

Open

tianon approved these changes Apr 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: add Plaform (OS and Architecture) to /containers/json #42464

API: add Plaform (OS and Architecture) to /containers/json #42464

thaJeztah commented Jun 4, 2021 •

edited

This comment has been minimized.

thaJeztah commented Jun 30, 2021

thaJeztah Jun 30, 2021

tianon Jul 6, 2021

tianon Jul 6, 2021

cpuguy83 Jul 7, 2021

thaJeztah Jul 7, 2021

thaJeztah Jul 5, 2021

thaJeztah Jul 5, 2021

tianon Jul 6, 2021

thaJeztah Jul 7, 2021

tianon Jul 14, 2021

tianon Jul 14, 2021

thaJeztah Jul 15, 2021

tianon Jul 16, 2021

thaJeztah commented Jul 5, 2021

thaJeztah commented Jul 23, 2021

tianon left a comment

tianon Apr 26, 2024

API: add Plaform (OS and Architecture) to /containers/json #42464

Are you sure you want to change the base?

API: add Plaform (OS and Architecture) to /containers/json #42464

Conversation

thaJeztah commented Jun 4, 2021 • edited

This comment has been minimized.

thaJeztah commented Jun 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Jul 5, 2021

thaJeztah commented Jul 23, 2021

tianon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah commented Jun 4, 2021 •

edited