feat: add cgroup CPU isolation for sandboxed walltime runs

src/executor/wall_time/executor.rs

@@ -1,5 +1,5 @@
 use super::helpers::validate_walltime_results;
-use super::isolation::{requires_isolation, wrap_isolation_scope};
+use super::isolation::Isolation;
 use super::profiler::Profiler;
 use super::profiler::perf::PerfProfiler;
 use super::profiler::samply::SamplyProfiler;
@@ -29,58 +29,8 @@ use std::cell::OnceCell;
 use std::fs::canonicalize;
 use std::io::Write;
 use std::path::Path;
-use std::path::PathBuf;
-use std::process::Command;
 use tempfile::NamedTempFile;
 
-struct HookScriptsGuard {
-    post_bench_script: PathBuf,
-}
-
-impl HookScriptsGuard {
-    fn execute_script_from_path<P: AsRef<Path>>(path: P) -> anyhow::Result<()> {
-        let path = path.as_ref();
-        if !path.exists() || !path.is_file() {
-            debug!("Script not found or not a file: {}", path.display());
-            return Ok(());
-        }
-
-        let output = Command::new("bash").args([&path]).output()?;
-        if !output.status.success() {
-            debug!("stdout: {}", String::from_utf8_lossy(&output.stdout));
-            debug!("stderr: {}", String::from_utf8_lossy(&output.stderr));
-            bail!("Failed to execute script: {}", path.display());
-        }
-
-        Ok(())
-    }
-
-    pub fn setup_with_scripts<P: AsRef<Path>>(pre_bench_script: P, post_bench_script: P) -> Self {
-        if let Err(e) = Self::execute_script_from_path(pre_bench_script.as_ref()) {
-            debug!("Failed to execute pre-bench script: {e}");
-        }
-
-        Self {
-            post_bench_script: post_bench_script.as_ref().to_path_buf(),
-        }
-    }
-
-    pub fn setup() -> Self {
-        Self::setup_with_scripts(
-            "/usr/local/bin/codspeed-pre-bench",
-            "/usr/local/bin/codspeed-post-bench",
-        )
-    }
-}
-
-impl Drop for HookScriptsGuard {
-    fn drop(&mut self) {
-        if let Err(e) = Self::execute_script_from_path(&self.post_bench_script) {
-            debug!("Failed to execute post-bench script: {e}");
-        }
-    }
-}
-
 pub struct WallTimeExecutor {
     profiler: Option<Box<dyn Profiler>>,
 
@@ -176,19 +126,16 @@ impl Executor for WallTimeExecutor {
         execution_context: &ExecutionContext,
         _mongo_tracer: &Option<MongoTracer>,
     ) -> Result<()> {
-        let _guard = HookScriptsGuard::setup();
+        // Resolve isolation once: this runs the pre-bench setup, wraps the bench
+        // leaf, and (for hook mode) runs post-bench on drop. Held for the whole run
+        // so its teardown fires after the benchmark completes. `requires_sudo` is
+        // read off it for the privilege wrapping below (or in the profiler).
+        let isolation = Isolation::resolve();
+        let requires_sudo = isolation.requires_sudo();
 
         let (_env_file, _script_file, cmd_builder) =
             WallTimeExecutor::walltime_bench_cmd(&execution_context.config, execution_context)?;
-
-        // Resolve the isolation decision once and reuse it for both the scope
-        // wrapping here and the privilege wrapping below (or in the profiler).
-        let isolate = requires_isolation();
-        let cmd_builder = if isolate {
-            wrap_isolation_scope(cmd_builder)?
-        } else {
-            cmd_builder
-        };
+        let cmd_builder = isolation.wrap_bench(cmd_builder)?;
 
         // Split-borrow `self` so the closure inside `run_with_profiler` can
         // capture `benchmark_state` while we hold `&mut profiler`.
@@ -204,16 +151,13 @@ impl Executor for WallTimeExecutor {
                     cmd_builder,
                     &execution_context.config,
                     &execution_context.profile_folder,
-                    isolate,
+                    requires_sudo,
                     benchmark_state,
                 )
                 .await
             }
             _ => {
-                // No profiler: still honor the isolation decision, since the
-                // scope (and the sudo it requires) is a property of the run, not
-                // of the profiler.
-                let cmd_builder = if isolate {
+                let cmd_builder = if requires_sudo {
                     wrap_with_sudo(cmd_builder)?
                 } else {
                     cmd_builder
@@ -262,11 +206,11 @@ async fn run_with_profiler(
     cmd_builder: CommandBuilder,
     config: &ExecutorConfig,
     profile_folder: &Path,
-    isolate: bool,
+    requires_sudo: bool,
     benchmark_state: &OnceCell<(FifoBenchmarkData, ExecutionTimestamps)>,
 ) -> Result<std::process::ExitStatus> {
     let wrapped = profiler
-        .wrap_command(cmd_builder, config, profile_folder, isolate)
+        .wrap_command(cmd_builder, config, profile_folder, requires_sudo)
         .await?;
     let cmd = wrapped.build();
     debug!("cmd: {cmd:?}");
@@ -304,51 +248,3 @@ async fn run_with_profiler(
     })
     .await
 }
-
-#[cfg(test)]
-mod tests {
-    use tempfile::NamedTempFile;
-
-    use super::*;
-    use std::{
-        io::{Read, Write},
-        os::unix::fs::PermissionsExt,
-    };
-
-    #[test]
-    fn test_env_guard_no_crash() {
-        fn create_run_script(content: &str) -> anyhow::Result<NamedTempFile> {
-            let rwx = std::fs::Permissions::from_mode(0o777);
-            let mut script_file = tempfile::Builder::new()
-                .suffix(".sh")
-                .permissions(rwx)
-                .disable_cleanup(true)
-                .tempfile()?;
-            script_file.write_all(content.as_bytes())?;
-
-            Ok(script_file)
-        }
-
-        let mut tmp_dst = tempfile::NamedTempFile::new().unwrap();
-
-        let pre_script = create_run_script(&format!(
-            "#!/usr/bin/env bash\necho \"pre\" >> {}",
-            tmp_dst.path().display()
-        ))
-        .unwrap();
-        let post_script = create_run_script(&format!(
-            "#!/usr/bin/env bash\necho \"post\" >> {}",
-            tmp_dst.path().display()
-        ))
-        .unwrap();
-
-        {
-            let _guard =
-                HookScriptsGuard::setup_with_scripts(pre_script.path(), post_script.path());
-        }
-
-        let mut result = String::new();
-        tmp_dst.read_to_string(&mut result).unwrap();
-        assert_eq!(result, "pre\npost\n");
-    }
-}