Skip to content

[draft] containerd integration: add support for image inspect #44621

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.

Sign up for GitHub

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

Jump to bottom
Closed
thaJeztah wants to merge 11 commits into from
Closed

[draft] containerd integration: add support for image inspect #44621

thaJeztah wants to merge 11 commits into from

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Dec 9, 2022

This upstreams a set of (partial) patches:

These patches were selected by looking at the history for https://github.com/rumpl/moby/commits/dd-4.15.0/daemon/containerd/image.go, and only the relevant parts for this feature were included.

Quite a few of those are touching-up previous commits, so we should consider squashing (some of) them, but keeping them separate for now to somewhat help review.

p 85d6399836 add support for image inspect
p fa430d6686 Implement run using the containerd snapshotter
p d37b797d34 introduce GetImageOpts to manage image inspect data in backend
p 4543499f86 Introduce support for docker commit
p f1213b08bd Fix linting issues
p 2840f63ee6 GetImage to return image tags with details
p 75df7b7a05 list images matching digest to discover all tags
p d9b76c6682 c8d/prune: Handle filters, don't delete used
p a930139556 Add ExposedPorts and Volumes to the image returned
p 8df0a88265 Refactor resolving/getting images
p d58184802b Return the image ID on inspect
p 61ea993343 consider digest and ignore tag when both are set
p eaa810ab98 docker run --platform
p ab6918735d c8d/builder: store untagged images
p 960cff2c71 c8d/commit: Save as dangling name
p f7962b37ef Remove the call to resolveImage
p 9ac5d768c7 Handle multi-platform images when listing them

Also included rumpl#122

p a5a01b5cf7 daemon/containerd: fix compat for "docker run" with image that's not present

@thaJeztah thaJeztah mentioned this pull request Dec 9, 2022
Merged
@thaJeztah thaJeztah force-pushed the image_inspect_step2 branch from e60adc3 to 73eef4e Compare December 9, 2022 18:24
@thaJeztah thaJeztah mentioned this pull request Dec 9, 2022
Merged
@thaJeztah thaJeztah force-pushed the image_inspect_step2 branch 3 times, most recently from 63bbb2c to ac08e60 Compare December 12, 2022 19:22
vvoland
vvoland approved these changes Dec 13, 2022
View reviewed changes
Copy link
Contributor

@vvoland vvoland left a comment

Choose a reason for hiding this comment

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

Is there something else that needs to be done for this one?

rumpl
rumpl reviewed Dec 13, 2022
View reviewed changes
daemon/containerd/image.go
User: ociimage.Config.User,
WorkingDir: ociimage.Config.WorkingDir,
ExposedPorts: exposedPorts,
Volumes: ociimage.Config.Volumes,
Copy link
Member

@rumpl rumpl Dec 13, 2022

Choose a reason for hiding this comment

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

You could add this one too rumpl#121

Copy link
Member Author

@thaJeztah thaJeztah Dec 15, 2022

Choose a reason for hiding this comment

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

Ah, I have that one in a branch for a follow-up, in case we still need a discussion (didn't want that discussion to block these changes).

@thaJeztah
Copy link
Member Author

thaJeztah commented Dec 15, 2022

Is there something else that needs to be done for this one?

@vvoland I think the only thing left (from my side) is to cleanup the commits / squash things; but code changes should already be good for review, so I'll have a look at doing so.

@thaJeztah thaJeztah added status/2-code-review containerd-integration Issues and PRs related to containerd integration labels Dec 15, 2022
ndeloof and others added 11 commits December 20, 2022 16:54
@ndeloof @thaJeztah
add support for image inspect
14be635 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
daemon/containerd: fix compat for "docker run" with image that's not …
f29c0bf 
…present

It's horrible, but some clients (such as the docker 17.03 CLI used in CI) still
perform string-matching to detect that the *image* wasn't found. This requires
the error to be prefixed with "No such image:". Without this prefix, the CLI
prints the error message, and exits.

Before this:

    docker run -it --rm hello-world
    docker: Error response from daemon: id not found.
    See 'docker run --help'.

After this:

    docker run -it --rm hello-world
    Unable to find image 'hello-world:latest' locally
    faa03e786c97: Download complete
    ...

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@ndeloof @thaJeztah
introduce GetImageOpts to manage image inspect data in backend
fcd321d 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@ndeloof @thaJeztah
GetImage to return image tags with details
a25cb89 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@ndeloof @thaJeztah
list images matching digest to discover all tags
d3e1044 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@rumpl @thaJeztah
Add ExposedPorts and Volumes to the image returned
97ea5bb 
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@rumpl @thaJeztah
Refactor resolving/getting images
f3e4e78 
There were a lot of useless duplicated code

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@rumpl @thaJeztah
Return the image ID on inspect
afecfb8 
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@ndeloof @thaJeztah
consider digest and ignore tag when both are set
02eee91 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof @thaJeztah
docker run --platform
38ec9df 
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
daemon/containerd: minor cleanup (to be squashed)
b642809 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

thaJeztah commented Jan 2, 2023

closing this one; superseded by #43818

@thaJeztah thaJeztah closed this Jan 2, 2023
@thaJeztah thaJeztah deleted the image_inspect_step2 branch October 28, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rumpl rumpl rumpl approved these changes

@vvoland vvoland vvoland approved these changes

Assignees

No one assigned

Labels

containerd-integration Issues and PRs related to containerd integration status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @rumpl @vvoland @ndeloof