Skip to content

Commit ddd6abb

Browse files
tommadydependabot[bot]github-actions[bot]
authored
fix(3207, 3209) Difference between the exec command in runc and youki (#3210)
* chore(deps): bump the patch group with 4 updates 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> * chore(deps): bump the patch group with 3 updates 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> * add exec command with_capabilities Signed-off-by: tommady <tommady@users.noreply.github.com> * add exec command with_ignore_paused Signed-off-by: tommady <tommady@users.noreply.github.com> * fix issue 3209 and change exec arg cgroup from Option of string into Option of vector of string Signed-off-by: tommady <tommady@users.noreply.github.com> * add exec command with_cgroup Signed-off-by: tommady <tommady@users.noreply.github.com> * add exec command with_process_label, but youki does not support selinux yet Signed-off-by: tommady <tommady@users.noreply.github.com> * add exec command with_preserve_fds Signed-off-by: tommady <tommady@users.noreply.github.com> * add exec command with_apparmor Signed-off-by: tommady <tommady@users.noreply.github.com> * fix build error Signed-off-by: tommady <tommady@users.noreply.github.com> * fix build error Signed-off-by: tommady <tommady@users.noreply.github.com> * add preserve-fds test Signed-off-by: tommady <tommady@users.noreply.github.com> * chore(deps): bump thiserror from 2.0.14 to 2.0.15 in the patch group 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> * chore(deps): bump serde_json from 1.0.142 to 1.0.143 in the patch group 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> * fix preserve_fds_test flank failure Signed-off-by: tommady <tommady@users.noreply.github.com> * chore(deps): bump thiserror from 2.0.15 to 2.0.16 in the patch group 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> * add --ignore-paused e2e test Signed-off-by: tommady <tommady@users.noreply.github.com> * chore(deps): bump regex from 1.11.1 to 1.11.2 in the patch group 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> * chore(deps): bump tracing-subscriber in the patch group 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> * chore(deps): bump chrono from 0.4.41 to 0.4.42 in the patch group Bumps the patch group with 1 update: [chrono](https://github.com/chronotope/chrono). Updates `chrono` from 0.4.41 to 0.4.42 - [Release notes](https://github.com/chronotope/chrono/releases) - [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md) - [Commits](chronotope/chrono@v0.4.41...v0.4.42) --- updated-dependencies: - dependency-name: chrono dependency-version: 0.4.42 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump errno from 0.3.13 to 0.3.14 in the patch group Bumps the patch group with 1 update: [errno](https://github.com/lambda-fairy/rust-errno). Updates `errno` from 0.3.13 to 0.3.14 - [Release notes](https://github.com/lambda-fairy/rust-errno/releases) - [Changelog](https://github.com/lambda-fairy/rust-errno/blob/main/CHANGELOG.md) - [Commits](https://github.com/lambda-fairy/rust-errno/commits) --- updated-dependencies: - dependency-name: errno dependency-version: 0.3.14 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com> * add cgroup tests Signed-off-by: tommady <tommady@users.noreply.github.com> * chore(deps): bump the patch group with 2 updates Bumps the patch group with 2 updates: [serde](https://github.com/serde-rs/serde) and [serde_json](https://github.com/serde-rs/json). Updates `serde` from 1.0.219 to 1.0.223 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.219...v1.0.223) Updates `serde_json` from 1.0.143 to 1.0.145 - [Release notes](https://github.com/serde-rs/json/releases) - [Commits](serde-rs/json@v1.0.143...v1.0.145) --- updated-dependencies: - dependency-name: serde dependency-version: 1.0.223 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch - dependency-name: serde_json dependency-version: 1.0.145 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump serde from 1.0.223 to 1.0.225 in the patch group Bumps the patch group with 1 update: [serde](https://github.com/serde-rs/serde). Updates `serde` from 1.0.223 to 1.0.225 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.223...v1.0.225) --- updated-dependencies: - dependency-name: serde dependency-version: 1.0.225 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patch ... Signed-off-by: dependabot[bot] <support@github.com> * fix linter Signed-off-by: tommady <tommady@users.noreply.github.com> * fix validate tests on runc Signed-off-by: tommady <tommady@users.noreply.github.com> * add no capability test Signed-off-by: tommady <tommady@users.noreply.github.com> * add new_privileges test Signed-off-by: tommady <tommady@users.noreply.github.com> * add ignoring for typo of checking the CapInh Signed-off-by: tommady <tommady@users.noreply.github.com> * add some_capabilities test Signed-off-by: tommady <tommady@users.noreply.github.com> * add capabilities_by_flag test Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment change to use utils normalize Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment add comment for cap test to more detail description Signed-off-by: tommady <tommady@users.noreply.github.com> * add more part of the cgroup tests request from saku3 Signed-off-by: tommady <tommady@users.noreply.github.com> * fix linter Signed-off-by: tommady <tommady@users.noreply.github.com> * add TODO comment for exec/cgroup_test.rs tests Signed-off-by: tommady <tommady@users.noreply.github.com> * fix linter Signed-off-by: tommady <tommady@users.noreply.github.com> * address comments Signed-off-by: tommady <tommady@users.noreply.github.com> * enhance ignore_paused_test.rs to ensure exec init then resume after Signed-off-by: tommady <tommady@users.noreply.github.com> * add -skip TestImagePullSchema1 since it is officially deprecated Signed-off-by: tommady <tommady@users.noreply.github.com> * fix potential race condition in init process channel Signed-off-by: tommady <tommady@users.noreply.github.com> * undo e2e workflow format Signed-off-by: tommady <tommady@users.noreply.github.com> * undo e2e workflow format Signed-off-by: tommady <tommady@users.noreply.github.com> * undo e2e workflow format Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment fix up e2e.yaml format Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment fix up e2e.yaml format Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment fix up e2e.yaml format Signed-off-by: tommady <tommady@users.noreply.github.com> * fix failures of merge main Signed-off-by: tommady <tommady@users.noreply.github.com> * fix test-contest exec after merge main Signed-off-by: tommady <tommady@users.noreply.github.com> * try to fix x86_64 musl failure Signed-off-by: tommady <tommady@users.noreply.github.com> * add code comment for preserve_fds_test.rs Signed-off-by: tommady <tommady@users.noreply.github.com> * address comment: fix logic bug in container state validation Signed-off-by: tommady <tommady@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: tommady <tommady@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent a68a38c commit ddd6abb

File tree

12 files changed

+789
-21
lines changed

12 files changed

+789
-21
lines changed

.typos.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ extend-exclude = [
1313
# instance than write a regex.
1414
569d5ce3afe1074769f67 = "569d5ce3afe1074769f67"
1515

16+
[default.extend-words]
17+
# This is the capabilities used in the exec integration test
18+
Inh = "Inh"
19+
1620
[type.rust.extend-words]
1721
ser = "ser"
1822
flate = "flate"

crates/libcontainer/src/container/builder_impl.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use crate::process::{self};
1717
use crate::syscall::syscall::SyscallType;
1818
use crate::user_ns::UserNamespaceConfig;
1919
use crate::utils;
20+
use crate::utils::PathBufExt;
2021
use crate::workload::Executor;
2122

2223
pub(super) struct ContainerBuilderImpl {
@@ -59,6 +60,12 @@ pub(super) struct ContainerBuilderImpl {
5960
pub stderr: Option<OwnedFd>,
6061
// Indicate if the init process should be a sibling of the main process.
6162
pub as_sibling: bool,
63+
// Run the process in an (existing) sub-cgroup(s)
64+
pub sub_cgroup_path: Option<String>,
65+
// Asm process label for the process commonly used with selinux.
66+
// TODO: youki does not support selinux yet
67+
#[allow(dead_code)]
68+
pub process_label: Option<String>,
6269
}
6370

6471
impl ContainerBuilderImpl {
@@ -85,9 +92,26 @@ impl ContainerBuilderImpl {
8592

8693
fn run_container(&mut self) -> Result<Pid, LibcontainerError> {
8794
let linux = self.spec.linux().as_ref().ok_or(MissingSpecError::Linux)?;
88-
let cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id);
95+
let base_cgroups_path = utils::get_cgroup_path(linux.cgroups_path(), &self.container_id);
96+
let mut final_cgroups_path = base_cgroups_path;
97+
98+
if let Some(sub_cgroup_path) = &self.sub_cgroup_path {
99+
if sub_cgroup_path != "/" {
100+
let potential_path = final_cgroups_path.join(sub_cgroup_path);
101+
let normalized = potential_path.normalize();
102+
103+
if !normalized.starts_with(&final_cgroups_path) {
104+
return Err(LibcontainerError::OtherCgroup(format!(
105+
"{} is not a sub cgroup path",
106+
sub_cgroup_path
107+
)));
108+
}
109+
final_cgroups_path = normalized;
110+
}
111+
}
112+
89113
let cgroup_config = libcgroups::common::CgroupConfig {
90-
cgroup_path: cgroups_path,
114+
cgroup_path: final_cgroups_path,
91115
systemd_cgroup: self.use_systemd || self.user_ns_config.is_some(),
92116
container_name: self.container_id.to_owned(),
93117
};

crates/libcontainer/src/container/init_builder.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ impl InitContainerBuilder {
117117
stdout: self.base.stdout,
118118
stderr: self.base.stderr,
119119
as_sibling: self.as_sibling,
120+
sub_cgroup_path: None,
121+
process_label: None,
120122
};
121123

122124
builder_impl.create()?;

crates/libcontainer/src/container/tenant_builder.rs

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use procfs::process::Namespace;
2121
use super::Container;
2222
use super::builder::ContainerBuilder;
2323
use crate::capabilities::CapabilityExt;
24+
use crate::container::ContainerStatus;
2425
use crate::container::builder_impl::ContainerBuilderImpl;
2526
use crate::error::{ErrInvalidSpec, LibcontainerError, MissingSpecError};
2627
use crate::notify_socket::NotifySocket;
@@ -48,6 +49,10 @@ pub struct TenantContainerBuilder {
4849
additional_gids: Vec<u32>,
4950
user: Option<u32>,
5051
group: Option<u32>,
52+
ignore_paused: bool,
53+
sub_cgroup: Option<String>,
54+
process_label: Option<String>,
55+
apparmor: Option<String>,
5156
}
5257

5358
/// This is a helper function to get capabilities for tenant container, based on
@@ -143,6 +148,10 @@ impl TenantContainerBuilder {
143148
additional_gids: vec![],
144149
user: None,
145150
group: None,
151+
ignore_paused: false,
152+
sub_cgroup: None,
153+
process_label: None,
154+
apparmor: None,
146155
}
147156
}
148157

@@ -206,6 +215,26 @@ impl TenantContainerBuilder {
206215
self
207216
}
208217

218+
pub fn with_ignore_paused(mut self, ignore_paused: bool) -> Self {
219+
self.ignore_paused = ignore_paused;
220+
self
221+
}
222+
223+
pub fn with_sub_cgroup(mut self, sub_cgroup: Option<String>) -> Self {
224+
self.sub_cgroup = sub_cgroup;
225+
self
226+
}
227+
228+
pub fn with_process_label(mut self, process_label: Option<String>) -> Self {
229+
self.process_label = process_label;
230+
self
231+
}
232+
233+
pub fn with_apparmor(mut self, apparmor: Option<String>) -> Self {
234+
self.apparmor = apparmor;
235+
self
236+
}
237+
209238
/// Joins an existing container
210239
pub fn build(self) -> Result<Pid, LibcontainerError> {
211240
let container_dir = self.lookup_container_dir()?;
@@ -252,6 +281,8 @@ impl TenantContainerBuilder {
252281
stdout: self.base.stdout,
253282
stderr: self.base.stderr,
254283
as_sibling: self.as_sibling,
284+
sub_cgroup_path: self.sub_cgroup,
285+
process_label: self.process_label,
255286
};
256287

257288
let pid = builder_impl.create()?;
@@ -419,12 +450,15 @@ impl TenantContainerBuilder {
419450

420451
fn load_container_state(&self, container_dir: PathBuf) -> Result<Container, LibcontainerError> {
421452
let container = Container::load(container_dir)?;
422-
if !container.can_exec() {
423-
tracing::error!(status = ?container.status(), "cannot exec as container");
424-
return Err(LibcontainerError::IncorrectStatus(container.status()));
425-
}
426453

427-
Ok(container)
454+
match container.status() {
455+
ContainerStatus::Running => Ok(container),
456+
ContainerStatus::Paused if self.ignore_paused => Ok(container),
457+
_ => {
458+
tracing::error!(status = ?container.status(), "cannot exec: invalid container state");
459+
Err(LibcontainerError::IncorrectStatus(container.status()))
460+
}
461+
}
428462
}
429463

430464
fn adapt_spec_for_tenant(
@@ -454,10 +488,14 @@ impl TenantContainerBuilder {
454488
}
455489
}
456490

457-
if let Some(no_new_priv) = self.get_no_new_privileges() {
491+
if let Some(no_new_priv) = self.get_no_new_privileges(spec) {
458492
process_builder = process_builder.no_new_privileges(no_new_priv);
459493
}
460494

495+
if let Some(ref apparmor) = self.apparmor {
496+
process_builder = process_builder.apparmor_profile(apparmor)
497+
}
498+
461499
let capabilities = get_capabilities(&self.capabilities, spec)?;
462500
process_builder = process_builder.capabilities(capabilities);
463501

@@ -564,8 +602,10 @@ impl TenantContainerBuilder {
564602
env
565603
}
566604

567-
fn get_no_new_privileges(&self) -> Option<bool> {
605+
fn get_no_new_privileges(&self, spec: &Spec) -> Option<bool> {
568606
self.no_new_privs
607+
.filter(|&is_set| is_set)
608+
.or_else(|| spec.process().as_ref().and_then(|p| p.no_new_privileges()))
569609
}
570610

571611
fn get_namespaces(

crates/liboci-cli/src/exec.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,9 @@ pub struct Exec {
5555
/// Execute a process in a sub-cgroup
5656
#[clap(long)]
5757
pub cgroup: Option<String>,
58-
5958
/// Container identifier
6059
#[clap(value_parser = clap::builder::NonEmptyStringValueParser::new(), required = true)]
6160
pub container_id: String,
62-
6361
/// Command that should be executed in the container
6462
#[clap(required = false, trailing_var_arg = true)]
6563
pub command: Vec<String>,

crates/youki/src/commands/exec.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ use nix::sys::wait::{WaitStatus, waitpid};
99
use crate::workload::executor::default_executor;
1010

1111
pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
12-
// TODO: not all values from exec are used here. We need to support
13-
// the remaining ones.
1412
let user = args.user.map(|(u, _)| u);
1513
let group = args.user.and_then(|(_, g)| g);
1614

@@ -19,6 +17,7 @@ pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
1917
.with_root_path(root_path)?
2018
.with_console_socket(args.console_socket.as_ref())
2119
.with_pid_file(args.pid_file.as_ref())?
20+
.with_preserved_fds(args.preserve_fds)
2221
.validate_id()?
2322
.as_tenant()
2423
.with_detach(args.detach)
@@ -30,6 +29,10 @@ pub fn exec(args: Exec, root_path: PathBuf) -> Result<i32> {
3029
.with_additional_gids(args.additional_gids)
3130
.with_user(user)
3231
.with_group(group)
32+
.with_capabilities(args.cap)
33+
.with_ignore_paused(args.ignore_paused)
34+
.with_sub_cgroup(args.cgroup)
35+
.with_apparmor(args.apparmor)
3336
.build()?;
3437

3538
// See https://github.com/youki-dev/youki/pull/1252 for a detailed explanation

0 commit comments

Comments
 (0)