-
Notifications
You must be signed in to change notification settings - Fork 22
Fix annotations for containerd #174
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
base: main
Are you sure you want to change the base?
Fix annotations for containerd #174
Conversation
Signed-off-by: Anton Ippolitov <[email protected]>
0a0843b to
a5ba66a
Compare
|
Interesting. Thanks for the PR. I was using this script https://github.com/containerd/containerd/blob/2bc9bdbcc0d0337772ff6cde0198bb60ba381ad3/contrib/checkpoint/checkpoint-restore-cri-test.sh while developing the code for containerd. Do you know why I don't need the manifest for my test code? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #174 +/- ##
==========================================
- Coverage 74.14% 73.55% -0.59%
==========================================
Files 13 13
Lines 1257 1267 +10
==========================================
Hits 932 932
- Misses 251 261 +10
Partials 74 74 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hmm I didn't know this script existed 🤔 I tried only the buildah / podman commands and I noticed that |
|
Ah, so your are exporting a podman container and trying to import it into containerd via CRI? I am pretty sure nobody has tried that and it will not work. The images are using different metadata all over the place. It should be possible to convert it, but it need to be done at many more locations. |
We briefly discussed this functionality in the following issue: #130 |
Not at all, all my tests were done with regular runc containers and containerd 2.1. I only mentioned podman because it was in the script you shared. Maybe you misunderstood "I tried only the buildah / podman commands" -> I was referring to the script. I just ran these commands from the script and that script uses podman |
Description
While trying out checkpoint restoration with
checkpointctlandcontainerd, I found a bug which is due to the fact thatcheckpointctldoesn't write the checkpoint annotations wherecontainerdis looking for them.In order to confirm that an image is a checkpoint,
containerdis looking for OCI Index (application/vnd.oci.image.index.v1+json) annotations:https://github.com/containerd/containerd/blob/2bc9bdbcc0d0337772ff6cde0198bb60ba381ad3/internal/cri/server/container_checkpoint_linux.go#L100
However,
checkpointctlwas writing image manifest (application/vnd.oci.image.manifest.v1+json) annotations instead:checkpointctl/internal/oci_image_build.go
Line 74 in fd18316
So checkpoint images were not recognized by
containerd. Example:The image has manifest annotations:
And there is no image index (== manifest list) (I think the terminology is a bit confusing here btw 🫤):
After my fix, the images still have the same annotations as before (for backwards compatibility):
And there are now manifest list annotations to satisfy containerd:
Notes
containerdas well, I don't feel strongly about either waybuildah>= 1.35 because themanifest annotate --indexflag was added in commit containers/buildah@aca884a. I tested withbuildah1.42 on my machine.