Initial checkpoint support #641

adrianreber · 2022-01-24T17:44:29Z

This is the initial draft for checkpoint support. No restore support yet.

Tests are still missing and it relies on the not yet officially release Rust CRIU bindings. For now it is just a draft.

It depends on #632

I called the checkpoint command ´checkpointt' with two ts so that Podman does not think youki already fully supports checkpoint/restore. Once everything works I will rename it.

codecov-commenter · 2022-01-25T17:13:49Z

Codecov Report

Merging #641 (7ae3c3c) into main (1b810d4) will decrease coverage by 0.48%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #641      +/-   ##
==========================================
- Coverage   69.69%   69.21%   -0.49%     
==========================================
  Files          84       85       +1     
  Lines       10983    11059      +76     
==========================================
- Hits         7655     7654       -1     
- Misses       3328     3405      +77

adrianreber · 2022-01-25T18:30:40Z

Hmm, the test succeeds locally with runc. Need to figure out why it does not work in CI.

adrianreber · 2022-01-26T10:11:45Z

Tests are still failing as I only tested on a cgroup v2 system. v1 needs some additional code.

rst0git · 2022-01-26T22:47:58Z

crates/libcontainer/src/container/container_checkpoint.rs

+        }
+
+        let directory = std::fs::File::open(&opts.image_path)?;
+        criu.set_images_dir_fd(directory.as_raw_fd());


Do we need to create the directory specified with image_path?

https://github.com/opencontainers/runc/blob/0c0ec3f53f92c01866449916d554b62962cda6ab/checkpoint.go#L86

Do we need to create the directory?

We do not need to create it, but we could if it does not exist. I think in Podman we create it to make sure it gets created with 0600. We could do it here. Or not. I have not strong opinion. If it does not exist the user will see an error. By creating the directory we could avoid that error. On the other hand, if it does not exist then the user has to make a conscious decision where the checkpoint directory is. I have a couple of checkpoint directories all around my development system because the directory is automatically created. Both options have advantages and disadvantages.

We could leave it for now and if users are complaining then it can still be added later. Or if one of the container engines needs it, it can also be added later.

adrianreber · 2022-02-14T16:36:59Z

Ready for review. All tests are successful.

utam0k · 2022-02-15T12:33:11Z

Thanks for your PR. But I don't have the time to review this PR on my weekday. So I'll check on next holiday, please wait just a moment 🙏

adrianreber · 2022-02-15T18:26:24Z

Thanks for your PR. But I don't have the time to review this PR on my weekday. So I'll check on next holiday, please wait just a moment pray

No problem.

crates/libcontainer/src/container/container_checkpoint.rs

Furisto · 2022-02-15T23:07:48Z

crates/libcontainer/src/container/container_checkpoint.rs

+        }
+
+        let directory = std::fs::File::open(&opts.image_path)?;
+        criu.set_images_dir_fd(directory.as_raw_fd());


Do we need to create the directory?

crates/libcontainer/src/container/container_checkpoint.rs

Furisto · 2022-02-15T23:12:06Z

Did not manage to go through everything today, so feel free to wait until I am finished.

adrianreber · 2022-02-16T14:38:09Z

Changed as suggested by @Furisto

crates/integration_test/src/tests/lifecycle/checkpoint.rs

utam0k · 2022-02-19T05:31:31Z

crates/youki/src/commands/checkpoint.rs

+pub fn checkpoint(args: Checkpoint, root_path: PathBuf) -> Result<()> {
+    log::debug!("start checkpointing container {}", args.container_id);
+    let mut container = load_container(root_path, &args.container_id)?;
+    let opts = libcontainer::container::CheckpointOptions {


Why don't you implement impl From<CheckpointOption> from Checkpoint?

I don't understand this question. Maybe my rust knowledge is not good enough. Can you be more specific about what needs to change here?

@adrianreber I'm sorry for my inadequate explanation. Why don't you implement this trait? I feel it would be more Rust-like if the conversion from Checkpoint to CheckpointOptions could be done using from(). WDYT?
https://doc.rust-lang.org/std/convert/trait.From.html

Ah, okay. I tried it, but I am not sure where. If I try to implement it in the youki create I get:

impl doesn't use only types from inside the current crate

If I add it to the liboci_cli crate I need to access information from libcontainer and also the other way around. Not sure if that is desired or even possible.

Any recommendations where I need to add the corresponding implementation?

utam0k · 2022-02-19T05:57:25Z

Great idea. I think we can represent the WIP by adding it to the feature table in the README.md, WDYT?

I called the checkpoint command ´checkpointt' with two ts so that Podman does not think youki already fully supports checkpoint/restore. Once everything works I will rename it.

crates/libcontainer/src/container/container_checkpoint.rs

crates/liboci-cli/src/checkpoint.rs

crates/libcontainer/src/container/container_checkpoint.rs

Furisto · 2022-02-20T20:16:54Z

.github/workflows/integration_tests_validation.yaml

@@ -36,9 +36,14 @@ jobs:
        uses: Swatinem/rust-cache@v1
      - run: sudo apt-get -y update
      - run: sudo apt-get install -y pkg-config libsystemd-dev libdbus-glib-1-dev libelf-dev libseccomp-dev
+      - name: Install runc 1.1.0


Why do we need to install this version of runc?

runc 1.1.0 changed how it handles --work-dir (or --work-path). Before 1.1.0 runc would create a directory called criu-work if the user does not specify a working directory. CRIU's default behaviour is to put the log and statistics files in the image path if work dir is not set. With runc 1.1.0 runc behaves like CRIU. That is also what I did for this PR and therefore the tests do not pass with older versions of runc.

adrianreber · 2022-02-21T18:55:26Z

Great idea. I think we can represent the WIP by adding it to the feature table in the README.md, WDYT?

I called the checkpoint command ´checkpointt' with two ts so that Podman does not think youki already fully supports checkpoint/restore. Once everything works I will rename it.

In this PR or in another PR?

utam0k · 2022-02-23T04:34:46Z

Great idea. I think we can represent the WIP by adding it to the feature table in the README.md, WDYT?

I called the checkpoint command ´checkpointt' with two ts so that Podman does not think youki already fully supports checkpoint/restore. Once everything works I will rename it.

In this PR or in another PR?

@adrianreber Can I ask for this PR?

adrianreber · 2022-02-24T14:29:27Z

Did most of the suggested changes (besides the one with impl From<) and also added a line to README.md.

0..2 does not include 2. Change it to 0..3 to include 2. Signed-off-by: Adrian Reber <areber@redhat.com>

This adds the first code to checkpoint a container. The checkpoint command is name 'checkpointt' (with two 't's at the end) so that container engines like Podman do not think to use this not yet finished checkpoint restore implementation. For Podman it is still necessary to tell CRIU that the network namespace is external at least and restoring needs special handling to support '--console-socket'. Signed-off-by: Adrian Reber <areber@redhat.com>

If stdin is not pointing to /dev/null checkpointing fails for now. Just point it to /dev/null. Signed-off-by: Adrian Reber <areber@redhat.com>

This still uses the subcommand 'checkpointt' until it works in combination with Podman. Signed-off-by: Adrian Reber <areber@redhat.com>

utam0k

@adrianreber LGTM 💯
Thanks!!!

adrianreber force-pushed the 2022-01-24-checkpointt branch 2 times, most recently from 5c4fca1 to 7ae3c3c Compare January 25, 2022 17:02

adrianreber force-pushed the 2022-01-24-checkpointt branch from 7ae3c3c to e62e325 Compare January 25, 2022 18:20

adrianreber force-pushed the 2022-01-24-checkpointt branch 6 times, most recently from 78cde5e to e9b0ed9 Compare January 26, 2022 09:59

adrianreber force-pushed the 2022-01-24-checkpointt branch 3 times, most recently from 1dd0f57 to 63105b0 Compare January 26, 2022 13:30

rst0git reviewed Jan 26, 2022

View reviewed changes

adrianreber mentioned this pull request Feb 3, 2022

Rust bindings for CRIU checkpoint-restore/criu#1722

Closed

adrianreber force-pushed the 2022-01-24-checkpointt branch from 63105b0 to 087fa93 Compare February 14, 2022 16:27

adrianreber marked this pull request as ready for review February 14, 2022 16:27

Furisto reviewed Feb 15, 2022

View reviewed changes

adrianreber force-pushed the 2022-01-24-checkpointt branch from 087fa93 to 8d44706 Compare February 16, 2022 14:37

adrianreber force-pushed the 2022-01-24-checkpointt branch from 8d44706 to 605766b Compare February 16, 2022 14:50

utam0k reviewed Feb 19, 2022

View reviewed changes

crates/integration_test/src/tests/lifecycle/checkpoint.rs Outdated Show resolved Hide resolved

utam0k reviewed Feb 19, 2022

View reviewed changes

crates/libcontainer/src/container/container_checkpoint.rs Show resolved Hide resolved

Furisto reviewed Feb 20, 2022

View reviewed changes

adrianreber force-pushed the 2022-01-24-checkpointt branch from 605766b to a0aef0e Compare February 24, 2022 14:27

adrianreber added 4 commits February 24, 2022 14:36

Fix off-by-one error

94bcd00

0..2 does not include 2. Change it to 0..3 to include 2. Signed-off-by: Adrian Reber <areber@redhat.com>

Also connect stdin to /dev/null for test containers

44ae6d7

If stdin is not pointing to /dev/null checkpointing fails for now. Just point it to /dev/null. Signed-off-by: Adrian Reber <areber@redhat.com>

Add checkpoint testing

0547fc2

This still uses the subcommand 'checkpointt' until it works in combination with Podman. Signed-off-by: Adrian Reber <areber@redhat.com>

adrianreber force-pushed the 2022-01-24-checkpointt branch from a0aef0e to 0547fc2 Compare February 24, 2022 14:36

utam0k approved these changes Feb 25, 2022

View reviewed changes

utam0k merged commit 6c60abd into youki-dev:main Feb 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial checkpoint support #641

Initial checkpoint support #641

adrianreber commented Jan 24, 2022 •

edited by gitpod-io bot

Loading

codecov-commenter commented Jan 25, 2022

adrianreber commented Jan 25, 2022

adrianreber commented Jan 26, 2022

rst0git Jan 26, 2022 •

edited

Loading

Furisto Feb 15, 2022 •

edited

Loading

adrianreber Feb 16, 2022

adrianreber commented Feb 14, 2022

utam0k commented Feb 15, 2022

adrianreber commented Feb 15, 2022

Furisto Feb 15, 2022 •

edited

Loading

Furisto commented Feb 15, 2022

adrianreber commented Feb 16, 2022

utam0k Feb 19, 2022 •

edited

Loading

adrianreber Feb 21, 2022

utam0k Feb 23, 2022

adrianreber Feb 23, 2022

utam0k commented Feb 19, 2022

Furisto Feb 20, 2022

adrianreber Feb 21, 2022

adrianreber commented Feb 21, 2022

utam0k commented Feb 23, 2022

adrianreber commented Feb 24, 2022

utam0k left a comment

Initial checkpoint support #641

Initial checkpoint support #641

Conversation

adrianreber commented Jan 24, 2022 • edited by gitpod-io bot Loading

codecov-commenter commented Jan 25, 2022

Codecov Report

adrianreber commented Jan 25, 2022

adrianreber commented Jan 26, 2022

rst0git Jan 26, 2022 • edited Loading

Choose a reason for hiding this comment

Furisto Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

adrianreber Feb 16, 2022

Choose a reason for hiding this comment

adrianreber commented Feb 14, 2022

utam0k commented Feb 15, 2022

adrianreber commented Feb 15, 2022

Furisto Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

Furisto commented Feb 15, 2022

adrianreber commented Feb 16, 2022

utam0k Feb 19, 2022 • edited Loading

Choose a reason for hiding this comment

adrianreber Feb 21, 2022

Choose a reason for hiding this comment

utam0k Feb 23, 2022

Choose a reason for hiding this comment

adrianreber Feb 23, 2022

Choose a reason for hiding this comment

utam0k commented Feb 19, 2022

Furisto Feb 20, 2022

Choose a reason for hiding this comment

adrianreber Feb 21, 2022

Choose a reason for hiding this comment

adrianreber commented Feb 21, 2022

utam0k commented Feb 23, 2022

adrianreber commented Feb 24, 2022

utam0k left a comment

Choose a reason for hiding this comment

adrianreber commented Jan 24, 2022 •

edited by gitpod-io bot

Loading

rst0git Jan 26, 2022 •

edited

Loading

Furisto Feb 15, 2022 •

edited

Loading

Furisto Feb 15, 2022 •

edited

Loading

utam0k Feb 19, 2022 •

edited

Loading