Don't append the container id to custom directory checkpoints. #35694
Conversation
…oby#34601. Signed-off-by: Ross Boucher <rboucher@gmail.com>
|
Two obvious options are:
|
|
power failure seems unrelated but I'm looking at it |
thaJeztah
added
kind/experimental
status/2-code-review
and removed
status/0-triage
labels
|
Oups, it's actually doable if a little convoluted to support from external directory. One would need to create the needed snapshot/manifest into containerd then provide that to the runtime when restoring. I thought I had done so, the commit must have been lost during one of my rebase 😓 |
|
@mlaventure I might have some time coming up to help fix the C/R from a custom directory use case. How large in scope is the work to address the TODO you mentioned? I have not yet contributed to docker/containerd, but I could take a crack at it if I got some pointers. |
|
@tswift242 should be pretty straightforward, the code to do so is here: https://github.com/containerd/containerd/blob/967caeeaccac497c6708d84f6c3c0c8205bda481/services/tasks/service.go#L439-L456 (minus the call to checkpoint). It creates a snapshot in containerd and the resulting reference can be used as a checkpoint |
|
Can we get this PR merged? Is anything blocking? Also, is there any chance this could be backported to 17.09.x? I believe that's before the containerd 1.0 integration, and so shouldn't have the new bug @boucher mentioned. I haven't had time yet to take a look at the containerd code related to the other issue. @boucher do you have time to look at the other issue? And should we create a separate github issue for tracking? |
- What I did
Issue #34601 documents a change in the way the
--checkpoint-dirflag works, breaking the behavior for people using it. This fixes that issue.- How I did it
Simply don't append the container id to the checkpoint path.
- How to verify it
You can verify by building and creating a custom dir checkpoint, and seeing that the resulting folder isn't nested inside one with the container's id.
Unfortunately, implementing this fix highlighted a new problem:
ddae20c032#diff-3cb140026df40998ea29c5bcb6bb292eR118
As of the above commit, restoring from a custom directory is no longer supported anyway. So, I'm not really sure what to do. If a subsequent release of pre containerd 1.0 docker is going to be created it would be nice to get this fix in I think.