API: add Platform (OS and Architecture) to /containers/json by jsternberg · Pull Request #49407 · moby/moby

jsternberg · 2025-02-06T20:50:40Z

Adds platform information to containers (for docker ps).

- What I did

Added the platform to the container.Summary struct so it could be returned in /containers/json.

- How I did it

Adapted this PR for the current master and upcoming api version: #42464

As mentioned in the original PR, there's still some follow up. I'm copying and pasting that message here.

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.

The last one seems to be a potentially big issue with the name. The Platform is used instead of the name OS, but Platform in that API is the same as the OS.

- How to verify it

docker run -d --name foo nginx:alpine

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

- Human readable description for the release notes

`GET /containers/json` now returns an `ImageManifestDescriptor` field matching the same field in `/containers/{name}/json`. This field is only populated if the daemon provides a multi-platform image store.

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

thaJeztah · 2025-02-06T20:58:22Z

Ah, right, I need to refresh my memory as I recall there was some back and forth on doing this and I think some considered that it was part of the image info, but it's a long time since I worked on #42464

thaJeztah · 2025-02-06T21:00:41Z

(i.e. ISTR there was some suggestion to include the image descriptor in the response or something along those lines ... but I need to dig really deep in memory 🤣)

jsternberg · 2025-02-06T21:16:41Z

I did need to update the original PR a bit, but it was fewer code changes rather than more. The daemon didn't require any change because the platform had been added as a parameter at some point so it was already present.

vvoland

Hmm, wondering if we should consider doing the same as we do for container inspect (/containers/.../json), where we return the full image manifest descriptor with the Platform field set.

See: #48855

jsternberg · 2025-02-10T15:21:02Z

@vvoland I'd say yes, but the Platform name is currently taken by the OS so it would be a breaking change. I'm not sure if there's a better name that can be used for it or how to resolve the breaking change.

vvoland · 2025-02-10T15:53:33Z

Yes, so that would actually solve that issue. If we exposed the whole image manifest descriptor like we did in container inspect:

moby/api/types/container/container.go

Lines 175 to 176 in 44ed306

    
           // ImageManifestDescriptor is the descriptor of a platform-specific manifest of the image used to create the container. 
        
           ImageManifestDescriptor *ocispec.Descriptor `json:",omitempty"`

the platform would also be a part of that descriptor:

moby/vendor/github.com/opencontainers/image-spec/specs-go/v1/descriptor.go

Lines 45 to 46 in e3feb05

    
           // This should only be used when referring to a manifest. 
        
           Platform *Platform `json:"platform,omitempty"`

moby/vendor/github.com/opencontainers/image-spec/specs-go/v1/descriptor.go

Lines 19 to 46 in e3feb05

    
           // Descriptor describes the disposition of targeted content. 
        
           // This structure provides `application/vnd.oci.descriptor.v1+json` mediatype 
        
           // when marshalled to JSON. 
        
           type Descriptor struct { 
        
           	// MediaType is the media type of the object this schema refers to. 
        
           	MediaType string `json:"mediaType"` 
        
           	// Digest is the digest of the targeted content. 
        
           	Digest digest.Digest `json:"digest"` 
        
           	// Size specifies the size in bytes of the blob. 
        
           	Size int64 `json:"size"` 
        
           	// URLs specifies a list of URLs from which this object MAY be downloaded 
        
           	URLs []string `json:"urls,omitempty"` 
        
           	// Annotations contains arbitrary metadata relating to the targeted content. 
        
           	Annotations map[string]string `json:"annotations,omitempty"` 
        
           	// Data is an embedding of the targeted content. This is encoded as a base64 
        
           	// string when marshalled to JSON (automatically, by encoding/json). If 
        
           	// present, Data can be used directly to avoid fetching the targeted content. 
        
           	Data []byte `json:"data,omitempty"` 
        
           	// Platform describes the platform which the image in the manifest runs on. 
        
           	// 
        
           	// This should only be used when referring to a manifest. 
        
           	Platform *Platform `json:"platform,omitempty"`

api/server/router/container/container_routes.go

vvoland · 2025-02-13T12:19:21Z

api/server/router/system/system_routes.go

+	if versions.LessThan(version, "1.48") {
+		// ImageManifestDescriptor information was added in API 1.48
+		for _, c := range systemDiskUsage.Containers {
+			c.ImageManifestDescriptor = nil
+		}
+	}


Wondering if we should include that in the system disk usage at all 🤔

Good point. ISTR that one was the horrible type that was reused "for convenience", but keeps popping up?

thaJeztah · 2025-02-13T12:28:07Z

api/types/container/container.go

+	Names                   []string
+	Image                   string
+	ImageID                 string
+	ImageManifestDescriptor *ocispec.Descriptor `json:",omitempty"`


oh! Before I blame myself; we've been very lax on specifying the intended casing of fields in JSON (depending on "whatever Go makes of it); this has resulted in inconsistencies where the Swagger documented the wrong casing (the Id one is a good example of that, where we had to later adjust the case to what it happened to be). Go itself treats JSON case-insensitive, but not all languages do (and ISTR Go is working on a "v2" JSON package to allow case-sensitivity).

Probably good to add an explicit json:"ImageManifestDescriptor,omitempty" (at least for new fields; we should also tackle existing fields at some point)

neersighted

LGTM with CI passing.

Adds platform information to containers (for `docker ps`). Signed-off-by: Jonathan A. Sternberg <[email protected]>

jsternberg · 2025-02-13T21:07:23Z

I've updated the test to only run when a multi-platform store is in use. I copied this from TestInspectImageManifestPlatform and I have also renamed the test function to match this format.

thaJeztah

LGTM

vvoland · 2025-02-14T10:03:35Z

container/view.go

 	snapshot.NetworkSettings = &container.NetworkSettingsSummary{Networks: networks}

+	if ctr.ImageManifest != nil && ctr.ImageManifest.Platform == nil {
+		ctr.ImageManifest.Platform = &ctr.ImagePlatform


ctr is supposed to be immutable, we shouldn't patch it in ctr directly.

This should be patched in snapshot.Summary

cc @jsternberg

jsternberg requested review from cpuguy83, thaJeztah and vvoland February 6, 2025 20:50

jsternberg added the impact/changelog label Feb 6, 2025

jsternberg mentioned this pull request Feb 6, 2025

formatter: include container image platform in container list response docker/cli#5804

Closed

vvoland reviewed Feb 10, 2025

View reviewed changes

vvoland added containerd-integration Issues and PRs related to containerd integration area/api API impact/api labels Feb 10, 2025

jsternberg force-pushed the containers-platform-json branch from c1ae171 to 626a3f0 Compare February 12, 2025 21:07

thaJeztah reviewed Feb 13, 2025

View reviewed changes

api/server/router/container/container_routes.go Show resolved Hide resolved

vvoland reviewed Feb 13, 2025

View reviewed changes

thaJeztah reviewed Feb 13, 2025

View reviewed changes

jsternberg force-pushed the containers-platform-json branch 3 times, most recently from 28dcca1 to 921ed17 Compare February 13, 2025 18:58

neersighted approved these changes Feb 13, 2025

View reviewed changes

API: add Platform (OS and Architecture) to /containers/json

927e07e

Adds platform information to containers (for `docker ps`). Signed-off-by: Jonathan A. Sternberg <[email protected]>

jsternberg force-pushed the containers-platform-json branch from 921ed17 to 927e07e Compare February 13, 2025 20:52

thaJeztah added this to the 28.0.0 milestone Feb 13, 2025

thaJeztah approved these changes Feb 13, 2025

View reviewed changes

thaJeztah merged commit 3b8eb1e into moby:master Feb 13, 2025
162 checks passed

jsternberg deleted the containers-platform-json branch February 14, 2025 00:44

vvoland reviewed Feb 14, 2025

View reviewed changes

jsternberg mentioned this pull request Feb 14, 2025

container: ensure image manifest is deep copied from the container into the snapshot #49469

Merged

vvoland mentioned this pull request Mar 3, 2025

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

Closed

vvoland linked an issue Mar 3, 2025 that may be closed by this pull request

Expose platform in docker ps #47764

Closed

Conversation

jsternberg commented Feb 6, 2025 • edited by vvoland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Feb 6, 2025

Uh oh!

thaJeztah commented Feb 6, 2025

Uh oh!

jsternberg commented Feb 6, 2025

Uh oh!

vvoland left a comment

Choose a reason for hiding this comment

Uh oh!

jsternberg commented Feb 10, 2025

Uh oh!

vvoland commented Feb 10, 2025

Uh oh!

Uh oh!

vvoland Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

thaJeztah Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

neersighted left a comment

Choose a reason for hiding this comment

Uh oh!

jsternberg commented Feb 13, 2025

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vvoland Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

vvoland Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsternberg commented Feb 6, 2025 •

edited by vvoland

Loading