Skip to content

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Dec 8, 2023

Before this change ParentId was filled for images when calling the /images/json (image list) endpoint but was not for the /images/<image>/json (image inspect).

This fixes part of tests.integration.api_image_test.CommitTest and tests.integration.api_image_test.CommitTest (not fully though, only the Parent value check).

- 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)

@vvoland vvoland added area/images Image Distribution kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Dec 8, 2023
@vvoland vvoland added this to the 25.0.0 milestone Dec 8, 2023
@vvoland vvoland self-assigned this Dec 8, 2023
@thaJeztah
Copy link
Member

Doh! This needs a rebase now

@vvoland vvoland force-pushed the c8d-inspect-parent branch 3 times, most recently from 4e65bcb to 146e571 Compare December 12, 2023 13:27
@vvoland vvoland requested a review from thaJeztah December 18, 2023 10:19
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

@vvoland
Copy link
Contributor Author

vvoland commented Dec 18, 2023

hmm some linting issue:

daemon/containerd/image.go:368: File is not `goimports`-ed (goimports)
	imgs, err := i.client.ImageService().List(ctx, "target.digest=="+target.String() + ",label." + labelKey)

@vvoland vvoland marked this pull request as draft December 18, 2023 15:22
Before this change `ParentId` was filled for images when calling the
`/images/json` (image list) endpoint but was not for the
`/images/<image>/json` (image inspect).

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland marked this pull request as ready for review December 18, 2023 15:23
@thaJeztah
Copy link
Member

LOL, race-condition in our comments; yup, just spotted that issue (was about to merge, but checked if only failures were in the containerd checks 😂)

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/images Image Distribution containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs status/4-merge
Projects
Development

Successfully merging this pull request may close these issues.

3 participants