Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

c8d/list: Fix shared size calculation #47597

Merged
merged 3 commits into from Mar 20, 2024

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Mar 19, 2024

Fix diffIDs being outputted instead of chainIDs

The identity.ChainIDs call was accidentally removed in b37ced2.

This broke the shared size calculation for images with more than one layer that were sharing the same compressed layer.

Handle unpacked layers when calculating shared size

After a535a65 the size reported by the image list was changed to include all platforms of that image.

This made the "shared size" calculation consider all diff ids of all the platforms available in the image which caused "snapshot not found" errors when multiple images were sharing the same layer which wasn't
unpacked.

Found by @milas

- What I did

- How I did it

- How to verify it
This was could be reproduced with:

$ docker pull docker.io/docker/desktop-kubernetes-coredns:v1.11.1
$ docker pull docker.io/docker/desktop-kubernetes-etcd:3.5.10-0
$ docker system df

- Description for the changelog

rc3 regression: containerd image store: Fix `docker system df` failing with `snapshot ... does not exist` message when multiple images which share the same layers are present in the image store.

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

@vvoland vvoland added status/2-code-review area/images kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Mar 19, 2024
@vvoland vvoland added this to the 26.0.0 milestone Mar 19, 2024
@vvoland vvoland self-assigned this Mar 19, 2024
After a535a65 the size reported by the
image list was changed to include all platforms of that image.

This made the "shared size" calculation consider all diff ids of all the
platforms available in the image which caused "snapshot not found"
errors when multiple images were sharing the same layer which wasn't
unpacked.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The `identity.ChainIDs` call was accidentally removed in
b37ced2.

This broke the shared size calculation for images with more than one
layer that were sharing the same compressed layer.

This was could be reproduced with:
```
$ docker pull docker.io/docker/desktop-kubernetes-coredns:v1.11.1
$ docker pull docker.io/docker/desktop-kubernetes-etcd:3.5.10-0
$ docker system df
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vvoland vvoland changed the title c8d/list: Handle unpacked layers when calculating shared size c8d/list: Fix shared size calculation Mar 20, 2024
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@neersighted neersighted merged commit 963e1f3 into moby:master Mar 20, 2024
138 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/images containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants