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

Use containerd importer #53

Open
wants to merge 1 commit into
base: containerd-integration
Choose a base branch
from
Open

Use containerd importer #53

wants to merge 1 commit into from

Conversation

SamWhited
Copy link
Collaborator

Fixes #48

Signed-off-by: Sam Whited sam@samwhited.com

@SamWhited SamWhited changed the title WIP: Use containerd importer Use containerd importer Oct 1, 2019
@SamWhited SamWhited marked this pull request as ready for review October 1, 2019 21:28
}
defer inflatedLayerData.Close()

desc, err := archive.ImportIndex(ctx, i.client.ContentStore(), rc)
Copy link
Owner

Choose a reason for hiding this comment

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

For this function is just importing a single layer. It should still register to the layer store for now. The creation of the image artifact will be different, but I think the flow will be

  1. register layer with layer store (we can add snapshotter support later), tee output into content store (if not compressed, compress)
  2. start containerd lease and create context
  3. create image config and put in content store, add label to point to registered layer
  4. create manifest from image config and created layer (add gc labels pointing to those artifacts)
  5. create image tag
  6. release lease

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe I'm misunderstanding what import index does, but that only partially made sense to me. What is the difference between the index, a layer, and the content? I haven't seen anything about a "containerd lease" either, what is that? I think the differences between docker and containerd are still confusing me.

Copy link
Owner

@dmcgowan dmcgowan Oct 3, 2019

Choose a reason for hiding this comment

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

When you import an "index", you are importing a tar in this format (notice how this format contains everything as far as metadata is concerned as well as all the layers) https://github.com/opencontainers/image-spec/blob/master/image-layout.md

When you import a "layer", you are importing a tar in this format https://github.com/opencontainers/image-spec/blob/master/layer.md

This is the main difference, but handling those is much different. In the layer case you are basically just given a container filesystem with no other metadata in the tar stream. The other metadata has to be provided separately (in this case in a ton of arguments). Feel free to change the interface to ImportImage to make more sense too, it should be called in a limited number of places.

Copy link
Owner

@dmcgowan dmcgowan Oct 3, 2019

Choose a reason for hiding this comment

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

I haven't seen anything about a "containerd lease" either, what is that? I think the differences between docker and containerd are still confusing me.

The containerd interfaces are much lower level than Dockers. In containerd there are two types of "root" objects, images and containers. These root images will link to a set of resources underneath them, such as content blobs or snapshots. Resource management in containerd is strict, meaning any object which is not reference by a root is eligible for garbage collection. Leases can act as a temporary root to reference objects to prevent the garbage collector from removing them before you create your final root object. For example, when creating an image, you will get the content first (whether from a registry or in this case directly from an io.Reader), then you will create an image which points to that content. The lease is created before any content is added, then it can ride the context to ensure any objects created with that context are leased. When the operation is finished, whether successful or not, that lease is removed and any leftover resources are cleaned up. The client then only has to worry about managing a single lease rather than tracking all the resources it may have used or left unused. NOTE: the leases can also have an expiry to handle cases where the client crashes and never comes back or may want to re-attempt the operation before expiration

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hadn't found the docs here; this is great, thanks!

@dmcgowan
Copy link
Owner

dmcgowan commented Oct 3, 2019

I think it is worth considering changing the name of ImportImage as it is very confusing. Image and layers used to be the same thing, this is a left over from that. Really this is creating a new image from a layer. So it takes in a layer stream and image metadata.

@SamWhited
Copy link
Collaborator Author

I see at least three different types of "Image" in containerd, and have found them used in different places in other parts of this package. What's the difference between containerd.Image, images.Image, and image.Image and which do you mean when you say to create an image config?

They all appear to have the same documentation (more or less) and the first two don't seem to implement the interface that's the last one.

@dmcgowan
Copy link
Owner

dmcgowan commented Oct 4, 2019

I did a rebase on the underlying branch if you want to update.

As for the different image types. The image.Image from github.com/docker/docker/image is deprecated and should not longer be used (that whole package). In contained, containerd.Image is an image representing a specific platform and has some helper functions to perform operations in context of all the services. images.Image on the other hand is primitive which mirrors the API and what gets stored in an image store. It has no context around it, it doesn't even necessarily need to present a container image.

@dmcgowan dmcgowan force-pushed the containerd-integration branch 2 times, most recently from 2bb3716 to 30fb3cb Compare October 9, 2019 20:28
Fixes #48

Signed-off-by: Sam Whited <sam@samwhited.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants