formatter: include container image platform in container list response #5804
Conversation
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5804 +/- ##
===========================================
- Coverage 59.26% 44.44% -14.83%
===========================================
Files 353 6 -347
Lines 29540 261 -29279
===========================================
- Hits 17508 116 -17392
+ Misses 11052 139 -10913
+ Partials 980 6 -974 |
|
For what it's worth, I was looking at the output and I'm not sure platform should be included in the main template since I think it likely clutters the screen with more information. It's useful information and should be available for the full output format and for the templating but I'm not sure it should be included in the default |
|
Yeah I agree that this probably shouldn't be the default. I'd say that running containers with a non-native platform is not the common case, so that would just show extra information that's not relevant most of the time. |
|
I've been contemplating adding a With some of the badges added in |
|
I'm going to close this because I think there's an agreement that this information shouldn't be included by default. It will be available through the format option or the JSON endpoint when moby/moby#49407 is merged. |
| // Platform returns a human-readable representation of the platform | ||
| // of the container if it is available. | ||
| func (c *ContainerContext) Platform() *container.ImagePlatform { | ||
| if c.c.Platform != nil { | ||
| return c.c.Platform | ||
| } | ||
| return &container.ImagePlatform{} | ||
| } | ||
|
|
There was a problem hiding this comment.
I think we still need this so the {{.Platform}} format works?
There was a problem hiding this comment.
Ah, yes was wondering the same and wanted to test.
@jsternberg TL;DR is that the "format" options don't act on the raw response from the API but use separate structs / (poorly named "context") types purely for formatting.
There was a problem hiding this comment.
I figured we'd probably want to avoid {{.Platform}} since it has a different meaning for the inspect response. I figure, since the data is there, we can always add an alias later when we've got a better idea of what we want. I can open a new PR though to add this alone if we can come up with something that makes sense.
- What I did
Added platforms column to the
docker psoutput.- How I did it
Utilized the change in moby/moby#49407 to retrieve the platform and modified the default table output to include it.
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)