-
Notifications
You must be signed in to change notification settings - Fork 18.8k
c8d/inspect: Fill Container
property
#46931
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
Conversation
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>
Store the container ID the image was committed from in containerd image object label. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
9707e98
to
ad6b579
Compare
Do we use this property for anything? I'm leaning towards going the reverse direction, and officially deprecating this information. If I recall correctly, this is information that was either used by the classic builder, or was more a "side effect" of how the classic builder (which uses However, this information has been cause for confusion in many situations; inspecting an image shows multiple similar fields (e.g. |
No, we don't need it from the engine side. Not sure if |
I think there's quite some tests in docker-py testing "what they observed" at the time. Which is good because it also spotted breakage in some occasions (fields that were there but not strictly relevant for internal use). But I also think that the containerd integration gives us a good opportunity to re-evaluate some of those. We can consider some "defendable" changes in the API (directly related to legacy implementation; no longer relevant with containerd). Of course; within reason (i.e. don't blindly just rip-out important fields) I recall I already set the stage for that by adding some disclaimers that these fields may be empty, depending on how the image was built; Lines 1719 to 1728 in f3cc936
Lines 1742 to 1749 in f3cc936
But perhaps this is the moment to start making those official deprecations (which could mean: omitting the field entirely in a future version of the API) |
Also related to the above; |
Parent
property #46912- What I did
Store the container ID the image was committed from in containerd image object label.
This fixes
test_commit_with_changes
and partiallytest_commit
fromtests.integration.api_image_test.CommitTest
.- How I did it
- How to verify it
docker-py:
test_commit_with_changes
should pass now- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)