fix(3207, 3209) Difference between the exec command in runc and youki by tommady · Pull Request #3210 · youki-dev/youki

tommady · 2025-07-29T12:43:14Z

Description

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactoring (no functional changes)
Performance improvement
Test updates
CI/CD related changes
Other (please describe):

Testing

Added new unit tests
Added new integration tests
Ran existing test suite
Tested manually (please provide steps)

Related Issues

Fixes #3207 #3209

Additional Context

Bumps the patch group with 4 updates: [libc](https://github.com/rust-lang/libc), [clap](https://github.com/clap-rs/clap), [serde_json](https://github.com/serde-rs/json) and [clap_complete](https://github.com/clap-rs/clap). Updates `libc` from 0.2.174 to 0.2.175 - [Release notes](https://github.com/rust-lang/libc/releases) - [Changelog](https://github.com/rust-lang/libc/blob/0.2.175/CHANGELOG.md) - [Commits](rust-lang/libc@0.2.174...0.2.175) Updates `clap` from 4.5.4 to 4.5.13 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.4...clap_complete-v4.5.13) Updates `serde_json` from 1.0.141 to 1.0.142 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.141...v1.0.142) Updates `clap_complete` from 4.5.1 to 4.5.13 - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.1...clap_complete-v4.5.13) --- updated-dependencies: - dependency-name: libc dependency-version: 0.2.175 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: clap dependency-version: 4.5.13 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: serde_json dependency-version: 1.0.142 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: clap_complete dependency-version: 4.5.13 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps the patch group with 3 updates: [oci-spec](https://github.com/youki-dev/oci-spec-rs), [thiserror](https://github.com/dtolnay/thiserror) and [anyhow](https://github.com/dtolnay/anyhow). Updates `oci-spec` from 0.8.1 to 0.8.2 - [Changelog](https://github.com/youki-dev/oci-spec-rs/blob/main/release.md) - [Commits](youki-dev/oci-spec-rs@v0.8.1...v0.8.2) Updates `thiserror` from 2.0.12 to 2.0.14 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.12...2.0.14) Updates `anyhow` from 1.0.98 to 1.0.99 - [Release notes](https://github.com/dtolnay/anyhow/releases) - [Commits](dtolnay/anyhow@1.0.98...1.0.99) --- updated-dependencies: - dependency-name: oci-spec dependency-version: 0.8.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: thiserror dependency-version: 2.0.14 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: anyhow dependency-version: 1.0.99 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

…Option of vector of string Signed-off-by: tommady <tommady@users.noreply.github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

…ux yet Signed-off-by: tommady <tommady@users.noreply.github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

Bumps the patch group with 1 update: [thiserror](https://github.com/dtolnay/thiserror). Updates `thiserror` from 2.0.14 to 2.0.15 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.14...2.0.15) --- updated-dependencies: - dependency-name: thiserror dependency-version: 2.0.15 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps the patch group with 1 update: [serde_json](https://github.com/serde-rs/json). Updates `serde_json` from 1.0.142 to 1.0.143 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.142...v1.0.143) --- updated-dependencies: - dependency-name: serde_json dependency-version: 1.0.143 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

Bumps the patch group with 1 update: [thiserror](https://github.com/dtolnay/thiserror). Updates `thiserror` from 2.0.15 to 2.0.16 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.15...2.0.16) --- updated-dependencies: - dependency-name: thiserror dependency-version: 2.0.16 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

Bumps the patch group with 1 update: [regex](https://github.com/rust-lang/regex). Updates `regex` from 1.11.1 to 1.11.2 - [Release notes](https://github.com/rust-lang/regex/releases) - [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md) - [Commits](rust-lang/regex@1.11.1...1.11.2) --- updated-dependencies: - dependency-name: regex dependency-version: 1.11.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Bumps the patch group with 1 update: [tracing-subscriber](https://github.com/tokio-rs/tracing). Updates `tracing-subscriber` from 0.3.19 to 0.3.20 - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-subscriber-0.3.19...tracing-subscriber-0.3.20) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-version: 0.3.20 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady · 2026-01-06T11:24:20Z

The changes in #3347 will help address and enhance the testcase requested by saku3.

Just a memo for myself — turning this PR back to draft.

saku3 · 2026-01-31T07:46:55Z

@tommady
I haven’t had much time to review this PR recently — sorry about that.

For this PR, if we can’t move it forward until issue #3347 is resolved, I think we could proceed by leaving TODO comments (or create sub-issue) for the parts related to #3347 for now.

What approach would be easiest for you to proceed?

tommady · 2026-02-02T05:42:27Z

@tommady I haven’t had much time to review this PR recently — sorry about that.

For this PR, if we can’t move it forward until issue #3347 is resolved, I think we could proceed by leaving TODO comments (or create sub-issue) for the parts related to #3347 for now.

What approach would be easiest for you to proceed?

Thanks for the suggestion!

From my point of view, this PR is ready for review.
Aside from the parts related to issue #3347, there was also an earlier comment from you:
#3210 (comment)

Because of that, some of the cgroup tests can’t be implemented right now.

So yes, please review it when you have time 🙂

Signed-off-by: tommady <tommady@users.noreply.github.com>

saku3 · 2026-02-02T21:23:38Z

Thank! I’ll review it this week.

saku3

I've left a few comments.
Everything else looks good to me.
Could you please take a look when you have a chance?
Thanks for the solid work on this!

saku3 · 2026-02-05T22:17:39Z

crates/libcontainer/src/container/tenant_builder.rs

+    fn get_no_new_privileges(&self, spec: &Spec) -> Option<bool> {
        self.no_new_privs
+            .filter(|&is_set| is_set)
+            .or_else(|| spec.process().clone().and_then(|p| p.no_new_privileges()))


It looks like we can avoid using clone().

Suggested change

.or_else(|| spec.process().clone().and_then(|p| p.no_new_privileges()))

.or_else(|| spec.process().as_ref().and_then(|p| p.no_new_privileges()))

saku3 · 2026-02-05T22:33:24Z

tests/contest/contest/src/tests/exec/ignore_paused_test.rs

+
+        let id2 = id.clone();
+        let dir2 = dir.to_path_buf();
+        std::thread::spawn(move || {


This means we want to resume the paused container at the end, right?

This might be prone to flakiness.
Would you consider using scopeguard::defer!?

scopeguard was removed earlier (see commit below), and in this case it would also resume the container too late since it only runs at scope exit. I’ll use an mpsc channel instead to explicitly synchronize the resume during exec --ignore-paused. Thanks for the suggestion.

123b51a

Sorry, I understand now.

In this case, I think the previous implementation (adding a short sleep) is acceptable.
It aligns with runc’s behavior, so I don’t expect it to cause issues in practice.

Also, since the spawn order doesn’t guarantee the actual execution order, resume may run first, meaning the test might not actually verify that exec works while the container is paused.

saku3 · 2026-02-07T06:34:52Z

crates/libcontainer/src/container/builder_impl.rs

+                        sub_cgroup_path
+                    )));
+                }
+                final_cgroups_path = potential_path;


Shouldn’t the final value we want to set be normalized?

Suggested change

final_cgroups_path = potential_path;

final_cgroups_path = normalized;

good catcha!

Signed-off-by: tommady <tommady@users.noreply.github.com>

saku3

Sorry for the delayed review.

I left comments on with_apparmor and ignore_paused_test.rs. Could you please take a look?

Also, since this(998d1c2 and 691e046) change is unrelated to the issue we’re addressing here, let’s handle it in a separate PR.

saku3 · 2026-02-28T04:36:30Z

crates/youki/src/commands/exec.rs

@@ -9,8 +9,6 @@ use nix::sys::wait::{WaitStatus, waitpid};
 use crate::workload::executor::default_executor;

 pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {


with_apparmor seems to be missing.

saku3 · 2026-02-28T05:06:26Z

tests/contest/contest/src/tests/exec/ignore_paused_test.rs

+
+        let id2 = id.clone();
+        let dir2 = dir.to_path_buf();
+        std::thread::spawn(move || {


Sorry, I understand now.

In this case, I think the previous implementation (adding a short sleep) is acceptable.
It aligns with runc’s behavior, so I don’t expect it to cause issues in practice.

Also, since the spawn order doesn’t guarantee the actual execution order, resume may run first, meaning the test might not actually verify that exec works while the container is paused.

saku3 · 2026-02-28T05:16:21Z

I think everything looks mostly fine aside from the parts I commented on.

PTAL @utam0k

utam0k added the kind/feature label Aug 11, 2025

dependabot bot and others added 14 commits August 11, 2025 20:48

Merge pull request #148 from tommady/dependabot/cargo/patch-e673b5859b

095d782

Merge pull request #150 from tommady/dependabot/cargo/patch-7466930332

14b314d

Merge branch 'youki-dev:main' into main

611bac8

add exec command with_capabilities

96c32fd

Signed-off-by: tommady <tommady@users.noreply.github.com>

add exec command with_ignore_paused

ad79172

Signed-off-by: tommady <tommady@users.noreply.github.com>

fix issue 3209 and change exec arg cgroup from Option of string into …

9b3e137

…Option of vector of string Signed-off-by: tommady <tommady@users.noreply.github.com>

add exec command with_cgroup

f5424e5

Signed-off-by: tommady <tommady@users.noreply.github.com>

add exec command with_process_label, but youki does not support selin…

a707ab8

…ux yet Signed-off-by: tommady <tommady@users.noreply.github.com>

add exec command with_preserve_fds

dd8e8ed

Signed-off-by: tommady <tommady@users.noreply.github.com>

add exec command with_apparmor

00fdace

Signed-off-by: tommady <tommady@users.noreply.github.com>

fix build error

165535d

Signed-off-by: tommady <tommady@users.noreply.github.com>

fix build error

9ea9fa1

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady force-pushed the close-issue-3207 branch from a173dd0 to 9ea9fa1 Compare August 14, 2025 08:35

tommady and others added 14 commits August 17, 2025 11:26

add preserve-fds test

9c46909

Signed-off-by: tommady <tommady@users.noreply.github.com>

Merge branch 'main' into close-issue-3207

c9c0654

Merge pull request #152 from tommady/dependabot/cargo/patch-a17342bb35

366cc71

Merge pull request #153 from tommady/dependabot/cargo/patch-b144f61cce

5982fbb

fix preserve_fds_test flank failure

bdab39b

Signed-off-by: tommady <tommady@users.noreply.github.com>

Merge pull request #154 from tommady/dependabot/cargo/patch-e69270f3df

797d35e

add --ignore-paused e2e test

f9180bc

Signed-off-by: tommady <tommady@users.noreply.github.com>

Merge pull request #156 from tommady/dependabot/cargo/patch-a47d9d9293

a9a2d82

Merge pull request #157 from tommady/dependabot/cargo/patch-2dc5bac7e8

1aef1e4

tommady mentioned this pull request Dec 2, 2025

fix(3293) Ambient capabilities are not applied as expected #3294

Merged

13 tasks

Merge branch 'main' into close-issue-3207

98574cc

tommady self-assigned this Dec 20, 2025

tommady added 3 commits December 20, 2025 08:14

address comment change to use utils normalize

26787fe

Signed-off-by: tommady <tommady@users.noreply.github.com>

address comment add comment for cap test to more detail description

30fb0f6

Signed-off-by: tommady <tommady@users.noreply.github.com>

add more part of the cgroup tests request from saku3

e3c3a0f

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady mentioned this pull request Jan 3, 2026

Try to join the cgroup of the init process of the parent container when apply_cgroup for a tenant container fails due to a "Device or resource busy" error #3347

Draft

13 tasks

tommady added 2 commits January 3, 2026 21:26

Merge branch 'main' into close-issue-3207

5da5a00

fix linter

99c0f18

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady marked this pull request as draft January 6, 2026 11:20

tommady added 2 commits February 2, 2026 05:45

add TODO comment for exec/cgroup_test.rs tests

5411bae

Signed-off-by: tommady <tommady@users.noreply.github.com>

fix linter

d4c84df

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady marked this pull request as ready for review February 2, 2026 05:58

saku3 reviewed Feb 7, 2026

View reviewed changes

tommady added 5 commits February 7, 2026 22:23

address comments

43d2efb

Signed-off-by: tommady <tommady@users.noreply.github.com>

Merge branch 'main' into close-issue-3207

02cfb2a

enhance ignore_paused_test.rs to ensure exec init then resume after

3593049

Signed-off-by: tommady <tommady@users.noreply.github.com>

add -skip TestImagePullSchema1 since it is officially deprecated

998d1c2

Signed-off-by: tommady <tommady@users.noreply.github.com>

fix potential race condition in init process channel

691e046

Signed-off-by: tommady <tommady@users.noreply.github.com>

tommady requested a review from saku3 February 8, 2026 02:54

tommady added 3 commits February 8, 2026 02:59

undo e2e workflow format

20d2ebe

Signed-off-by: tommady <tommady@users.noreply.github.com>

undo e2e workflow format

cc744fd

Signed-off-by: tommady <tommady@users.noreply.github.com>

undo e2e workflow format

bb6b88f

Signed-off-by: tommady <tommady@users.noreply.github.com>

saku3 reviewed Feb 28, 2026

View reviewed changes

saku3 mentioned this pull request Feb 28, 2026

Make more clear what flags are not implemented #3430

Open

13 tasks

	.or_else(\|\| spec.process().clone().and_then(\|p\| p.no_new_privileges()))
	.or_else(\|\| spec.process().as_ref().and_then(\|p\| p.no_new_privileges()))

	final_cgroups_path = potential_path;
	final_cgroups_path = normalized;

		@@ -9,8 +9,6 @@ use nix::sys::wait::{WaitStatus, waitpid};
		use crate::workload::executor::default_executor;

		pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {

Conversation

tommady commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of Change

Testing

Related Issues

Additional Context

Uh oh!

tommady commented Jan 6, 2026

Uh oh!

saku3 commented Jan 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tommady commented Feb 2, 2026

Uh oh!

saku3 commented Feb 2, 2026

Uh oh!

saku3 left a comment

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

tommady Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

tommady Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

saku3 left a comment

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

saku3 Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

saku3 commented Feb 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tommady commented Jul 29, 2025 •

edited

Loading

saku3 commented Jan 31, 2026 •

edited

Loading

tommady Feb 7, 2026 •

edited

Loading