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

Containerd image and snapshot backend integration #38738

Closed
wants to merge 67 commits into from

Conversation

dmcgowan
Copy link
Member

@dmcgowan dmcgowan commented Feb 14, 2019

Opening this up as a draft as there are many things left to do before this is mergeable.

Highlights

  • Image and reference store deprecated for containerd backend
  • ImageID can reference manifests, manifest lists, or image configs (allowing better support for multi-arch images)
  • Support for multiple unpacked storage backends (images point to their backend)
  • Multi-arch Image support

Major things left to do

  • Migration of existing image store data
  • Build code
  • Artifact creation on push and commit
  • Snapshot support as an option for images to point to
  • Many, many TODOs

Implements #38043

Does not include, both snapshotters and graphdriver interfaces will be supported in this PR

  • Graph drivers scoped down (no more mounting/diffing)
  • Graph drivers supported as containerd snapshotter plugins

Closes #26146
Closes #38043
Closes #33355
Closes docker/cli#122

@AkihiroSuda
Copy link
Member

Is this planned to be merged before Docker v19.03?

@dmcgowan
Copy link
Member Author

Is this planned to be merged before Docker v19.03?

That is desirable but unrealistic. I want to complete as many or all of the major things by that time frame, but until all the functionality is complete it is too hard to pin down an exact date for "stable enough to merge".

It is very important we get the design parts mentioned in "highlights" correct so we aren't in a position where we need to correct storage code later (adding more support permutations and instability). Please comment on the design and help wherever possible will certainly help stabilize this change.

const (
// LabelImageID refers to the image ID used by Docker
// Deprecate this to support multi-arch images
LabelImageID = "docker.io/image.id"
Copy link
Member

Choose a reason for hiding this comment

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

nit: mobyproject.org? docker.com?

Copy link
Member

Choose a reason for hiding this comment

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

If these can conflict with user-provided labels; reverse-dns notation was originally marked as reserved; https://docs.docker.com/config/labels-custom-metadata/#key-format-recommendations

The com.docker.*, io.docker.*, and org.dockerproject.* namespaces are reserved by Docker for internal use.

@cpuguy83
Copy link
Member

cpuguy83 commented Mar 2, 2019

Pushed a rebase.

@dmcgowan dmcgowan force-pushed the containerd-integration branch 4 times, most recently from 3018ba4 to a2ee367 Compare March 5, 2019 00:54
@cpuguy83 cpuguy83 force-pushed the containerd-integration branch from ee7d1b3 to 1888615 Compare March 6, 2019 21:37
@dmcgowan dmcgowan force-pushed the containerd-integration branch from 1888615 to b7480ac Compare March 11, 2019 21:56
@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 13, 2019
@dmcgowan dmcgowan force-pushed the containerd-integration branch from b7db406 to f3dacb4 Compare March 13, 2019 00:33
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 13, 2019
@moby moby deleted a comment from GordonTheTurtle Mar 13, 2019
@dmcgowan dmcgowan force-pushed the containerd-integration branch from f3dacb4 to 9d443a0 Compare March 13, 2019 18:10
@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 14, 2019
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "containerd-integration" git@github.com:dmcgowan/docker.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842359062744
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

AntaresS and others added 10 commits October 9, 2019 13:28
fix already exists error during image tagging

Signed-off-by: Anda Xu <axu@knox.edu>
Remove image store instantiation from image service

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Move logic to image service and merge with cache loading.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Replace use of image id with descriptor

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Update load to use full image name annotation

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Prevent panics on unimplemented code and remove
unnecessary lookups on commit.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@dmcgowan dmcgowan force-pushed the containerd-integration branch from 2bb3716 to 30fb3cb Compare October 9, 2019 20:28
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@carlosedp
Copy link
Contributor

Any news about this? It's pretty important for local development with buildx avoiding problems like docker/buildx#59.
Thanks!

@CameronMcWilliam
Copy link

Also interested to see if this has made any progress recently! :)

@t0rr3sp3dr0
Copy link

@dmcgowan do you plan to continue developing this?

@milahu
Copy link

milahu commented Oct 11, 2021

so, after so many years of back-and-forth with this trivial patch, docker is still not OCI compliant?

$ docker load < some-oci-image.tar
open /var/lib/docker/tmp/docker-import-xxxxxxxxxx/blobs/json: no such file or directory

$ id=$(docker import some-oci-image.tar)
$ docker run $id
docker: Error response from daemon: No command specified.

with podman, this just works as expected

$ id=$(podman load < some-oci-image.tar)
$ podman run $id
hello world

image layout

$ tar tf some-oci-image.tar
./
blobs/
blobs/sha256/
blobs/sha256/12717fff989356b696302292db210ba0f182f9de6f21583c8eeebae1fcfc74b5
blobs/sha256/241c7fb8a84d580b7c8e1ccc489f7f55c1dcac98a7f536be158e2248d9851d80
blobs/sha256/6767daaf3d46fc9e5cb15a70ca9037ab7b077132a5c26d4c1299573867ee1b9a
blobs/sha256/e2eb06d8af8218cfec8210147357a68b7e13f7c485b991c288c2d01dc228bb68
index.json

@tianon
Copy link
Member

tianon commented Oct 11, 2021

There's definitely nothing trivial about this patch? 😕

+10,091 −1,817

This isn't just about support for loading OCI image bundles, but rather switching the image management backend over to containerd, which is a very complicated transition.

@milahu
Copy link

milahu commented Oct 11, 2021

oh, i see. im still surprised, cos docker load < oci.tar would be trivial to implement (no need for containerd), since the only difference between oci and docker are the file locations, e.g. layerhash.tar vs blobs/sha256/layerhash (please tell me im terribly wrong ^^). imho oci is better, cos sha256sum is much faster than tarsum

@tianon
Copy link
Member

tianon commented Oct 11, 2021

I don't think you're wrong, but this is not the right place to discuss it 😅

(A new PR would be very interesting 👀)

Edit: specifically, the appropriate issue appears to be #25779 🙈

@smthpickboy
Copy link

Any progress on this? Storing the images twice is such a waste.

@RoyHP
Copy link

RoyHP commented Jan 4, 2022

This would be great to see, especially now with Apple Silicon creating a barrier to entry for people trying to run amd64 containers in the cloud but arm64 locally on their machines.

@dmcgowan
Copy link
Member Author

dmcgowan commented Jan 5, 2022

I'm going to close this one since I don't want anyone to get the impression I am still working on it. The branches live on in my fork if anyone wants to play around with it. Reaching full behavior parity and just keeping this branch updated is non-trivial.

@pkit
Copy link

pkit commented Sep 21, 2022

@tianon

I don't think you're wrong, but this is not the right place to discuss it sweat_smile

I'm not sure how come it's not the right place to discuss it?
This issue is a direct descendant of the original issue #25779 (opened in 2016!) that wanted exactly that: docker load < oci.tar
But okay, let's reopen #25779 then.

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