containerd icon indicating copy to clipboard operation
containerd copied to clipboard

[draft] KEP 4216: Implement image pull with runtime class

Open kiashok opened this issue 1 year ago • 5 comments

This PR is a trimmed down version that implements the main logic needed to support image pull per runtime class. Intent of the PR is to get comments from containerd community and maintainers.

  • In K8s v1.29 CRI, a new field for RuntimeHandler was added in the ImageSpec struct

  • With these changes, an image in containerd will now be referenced as a tuple of (imageID/imageName, runtimeHandler) instead of just the imageName or imageID.

  • During CRIImageService and CRIService init(), a new field called runtimeHandlerToPlatformMap map[string]ocispec.Platform field is initialized to maintain a map of runtimeHandler to its ocispec.Platform field defined in the containerd.toml file

  • Image pull in containerd can happen via CRI or ctr.

    • Image pull through CRI:

      • CRIImageService.PullImage() checks if runtime handler was passed to it from kubelet. If its empty, default runtime handler is used. A new runtime handler label is added to the containerd.Image to indicate the runtime handler used to pull the image. This is particularly useful when Publish() event calls CRIService.UpdateImage() to update CRI's image store and containerd metadata db for images.
    • Image pull through ctr:

      • ctr has option to specify ocispec.Platform via "--platform" flag or specify "--all-platforms" to pull all manifests and layers of an image. A new "--runtime-handler" field is added to specify the runtime handler to use while pulling the image. This new field is compulsory to specify.
  • CRI image store is updated to reference an image as a tuple of imageID and runtimeHandler

  • Containerd reload: - Each image can now be referenced by multiple runtime handlers as there can be more than one runtime handler defined for the same platform. The newly added imageRuntimeHandler label will help to keep track of this. So when containerd is restarted, each image is unpacked for each of the runtime handlers that reference it.

  • CRI image delete:

    • On CRI RemoveImage() as well, runtime handler can be specified to remove reference of just that runtime handler. If nothing is specified explicitly, the default runtime handler is used. If there are no runtime handlers referencing an image only then the image is completely removed. Otherwise, only the label for this runtime handler is removed from containerd.Image.

kiashok avatar Jan 05 '24 21:01 kiashok

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

k8s-ci-robot avatar Jan 05 '24 21:01 k8s-ci-robot

@dcantah @dmcgowan @mikebrow @jsturtevant @marosset could you please take a look and let me know what you think about the changes please? This PR contains the main logic needed to implement image pull per runtime class. Based on the discussion that happened in the last containerd community meeting, submitting a draft PR with the main logic to help give an understanding of the approach taken. Once there is a general consensus on the approach, will break down the changes into smaller PRs with tests.

kiashok avatar Jan 05 '24 21:01 kiashok

@dmcgowan @mikebrow could you take a look when you have some time please? Thanks!

kiashok avatar Jan 17 '24 17:01 kiashok

@kiashok In the last refactor of cri images this could be much simpler now. We can scope this initial change to adding multi-platform support to pkg/cri/server/images and updating the necessary interfaces to resolve the image by runtime handler from the cri runtime (just like https://github.com/containerd/containerd/pull/9612/files#diff-e178ec8b2ff2601a9dfcbf13f38bc3c2ef0485af6585c4203ea35c539ab75745L108).

  • A configuration for runtime platform mapping was added which can be directly used https://github.com/containerd/containerd/blob/main/pkg/cri/config/config.go#L281 and https://github.com/containerd/containerd/blob/main/pkg/cri/server/images/service.go#L65. We shouldn't need to keep this runtime mapping anywhere else since CRI runtime handler -> platform resolution should be owned by the CRI image service.
  • The image reload/recover logic is fully inside the CRI image service https://github.com/containerd/containerd/blob/main/pkg/cri/server/images/check.go#L39. With this, we shouldn't need to track external labels, the reload can just use the platform mapping to ensure that images are reloaded for each snapshotter which has a configuration. I believe implementing this TODO will accomplish this.
  • The CRI image store/cache should just be able to be updated to store the images with the image platform without needing to know about runtime handler. Since this store/cache should only be used by the CRI image service, the image service could handle the resolution and lookup.

I think the initial change could be limited to the CRI updates and determine what is missing from there. The runtime handler is not a concept which needs to show up outside of pkg/cri if the plugins are properly decoupled. Likewise, no need to add support via ctr since the images will show up by their platform. By having the CRI image service own the platform resolution, the set of "images for a given runtime handler" can remain dynamic based on the configuration.

Also feel free to reach out to walk through some of this as well, there is a lot of links and the still some parts of the code which is not fully refactored.

dmcgowan avatar Jan 17 '24 20:01 dmcgowan

@kiashok In the last refactor of cri images this could be much simpler now. We can scope this initial change to adding multi-platform support to pkg/cri/server/images and updating the necessary interfaces to resolve the image by runtime handler from the cri runtime (just like https://github.com/containerd/containerd/pull/9612/files#diff-e178ec8b2ff2601a9dfcbf13f38bc3c2ef0485af6585c4203ea35c539ab75745L108).

  • A configuration for runtime platform mapping was added which can be directly used https://github.com/containerd/containerd/blob/main/pkg/cri/config/config.go#L281 and https://github.com/containerd/containerd/blob/main/pkg/cri/server/images/service.go#L65. We shouldn't need to keep this runtime mapping anywhere else since CRI runtime handler -> platform resolution should be owned by the CRI image service.
  • The image reload/recover logic is fully inside the CRI image service https://github.com/containerd/containerd/blob/main/pkg/cri/server/images/check.go#L39. With this, we shouldn't need to track external labels, the reload can just use the platform mapping to ensure that images are reloaded for each snapshotter which has a configuration. I believe implementing this TODO will accomplish this.
  • The CRI image store/cache should just be able to be updated to store the images with the image platform without needing to know about runtime handler. Since this store/cache should only be used by the CRI image service, the image service could handle the resolution and lookup.

I think the initial change could be limited to the CRI updates and determine what is missing from there. The runtime handler is not a concept which needs to show up outside of pkg/cri if the plugins are properly decoupled. Likewise, no need to add support via ctr since the images will show up by their platform. By having the CRI image service own the platform resolution, the set of "images for a given runtime handler" can remain dynamic based on the configuration.

Also feel free to reach out to walk through some of this as well, there is a lot of links and the still some parts of the code which is not fully refactored.

Thanks @dmcgowan . I will go through this today and revert with any questions. Really appreciate your help!

kiashok avatar Jan 17 '24 21:01 kiashok