criu: enable setting of RPC config file

[rst0git]

This commit adds support for specifying a CRIU RPC configuration file. This allows users to overwrite the default CRIU options used by the container runtimes, for example, to specify options such as --tcp-established when checkpointing containers in Kubernetes. For compatibility with runc, the default config file path is set to /etc/criu/runc.conf.

We check for newer CRIU version than 4.1.1 as libcriu doesn't provide criu_set_config_file() in previous versions: checkpoint-restore/criu#2777

Summary by Sourcery

Add support for configuring CRIU via RPC config files, with sensible defaults and annotation-based overrides, and extend checkpoint/restore tests to cover these behaviors.

New Features:

• Allow specifying a CRIU RPC configuration file for checkpoint and restore operations, defaulting to /etc/criu/runc.conf and supporting a crun-specific config and annotation-based overrides.

Enhancements:

• Gracefully handle older CRIU versions without criu_set_config_file by ignoring implicit configs while failing only when an explicit config annotation is used.

Tests:

• Extend checkpoint/restore tests to verify default runc config usage, crun-specific config precedence, and annotation-based CRIU config selection including log file generation.

[sourcery-ai]

Reviewer's Guide

Adds support for specifying a CRIU RPC configuration file (with defaults and annotations) when checkpointing/restoring containers, including tests that validate config resolution order and compatibility with different CRIU versions.

File-Level Changes

Change	Details	Files
Wire CRIU RPC config-file selection into checkpoint/restore flow with default and override behavior.	Introduce constants for default runc and crun CRIU config file paths. Extend the libcriu wrapper to optionally load and expose criu_set_config_file from libcriu, treating it as mandatory for enabling config-file support but optional for older CRIU versions. Add handle_criu_config_file helper that resolves the config file from annotation, crun-specific config, or runc default, and gracefully handles absence of criu_set_config_file on older CRIU versions. Invoke handle_criu_config_file during both checkpoint and restore to ensure consistent CRIU configuration behavior, surfacing a clear error when explicit annotation is used but libcriu is too old.	src/libcrun/criu.c
Extend checkpoint/restore tests to cover CRIU config-file behavior and config precedence.	Generalize run_cr_test to accept callbacks before checkpoint and restore, enabling dynamic CRIU config setup. Add helpers to create and clean up CRIU config files under /etc/criu, including temporary log file locations for dump/restore runs. Introduce _run_cr_test_with_config to run CR tests with a specified config name, optional extra configs, and optional annotations, and validate that CRIU log files are created and non-empty. Add tests to verify using the default runc-style config, crun-specific config overriding runc config, and an annotation-specified config that overrides both. Register the new tests in the all_tests mapping so they are exercised by the test suite.	tests/test_checkpoint_restore.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[rst0git]

cc: @adrianreber

[adrianreber]

The correct name of the file is tricky. Not sure if we should use runc.conf or crun.conf.

@rst0git You should include the annotation possibility from runc also https://github.com/opencontainers/runc/blob/main/docs%2Fcheckpoint-restore.md

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[packit-as-a-service]

Ephemeral COPR build failed. @containers/packit-build please check.

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• In _run_cr_test_with_config, the result of run_cr_test is ignored and the function only checks for log files, so a failing checkpoint/restore that still writes logs would incorrectly be reported as success; consider propagating the run_cr_test exit status as part of the return value.
• The tests write and delete config files under /etc/criu, which can require elevated privileges and may interfere with host configuration; it would be safer to use a test-local directory and point CRIU to it rather than mutating /etc.
• The error from handle_criu_config_file is wrapped at the call sites with a generic "libcriu doesn't support RPC config file" message, which hides the more specific reason (e.g. CRIU version too old); consider returning or augmenting the original error text instead of overwriting it.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `_run_cr_test_with_config`, the result of `run_cr_test` is ignored and the function only checks for log files, so a failing checkpoint/restore that still writes logs would incorrectly be reported as success; consider propagating the `run_cr_test` exit status as part of the return value.
- The tests write and delete config files under `/etc/criu`, which can require elevated privileges and may interfere with host configuration; it would be safer to use a test-local directory and point CRIU to it rather than mutating `/etc`.
- The error from `handle_criu_config_file` is wrapped at the call sites with a generic `"libcriu doesn't support RPC config file"` message, which hides the more specific reason (e.g. CRIU version too old); consider returning or augmenting the original error text instead of overwriting it.

## Individual Comments

### Comment 1
<location> `src/libcrun/criu.c:495` </location>
<code_context>
+      config_file = CRIU_CRUN_CONFIG_FILE;
+    }
+
+  libcriu_wrapper->criu_set_config_file (config_file);
+
+  return 0;
</code_context>

<issue_to_address>
**issue (bug_risk):** The return value of criu_set_config_file is ignored, potentially hiding CRIU-side configuration errors.

`criu_set_config_file` returns an `int`, so it can fail (e.g., invalid path, permissions, CRIU errors). Ignoring this means checkpoint/restore continues as if the config was applied when it may not have been.

Please capture and return the error, e.g.:

```c
int ret = libcriu_wrapper->criu_set_config_file(config_file);
if (UNLIKELY(ret < 0))
  return crun_make_error(err, 0, "failed to set CRIU config file '%s'", config_file);
```

and have callers of `handle_criu_config_file` propagate the error so config failures are visible to the caller.
</issue_to_address>

### Comment 2
<location> `tests/test_checkpoint_restore.py:283-285` </location>
<code_context>
+            raise
+
+
+def _clean_up_criu_configs():
+    for conf_file in ["crun.conf", "runc.conf", "annotation.conf"]:
+        _remove_file(os.path.join("/etc/criu", conf_file))
+
+
</code_context>

<issue_to_address>
**suggestion (testing):** Consider guarding tests that modify `/etc/criu` to avoid permission-related flakiness

These tests create and delete files under `/etc/criu`, which typically requires root. When run by an unprivileged user, they’ll fail with permission errors rather than exercising the intended behavior. Please either restrict them to privileged/CI runs or add a skip condition (e.g., `os.access("/etc/criu", os.W_OK)`) to avoid flaky local failures.

Suggested implementation:

```python
import errno
import pytest

```

For this guard to be effective for all tests that touch `/etc/criu`, ensure that:
1. Each test that creates/deletes `/etc/criu` config files calls `_clean_up_criu_configs()` before performing any write/cleanup under `/etc/criu`.
2. If there are other helpers that write into `/etc/criu` directly (not via `_clean_up_criu_configs()`), consider adding a similar `os.access("/etc/criu", os.W_OK)` check (and `pytest.skip`) in those helpers or at the top of the respective tests.
</issue_to_address>

### Comment 3
<location> `tests/test_checkpoint_restore.py:342-348` </location>
<code_context>
+    return _run_cr_test_with_config("crun", ("crun-dump.log", "crun-restore.log"), extra_configs=extra)
+
+
+def test_cr_with_annotation_config():
+    # Create annotation config file
+    annotations = {"org.criu.config": "/etc/criu/annotation.conf"}
+    _create_criu_config("annotation", f"log-file=annotation.log")
+    # The following config files should be ignored by crun
+    extra = {"runc": "log-file=test-runc.log", "crun": "log-file=test-crun.log"}
+    return _run_cr_test_with_config("annotation", ("annotation.log", "annotation.log"), extra_configs=extra, annotations=annotations)
+
+
</code_context>

<issue_to_address>
**nitpick:** Redundant pre-creation of `annotation.conf` in `test_cr_with_annotation_config` may obscure the behavior being tested

In `test_cr_with_annotation_config`, the call to `_create_criu_config("annotation", "log-file=annotation.log")` is immediately overridden by the `before_checkpoint_cb`/`before_restore_cb` logic in `_run_cr_test_with_config`, which writes a new `annotation.conf` in the temp directory. This makes the initial creation redundant and potentially misleading, since it suggests the test relies on a pre-existing config. Please either remove the initial `_create_criu_config` call or add a brief comment noting that the callbacks always overwrite any existing file.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[rst0git]

@giuseppe @adrianreber I've updated the pull request with support for crun-version of the config file and the possibility to set a config file with container annotation.

[packit-as-a-service]

TMT tests failed. @containers/packit-build please check.