Skip to content

c8d/list: Don't require opts.ContainerCount for manifest containers#48345

Merged
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-manifests-containers-always
Aug 16, 2024
Merged

c8d/list: Don't require opts.ContainerCount for manifest containers#48345
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-manifests-containers-always

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 16, 2024

The GET /images/json requires an optional container-count parameter which set the Containers property of in the ImageSummary to a number of containers using that image.

This was also propagated to the new manifest list property which includes a list of all the container IDs that are using this specific image manifest.

Disconnect the ImageData.Containers property from this option and always include it by default without an explicit opt-in.

- What I did

- How I did it

- How to verify it

- Description for the changelog

#47526 wasn't released yet so no changelog needed.

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

The `GET /images/json` requires an optional `container-count` parameter
which set the `Containers` property of in the ImageSummary to a number
of containers using that image.

This was also propagated to the new manifest list property which
includes a list of all the container IDs that are using this specific
image manifest.

Disconnect the `ImageData.Containers` property from this option and
always include it by default without an explicit opt-in.

Signed-off-by: Paweł Gronowski <[email protected]>
@vvoland vvoland added area/api API status/2-code-review area/images Image Distribution containerd-integration Issues and PRs related to containerd integration labels Aug 16, 2024
@vvoland vvoland added this to the 27.2.0 milestone Aug 16, 2024
@vvoland vvoland self-assigned this Aug 16, 2024
@vvoland vvoland modified the milestones: 27.2.0, 28.0.0 Aug 16, 2024
@thaJeztah
Copy link
Member

LOL, looks like GHA runners ran out of space. Hope it's not a change in the default size for those.

=== RUN   TestBuildWithHugeFile
    build_test.go:530: assertion failed: string "{\"stream\":\"Step 1/2 : FROM busybox\"}\r\n{\"stream\":\"\\n\"}\r\n{\"stream\":\" ---\\u003e 328dac05c07e\\n\"}\r\n{\"stream\":\"Step 2/2 : RUN for g in $(seq 0 8); do dd if=/dev/urandom of=rnd bs=1K count=1 seek=$((1024*1024*g)) status=none; done  \\u0026\\u0026 ls -la rnd \\u0026\\u0026 du -sk rnd\"}\r\n{\"stream\":\"\\n\"}\r\n{\"stream\":\" ---\\u003e Running in 2dbdda45f69f\\n\"}\r\n{\"stream\":\"-rw-r--r--    1 root     root     8589935616 Aug 16 11:56 rnd\\n\"}\r\n{\"stream\":\"36\\trnd\\n\"}\r\n{\"stream\":\" ---\\u003e Removed intermediate container 2dbdda45f69f\\n\"}\r\n{\"errorDetail\":{\"message\":\"failed to apply diff: write /var/lib/docker/containerd/daemon/io.containerd.snapshotter.v1.overlayfs/snapshots/192/fs/rnd: no space left on device: unknown\"},\"error\":\"failed to apply diff: write /var/lib/docker/containerd/daemon/io.containerd.snapshotter.v1.overlayfs/snapshots/192/fs/rnd: no space left on device: unknown\"}\r\n" does not contain "Successfully built"

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api API area/images Image Distribution containerd-integration Issues and PRs related to containerd integration process/cherry-picked status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants