Fix image size calculation to show unpacked size#991
Fix image size calculation to show unpacked size#991Bortnyak wants to merge 18 commits intoapple:mainfrom
Conversation
|
Hey @dcantah, take a look at this PR please. |
|
@Bortnyak I'll try to review it next week. Thanks for the reminder, and sorry for the delay! Could you rebase it PR to resolve the conflicts? |
Bortnyak
force-pushed
the
fix-image-size
branch
from
6c979b4 to
50eccb4
Compare
|
Thanks @jglogan. |
|
@Bortnyak Sorry, still owe this a review, I haven’t forgotten |
There was a problem hiding this comment.
See the implementation in #862, if that moves into the image client and we merge that, you could use the same logic for the OCI image size, and then you could extend it so it instead returns a struct containing say, ociImageSize and snapshotSize.
There was a problem hiding this comment.
Thanks to both of you. I'll wait until #1098 is merged and will use that logic
There was a problem hiding this comment.
Hey @jglogan
I've added the change consolidating size retrieval into a single method that returns both ociImageSize and snapshotSize in an ImageSizeInfo struct, following your suggestion.
Check the PR, please.
Bortnyak
force-pushed
the
fix-image-size
branch
from
f1ba52f to
f9f1cec
Compare
…iImageSize and snapshotSize in an ImageSizeInfo struct Signed-off-by: Bortniak Volodymyr <Bortnyak@users.noreply.github.com>
Bortnyak
force-pushed
the
fix-image-size
branch
from
f9f1cec to
a005a13
Compare
| return response.uint64(key: .imageSize) | ||
| } | ||
|
|
||
| /// Returns both OCI image size and snapshot size for the specified platform. |
There was a problem hiding this comment.
Let's update this comment follow the docc format style. You can follow how it is done in the getFullImageSize function in this file.
| return fs | ||
| } | ||
|
|
||
| public func getSnapshotSize(platform: Platform) async throws -> UInt64 { |
There was a problem hiding this comment.
nit: function docc updates
| return try await self.snapshotStore.get(for: img, platform: platform) | ||
| } | ||
|
|
||
| public func getSnapshotSize(description: ImageDescription, platform: Platform) async throws -> UInt64 { |
There was a problem hiding this comment.
nit: function docc updates
|
The |
|
Are these the different types of image sizes that can be outputted?
I am thinking it might be worthwhile to output both the sizes in the |
4 participants
Type of Change
Motivation and Context
#709
Previously,
container image list -vonly showed compressed blob size. Now it shows:Snapshot size (unpacked) when available
Compressed blob size as fallback when image hasn't been run yet
This better reflects actual storage usage and matches Docker's behavior of showing unpacked size.
Tests use >= comparisons because filesystems allocate space in fixed blocks (4KB on macOS), making exact byte counts unreliable.
Testing
p.s.: if this doesn't match with the maintainers' vision of how image size should be represented, feel free to close this PR :)