c8d: align "Size" and "VirtualSize" for images#45347
Conversation
| // TODO(thaJeztah): include content-store size for the image (similar to "GET /images/json") | ||
| return usage.Size, usage.Size + snapShotSize, nil |
There was a problem hiding this comment.
wasn't 100% sure here, but given that a container is "image size" + "writable layer size", I think we're missing a size here in the total (if I'm wrong, I'll remove this TODO 😂)
There was a problem hiding this comment.
I think it makes sense to NOT include the content-store size. Once created, snapshot exists independently from the content. We could even delete the content and the snapshot will not be affected.
There was a problem hiding this comment.
Hmm.. right, yeah. Thinking how the parallel is with pre-snapshottee; I guess in that case we have mounts for the container, and I think we also allow the image to be deleted if there's still a container using it (in which case those layers basically get "re-parented" and now are only used by the container, so will be garbage-collected once the container is gone.
/cc @rumpl @tianon @tonistiigi any thoughts what makes most sense to include?
There was a problem hiding this comment.
As I noted in the meeting today, I'd personally be surprised to find that the docker image ls listed size doesn't include the layers still in the content store as well as the extracted content in the snapshotter. Just because we can remove the image from the content store when it's extracted doesn't mean we would or even should, and IMO the default top-level size field should absolutely reflect all parts of "this image taking up space" including any content and all snapshotters (ideally).
It'd be great if we could dial into that somehow (maybe a separate struct in the API response that includes the individual details of the content store and each snapshotter/snapshot size?), but I don't think that's critical (especially given we can mostly query that from containerd itself outside of Docker if absolutely necessary).
There was a problem hiding this comment.
This affects docker container ls (size of the container), not docker image ls :).
There was a problem hiding this comment.
- It's somewhat orthogonal to this PR (just something I noticed while working on it), so I created a tracking ticket; UX: containerd integration: discuss what to include in "container" size #45351
There was a problem hiding this comment.
Ahhhh, the "xxxB (virtual yyyB)" value, hmm - so that should probably be "xxx" is the size of the RW layer and "yyy" is that plus whatever value docker image ls shows, right? (it'd be weird if docker image ls tells me 20MiB, then a container tells me only 10MiB or something)
tianon
left a comment
There was a problem hiding this comment.
It's not ideal, but the difference between VirtualSize and Size has long since been lost, I think (they've been the same in every case I've looked closely at for years now), so I think this is good. 😄
In versions of Docker before v1.10, this field was calculated from the image itself and all of its parent images. Images are now stored self-contained, and no longer use a parent-chain, making this field an equivalent of the Size field. For the containerd integration, the Size should be the sum of the image's compressed / packaged and unpacked (snapshots) layers. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
f89c5ce to
e798027
Compare
|
Here's an example to illustrate why I think including the snapshotter size is really important: (the short version of this is that the compressed content of / # docker version
Client:
Version:
API version: 1.43
Go version: go1.19.8
Git commit: 67c4570f280ca3c4d40ca69d8686370784bebc04
Built: Fri Apr 14 20:32:07 2023
OS/Arch: linux/amd64
Context: default
Server:
Engine:
Version: dev
API version: 1.43 (minimum version 1.12)
Go version: go1.19.8
Git commit: ad2ab4927cb1f95b3767c00f95c9696a5975dd0a
Built: Mon Apr 17 20:33:14 2023
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: v1.7.0
GitCommit: 1fbd70374134b891f97ce19c70b6e50c7b9f4e0d
runc:
Version: 1.1.5
GitCommit: v1.1.5-0-gf19387a6
docker-init:
Version: 0.19.0
GitCommit: de40ad0
/ # du -hsx /var/lib/docker/containerd/
272.0K /var/lib/docker/containerd/
/ # ctr content fetch docker.io/library/bash:latest
docker.io/library/bash:latest: resolved |++++++++++++++++++++++++++++++++++++++|
index-sha256:e0acf0b8fb59c01b6a2b66de360c86bcad5c3cd114db325155970e6bab9663a0: done |++++++++++++++++++++++++++++++++++++++|
manifest-sha256:565db8a061f00b6ae93dcfea37faab27b880f644c31e2c0b57e65b94fe84668f: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:f56be85fc22e46face30e2c3de3f7fe7c15f8fd7c4e5add29d7f64b87abdaa09: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:1eb2595706a1249a9fd53b5e09ca99a98e70bd497edc6f2e34683ba47886b702: done |++++++++++++++++++++++++++++++++++++++|
config-sha256:f5c1538e3a18fe20ee8e917e749202f36016df07092278163876a5bf885654d6: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:4abd0b518e7b49a663bbe01b70ae16047d3fc7c0f6baa7841c94b6920e005aa1: done |++++++++++++++++++++++++++++++++++++++|
elapsed: 2.3 s total: 4.6 Mi (2.0 MiB/s)
/ # du -hsx /var/lib/docker/containerd/
6.1M /var/lib/docker/containerd/
/ # docker images
REPOSITORY TAG IMAGE ID CREATED SIZE
bash latest e0acf0b8fb59 8 seconds ago 6.13MB
/ # ctr i pull docker.io/library/bash:latest
docker.io/library/bash:latest: resolved |++++++++++++++++++++++++++++++++++++++|
index-sha256:e0acf0b8fb59c01b6a2b66de360c86bcad5c3cd114db325155970e6bab9663a0: done |++++++++++++++++++++++++++++++++++++++|
manifest-sha256:565db8a061f00b6ae93dcfea37faab27b880f644c31e2c0b57e65b94fe84668f: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:1eb2595706a1249a9fd53b5e09ca99a98e70bd497edc6f2e34683ba47886b702: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:f56be85fc22e46face30e2c3de3f7fe7c15f8fd7c4e5add29d7f64b87abdaa09: done |++++++++++++++++++++++++++++++++++++++|
layer-sha256:4abd0b518e7b49a663bbe01b70ae16047d3fc7c0f6baa7841c94b6920e005aa1: done |++++++++++++++++++++++++++++++++++++++|
config-sha256:f5c1538e3a18fe20ee8e917e749202f36016df07092278163876a5bf885654d6: done |++++++++++++++++++++++++++++++++++++++|
elapsed: 0.7 s total: 0.0 B (0.0 B/s)
unpacking linux/amd64 sha256:e0acf0b8fb59c01b6a2b66de360c86bcad5c3cd114db325155970e6bab9663a0...
done: 147.279164ms
/ # docker images
REPOSITORY TAG IMAGE ID CREATED SIZE
bash latest e0acf0b8fb59 20 seconds ago 6.13MB
/ # du -hsx /var/lib/docker/containerd/
20.4M /var/lib/docker/containerd/(graphdriver-based Docker lists ~13.7MiB in |
In versions of Docker before v1.10, this field was calculated from the image itself and all of its parent images. Images are now stored self-contained, and no longer use a parent-chain, making this field an equivalent of the Size field.
For the containerd integration, the Size should be the sum of the image's compressed / packaged and unpacked (snapshots) layers.
- 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)