Skip to content

Commit 40f9325

Browse files
feat: add cgroup CPU isolation for sandboxed walltime runs
The walltime executor pins the benchmark to dedicated cores with `systemd-run --scope --slice=codspeed.slice`, but that needs a reachable host systemd — absent inside the macro-agent sandbox, where the runner is PID 1 of a private namespace with no host authority. Add a `Cgroup` isolation mode for that case. The privileged side delegates a scope cgroup (via `CODSPEED_CGROUP`) already split into `support` (system cores) and `bench` (the rest); the runner just relocates with two plain `cgroup.procs` writes — itself into `support`, so it and the profiler stay off the bench cores, and the benchmark into `bench` via a small `bash` shim. No cpuset arithmetic and no privilege: the benchmark stays in the profiler's process subtree, so it records unprivileged. Unlike the systemd scope, this keeps the benchmark a descendant of the profiler, so the `isolate` flag threaded through the profilers becomes `requires_sudo`, which is true only for the systemd mode. Refs COD-3012 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7ce2c98 commit 40f9325

5 files changed

Lines changed: 238 additions & 75 deletions

File tree

src/executor/wall_time/executor.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::helpers::validate_walltime_results;
2-
use super::isolation::{requires_isolation, wrap_isolation_scope};
2+
use super::isolation::resolve_isolation_mode;
33
use super::profiler::Profiler;
44
use super::profiler::perf::PerfProfiler;
55
use super::profiler::samply::SamplyProfiler;
@@ -181,14 +181,11 @@ impl Executor for WallTimeExecutor {
181181
let (_env_file, _script_file, cmd_builder) =
182182
WallTimeExecutor::walltime_bench_cmd(&execution_context.config, execution_context)?;
183183

184-
// Resolve the isolation decision once and reuse it for both the scope
184+
// Resolve the isolation decision once and reuse it for both the leaf
185185
// wrapping here and the privilege wrapping below (or in the profiler).
186-
let isolate = requires_isolation();
187-
let cmd_builder = if isolate {
188-
wrap_isolation_scope(cmd_builder)?
189-
} else {
190-
cmd_builder
191-
};
186+
let isolation_mode = resolve_isolation_mode();
187+
let cmd_builder = isolation_mode.wrap_bench(cmd_builder)?;
188+
let requires_sudo = isolation_mode.requires_sudo();
192189

193190
// Split-borrow `self` so the closure inside `run_with_profiler` can
194191
// capture `benchmark_state` while we hold `&mut profiler`.
@@ -204,16 +201,13 @@ impl Executor for WallTimeExecutor {
204201
cmd_builder,
205202
&execution_context.config,
206203
&execution_context.profile_folder,
207-
isolate,
204+
requires_sudo,
208205
benchmark_state,
209206
)
210207
.await
211208
}
212209
_ => {
213-
// No profiler: still honor the isolation decision, since the
214-
// scope (and the sudo it requires) is a property of the run, not
215-
// of the profiler.
216-
let cmd_builder = if isolate {
210+
let cmd_builder = if requires_sudo {
217211
wrap_with_sudo(cmd_builder)?
218212
} else {
219213
cmd_builder
@@ -262,11 +256,11 @@ async fn run_with_profiler(
262256
cmd_builder: CommandBuilder,
263257
config: &ExecutorConfig,
264258
profile_folder: &Path,
265-
isolate: bool,
259+
requires_sudo: bool,
266260
benchmark_state: &OnceCell<(FifoBenchmarkData, ExecutionTimestamps)>,
267261
) -> Result<std::process::ExitStatus> {
268262
let wrapped = profiler
269-
.wrap_command(cmd_builder, config, profile_folder, isolate)
263+
.wrap_command(cmd_builder, config, profile_folder, requires_sudo)
270264
.await?;
271265
let cmd = wrapped.build();
272266
debug!("cmd: {cmd:?}");
Lines changed: 216 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,107 @@
1+
use std::path::Path;
2+
13
use crate::executor::helpers::command::CommandBuilder;
24
use crate::prelude::*;
35

4-
/// Whether the benchmark must run inside the privileged systemd scope, which in
5-
/// turn requires the profiler itself to record under elevated privileges.
6-
///
7-
/// When isolated, the scope reparents the benchmark out of the profiler's
8-
/// process subtree, so the profiler can only observe it by recording
9-
/// system-wide — which needs `sudo`. When not isolated, the profiler records its
10-
/// own descendant tree and runs unprivileged (relying on a permissive
11-
/// `perf_event_paranoid`).
12-
///
13-
/// `CODSPEED_ISOLATION` overrides the decision; otherwise we isolate only when we
14-
/// can elevate without prompting (root or passwordless sudo, as on CI), so a
15-
/// local run never blocks on a password.
16-
///
17-
/// Isolation relies on `systemd-run`, so it is Linux-only; other platforms always
18-
/// record their own descendant tree unprivileged.
19-
pub fn requires_isolation() -> bool {
20-
if !cfg!(target_os = "linux") {
21-
return false;
6+
/// How the benchmark is pinned to dedicated cores, away from the rest of the
7+
/// system. Whether the profiler must run under sudo depends on the mode; see
8+
/// [`requires_sudo`](Self::requires_sudo).
9+
#[derive(Debug, Clone, PartialEq, Eq)]
10+
pub enum IsolationMode {
11+
/// The benchmark runs in the runner's own process subtree, unpinned.
12+
None,
13+
/// `systemd-run --scope --slice=codspeed.slice`. The scope reparents the
14+
/// benchmark out of the profiler's process subtree, so the profiler needs
15+
/// elevated privileges (sudo) to observe it.
16+
Systemd,
17+
/// The benchmark runs in a delegated cgroup scope.
18+
/// The scope is expected to be created with the expected leaves before CLI invocation.
19+
Cgroup {
20+
/// The delegated scope cgroup directory, as seen from the runner.
21+
scope_dir: String,
22+
},
23+
}
24+
25+
impl IsolationMode {
26+
/// Whether the profiler must run under sudo. True only when the benchmark is
27+
/// reparented out of the profiler's subtree (i.e. for [`Systemd`](Self::Systemd)),
28+
/// where the profiler needs elevated privileges to observe it.
29+
pub fn requires_sudo(&self) -> bool {
30+
matches!(self, IsolationMode::Systemd)
2231
}
23-
match std::env::var("CODSPEED_ISOLATION").as_deref() {
24-
Ok("true") => true,
25-
Ok("false") => false,
26-
_ => {
27-
let can_isolate = crate::executor::helpers::run_with_sudo::can_elevate_without_prompt();
28-
if !can_isolate {
29-
info!(
30-
"Running without process isolation: elevating privileges would require a \
31-
password prompt. Set CODSPEED_ISOLATION=true to force it."
32-
);
32+
33+
/// Wrap the benchmark leaf so it runs pinned according to this mode. The
34+
/// profiler wraps its recorder around the result, so this must only touch the
35+
/// leaf and never move the profiler onto the bench cores.
36+
pub fn wrap_bench(&self, bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
37+
match self {
38+
IsolationMode::None => Ok(bench_cmd),
39+
IsolationMode::Systemd => wrap_isolation_scope(bench_cmd),
40+
IsolationMode::Cgroup { scope_dir } => {
41+
// Move the runner (and the profiler it later spawns) onto the
42+
// system cores before wrapping the benchmark for the bench cores.
43+
place_runner_in_support(scope_dir)?;
44+
wrap_cgroup_isolation(bench_cmd, scope_dir)
3345
}
34-
can_isolate
3546
}
3647
}
3748
}
3849

39-
/// Run the benchmark command in an isolated process scope.
50+
/// Environment variable selecting how walltime runs isolate the benchmark.
51+
const ISOLATION_ENV: &str = "CODSPEED_ISOLATION";
52+
53+
/// Resolve how this run should isolate the benchmark from [`ISOLATION_ENV`]:
4054
///
41-
/// On Linux, the command is wrapped with `systemd-run --scope` so it runs inside the
42-
/// `codspeed.slice` cgroup (predefined on CodSpeed CI runners to pin and isolate the
43-
/// benchmark). Only applied when [`requires_isolation`] is true.
55+
/// - non-Linux: [`None`](IsolationMode::None), no mechanism is available;
56+
/// - unset: [`Systemd`](IsolationMode::Systemd) if we can elevate without a prompt
57+
/// (root or passwordless sudo, as on CI), else [`None`](IsolationMode::None) so a
58+
/// local run never blocks on a password prompt;
59+
/// - `false`: [`None`](IsolationMode::None);
60+
/// - `CGROUP:<dir>`: [`Cgroup`](IsolationMode::Cgroup) under `<dir>`, the only mode
61+
/// that works without host systemd (used inside the sandbox);
62+
/// - anything else: [`Systemd`](IsolationMode::Systemd).
63+
pub fn resolve_isolation_mode() -> IsolationMode {
64+
if !cfg!(target_os = "linux") {
65+
return IsolationMode::None;
66+
}
67+
68+
let Some(value) = std::env::var(ISOLATION_ENV).ok().filter(|v| !v.is_empty()) else {
69+
// Unset: isolate only if we can elevate silently, otherwise stay unpinned
70+
// so a local run never blocks on a password prompt.
71+
if crate::executor::helpers::run_with_sudo::can_elevate_without_prompt() {
72+
return IsolationMode::Systemd;
73+
}
74+
info!(
75+
"Running without process isolation: elevating privileges would require a \
76+
password prompt. Set {ISOLATION_ENV}=true to force it."
77+
);
78+
return IsolationMode::None;
79+
};
80+
81+
match value.as_str() {
82+
"false" => IsolationMode::None,
83+
_ => match value.strip_prefix("CGROUP:") {
84+
Some(scope_dir) => IsolationMode::Cgroup {
85+
scope_dir: scope_dir.to_string(),
86+
},
87+
None => IsolationMode::Systemd,
88+
},
89+
}
90+
}
91+
92+
/// Wrap the benchmark leaf so it runs inside the systemd `codspeed.slice` scope.
4493
///
45-
/// Remarks:
46-
/// - We're using `--scope` so that the profiler is able to capture the events of the benchmark
47-
/// process.
48-
/// - We can't use `--user` here because we need to run in `codspeed.slice`, otherwise we'd run in
49-
/// `user.slice` (which is isolated). We use `--uid` and `--gid` to keep running as the current
50-
/// user.
51-
/// - `--scope` only inherits the system environment, so the caller is expected to have already
52-
/// forwarded the relevant variables (via `wrap_with_env`).
53-
/// - The caller is expected to have already set the working directory on `bench_cmd`; it will be
54-
/// propagated to `systemd-run` via [`CommandBuilder::wrap_with`], and `--same-dir` makes the
55-
/// spawned scope inherit it.
94+
/// Notes on the `systemd-run` flags:
95+
/// - `--scope` (rather than a transient service) keeps the benchmark a child of
96+
/// the runner, so the profiler can capture its events; see
97+
/// [`IsolationMode::Systemd`].
98+
/// - `--user` would land us in `user.slice`, not `codspeed.slice`; `--uid`/`--gid`
99+
/// instead keep the scope running as the current user.
100+
/// - `--scope` only inherits the system environment, so the caller must have
101+
/// already forwarded the benchmark's variables (via `wrap_with_env`) and set
102+
/// its working directory, which `--same-dir` propagates into the scope.
56103
#[cfg(target_os = "linux")]
57-
pub fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
104+
fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
58105
use crate::executor::helpers::env::is_codspeed_debug_enabled;
59106

60107
let mut cmd_builder = CommandBuilder::new("systemd-run");
@@ -75,6 +122,129 @@ pub fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result<CommandBuil
75122
/// Dummy implementation on non-Linux platforms: the benchmark command is returned as-is.
76123
// TODO(COD-2513): implement an equivalent process-isolation mechanism on macOS
77124
#[cfg(not(target_os = "linux"))]
78-
pub fn wrap_isolation_scope(bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
125+
fn wrap_isolation_scope(bench_cmd: CommandBuilder) -> Result<CommandBuilder> {
79126
Ok(bench_cmd)
80127
}
128+
129+
/// Names of the leaves that are expected to be created prior to the CLI invocation.
130+
const BENCH_LEAF: &str = "bench";
131+
const SUPPORT_LEAF: &str = "support";
132+
133+
/// Move the runner itself into the scope's `support` leaf, so it and every child
134+
/// it later spawns (notably the profiler) run on the system cores rather than the
135+
/// bench cores. Call before spawning the profiler.
136+
///
137+
/// The workload starts in the scope's `all` leaf (every core); this opts the
138+
/// runner down onto the system cores, leaving the bench cores idle for the
139+
/// benchmark the shim later places into `bench`.
140+
fn place_runner_in_support(scope_dir: &str) -> Result<()> {
141+
let procs = Path::new(scope_dir).join(SUPPORT_LEAF).join("cgroup.procs");
142+
let pid = std::process::id().to_string();
143+
std::fs::write(&procs, &pid).with_context(|| format!("move runner into {}", procs.display()))
144+
}
145+
146+
/// Wrap the benchmark so it self-places into the scope's `bench` leaf.
147+
///
148+
/// The privileged side splits the delegated scope and pins the leaves; here a
149+
/// tiny `bash` shim writes its own PID into `<scope>/bench/cgroup.procs` (moving
150+
/// itself, and the benchmark it `exec`s, into the leaf) and then `exec`s the
151+
/// original command, so the benchmark inherits the leaf's `cpuset.cpus` pinning
152+
/// while staying the same PID and the same descendant of the profiler.
153+
///
154+
/// Must wrap only the leaf, before the profiler wraps the recorder around it, so
155+
/// the profiler itself is never moved onto the bench cores.
156+
fn wrap_cgroup_isolation(mut bench_cmd: CommandBuilder, scope_dir: &str) -> Result<CommandBuilder> {
157+
let bench = Path::new(scope_dir).join(BENCH_LEAF);
158+
wait_for_bench_leaf(&bench)?;
159+
160+
let procs_path = bench.join("cgroup.procs");
161+
let quoted = shell_words::quote(&procs_path.to_string_lossy()).into_owned();
162+
// `$0`/`$@`: the shim's own argv[0] is a throwaway "bash"; the real program
163+
// and its arguments follow and are re-exec'd verbatim via `exec "$@"`.
164+
let script = format!("echo $$ > {quoted} && exec \"$@\"");
165+
166+
bench_cmd.wrap("bash", ["-c".to_string(), script, "bash".to_string()]);
167+
Ok(bench_cmd)
168+
}
169+
170+
/// Wait for the privileged side to create the `bench` leaf, which it does
171+
/// asynchronously after spawning the scope. Bounded (~5s) so a misconfigured run
172+
/// fails with a clear error rather than the shim's opaque write failure.
173+
fn wait_for_bench_leaf(bench: &Path) -> Result<()> {
174+
for _ in 0..200 {
175+
if bench.is_dir() {
176+
return Ok(());
177+
}
178+
std::thread::sleep(std::time::Duration::from_millis(25));
179+
}
180+
bail!("delegated bench cgroup {} never appeared", bench.display())
181+
}
182+
183+
#[cfg(test)]
184+
mod tests {
185+
use super::*;
186+
use temp_env::with_vars;
187+
188+
#[test]
189+
fn cgroup_prefix_selects_cgroup_mode() {
190+
if !cfg!(target_os = "linux") {
191+
return;
192+
}
193+
with_vars(
194+
[(ISOLATION_ENV, Some("CGROUP:/sys/fs/cgroup/scope"))],
195+
|| {
196+
assert_eq!(
197+
resolve_isolation_mode(),
198+
IsolationMode::Cgroup {
199+
scope_dir: "/sys/fs/cgroup/scope".to_string(),
200+
}
201+
);
202+
},
203+
);
204+
}
205+
206+
#[test]
207+
fn isolation_false_disables() {
208+
with_vars([(ISOLATION_ENV, Some("false"))], || {
209+
assert_eq!(resolve_isolation_mode(), IsolationMode::None);
210+
});
211+
}
212+
213+
#[test]
214+
fn other_value_selects_systemd() {
215+
if !cfg!(target_os = "linux") {
216+
return;
217+
}
218+
with_vars([(ISOLATION_ENV, Some("true"))], || {
219+
assert_eq!(resolve_isolation_mode(), IsolationMode::Systemd);
220+
});
221+
}
222+
223+
#[test]
224+
fn cgroup_mode_keeps_profiler_unprivileged() {
225+
let cgroup = IsolationMode::Cgroup {
226+
scope_dir: "/x".to_string(),
227+
};
228+
assert!(!cgroup.requires_sudo());
229+
assert!(IsolationMode::Systemd.requires_sudo());
230+
}
231+
232+
#[test]
233+
fn cgroup_wrap_places_then_execs() {
234+
// The shim targets <scope>/bench/cgroup.procs; the leaf must exist first.
235+
let scope = tempfile::tempdir().unwrap();
236+
std::fs::create_dir(scope.path().join("bench")).unwrap();
237+
let bench_procs = scope.path().join("bench/cgroup.procs");
238+
239+
let mut cmd = CommandBuilder::new("bash");
240+
cmd.arg("/tmp/bench.sh");
241+
let wrapped = wrap_cgroup_isolation(cmd, &scope.path().to_string_lossy()).unwrap();
242+
assert_eq!(
243+
wrapped.as_command_line(),
244+
format!(
245+
"bash -c 'echo $$ > {} && exec \"$@\"' bash bash /tmp/bench.sh",
246+
bench_procs.display()
247+
)
248+
);
249+
}
250+
}

src/executor/wall_time/profiler/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,16 @@ pub trait Profiler {
4848
/// Profilers stash any live state they need for the duration of the run
4949
/// (control fifos, output paths) on `self`.
5050
///
51-
/// `isolate` mirrors the decision the executor used to wrap the benchmark in
52-
/// the systemd scope: when set, the profiler records system-wide under sudo;
53-
/// otherwise it records its own descendant tree unprivileged.
51+
/// When `requires_sudo` is set, the profiler must run elevated: the
52+
/// isolation mechanism reparented the benchmark out of its subtree, so it can
53+
/// only observe it with elevated privileges. Otherwise it records its own
54+
/// descendant tree unprivileged.
5455
async fn wrap_command(
5556
&mut self,
5657
cmd: CommandBuilder,
5758
config: &ExecutorConfig,
5859
profile_folder: &Path,
59-
isolate: bool,
60+
requires_sudo: bool,
6061
) -> anyhow::Result<CommandBuilder>;
6162

6263
/// The benchmarked process signaled the start of a measured region.

src/executor/wall_time/profiler/perf/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl Profiler for PerfProfiler {
9191
mut cmd_builder: CommandBuilder,
9292
config: &ExecutorConfig,
9393
profile_folder: &Path,
94-
isolate: bool,
94+
requires_sudo: bool,
9595
) -> anyhow::Result<CommandBuilder> {
9696
let perf_fifo = PerfFifo::new()?;
9797
let perf_file_path = profile_folder.join(PERF_PIPEDATA_FILE_NAME);
@@ -183,10 +183,9 @@ impl Profiler for PerfProfiler {
183183
self.perf_fifo = Some(perf_fifo);
184184
self.perf_file_path = Some(perf_file_path);
185185

186-
// Isolated runs reparent the benchmark out of perf's subtree, so perf
187-
// must record system-wide under sudo. Unisolated runs record perf's own
188-
// descendant tree unprivileged.
189-
let wrapped_builder = if isolate {
186+
// When the benchmark was reparented out of perf's subtree, perf needs
187+
// sudo to observe it; otherwise it records its own descendants unprivileged.
188+
let wrapped_builder = if requires_sudo {
190189
wrap_with_sudo(wrapped_builder)?
191190
} else {
192191
wrapped_builder

0 commit comments

Comments
 (0)