mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 08:04:25 +00:00
fix: use tini with unshare to preserve signals (e.g oom) (#7471)
* use tini with unshare to preserve signals (e.g oom) * fix ci * ci as nsjail * simplify * fix flaky go integration test --------- Co-authored-by: Ruben Fiszel <ruben@windmill.dev>
This commit is contained in:
committed by
GitHub
co-authored by
Ruben Fiszel
parent
51c7faf881
commit
eaca36f4b6
@@ -593,9 +593,10 @@ jobs:
|
||||
${{ steps.meta-ee-public.outputs.labels }}
|
||||
|
||||
build_ee_slim:
|
||||
if: ${{ startsWith(github.ref, 'refs/tags/v') }} || ((github.event_name != 'workflow_dispatch') || (github.event.inputs.slim))
|
||||
needs: [build_ee]
|
||||
runs-on: ubicloud
|
||||
if: (github.event_name != 'pull_request') && ((github.event_name != 'workflow_dispatch') || (github.event.inputs.ee || github.event.inputs.slim))
|
||||
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
|
||||
+1
-1
@@ -129,7 +129,7 @@ ENV PATH /usr/local/bin:/root/.local/bin:/tmp/.local/bin:$PATH
|
||||
|
||||
|
||||
RUN apt-get update \
|
||||
&& apt-get install -y --no-install-recommends netbase tzdata ca-certificates wget curl jq unzip build-essential unixodbc xmlsec1 software-properties-common \
|
||||
&& apt-get install -y --no-install-recommends netbase tzdata ca-certificates wget curl jq unzip build-essential unixodbc xmlsec1 software-properties-common tini \
|
||||
&& apt-get clean \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
|
||||
@@ -642,7 +642,19 @@ pub fn build_command_with_isolation(program: &str, args: &[&str]) -> Command {
|
||||
cmd.arg(flag);
|
||||
}
|
||||
|
||||
cmd.arg("--");
|
||||
// If tini is available, use it for proper PID 1 signal handling
|
||||
// (especially OOM exit codes which return 137 instead of sigprocmask errors).
|
||||
// Note: --fork should already be in the flags for proper namespace setup.
|
||||
if let Some(tini_path) = crate::TINI_AVAILABLE.as_ref() {
|
||||
cmd.arg("--");
|
||||
cmd.arg(tini_path);
|
||||
cmd.arg("-s");
|
||||
cmd.arg("--");
|
||||
} else {
|
||||
// Without tini, just run the command directly (--fork is in flags)
|
||||
cmd.arg("--");
|
||||
}
|
||||
|
||||
cmd.arg(program);
|
||||
cmd.args(args);
|
||||
cmd
|
||||
|
||||
@@ -315,16 +315,73 @@ lazy_static::lazy_static! {
|
||||
.and_then(|x| x.parse::<bool>().ok())
|
||||
.unwrap_or(false);
|
||||
|
||||
pub static ref UNSHARE_TINI_PATH: String = {
|
||||
std::env::var("UNSHARE_TINI_PATH").unwrap_or_else(|_| "tini".to_string())
|
||||
};
|
||||
|
||||
// --fork is required for unshare to work with --pid --mount-proc.
|
||||
// When tini is available, it runs as PID 1 inside the forked namespace for proper signal handling.
|
||||
pub static ref UNSHARE_ISOLATION_FLAGS: String = {
|
||||
std::env::var("UNSHARE_ISOLATION_FLAGS")
|
||||
.unwrap_or_else(|_| "--user --map-root-user --pid --fork --mount-proc".to_string())
|
||||
};
|
||||
|
||||
// Check if tini is available for proper PID 1 handling in unshare namespaces.
|
||||
// tini handles OOM signals correctly, returning exit code 137 instead of sigprocmask errors.
|
||||
pub static ref TINI_AVAILABLE: Option<String> = {
|
||||
let tini_path = UNSHARE_TINI_PATH.as_str();
|
||||
let test_result = std::process::Command::new(tini_path)
|
||||
.args(["-s", "--", "true"])
|
||||
.output();
|
||||
|
||||
match test_result {
|
||||
Ok(output) if output.status.success() => {
|
||||
tracing::info!("tini available at: {}", tini_path);
|
||||
Some(tini_path.to_string())
|
||||
}
|
||||
Ok(output) => {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
tracing::warn!(
|
||||
"tini test failed: {}. Proceeding without tini (OOM exit codes may be incorrect).",
|
||||
stderr.trim()
|
||||
);
|
||||
None
|
||||
}
|
||||
Err(e) => {
|
||||
if e.kind() == std::io::ErrorKind::NotFound {
|
||||
tracing::warn!(
|
||||
"tini not found at '{}'. Install tini for correct OOM exit codes, or set UNSHARE_TINI_PATH.",
|
||||
tini_path
|
||||
);
|
||||
} else {
|
||||
tracing::warn!(
|
||||
"Failed to test tini: {}. Proceeding without tini.",
|
||||
e
|
||||
);
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
pub static ref UNSHARE_PATH: Option<String> = {
|
||||
let flags = UNSHARE_ISOLATION_FLAGS.as_str();
|
||||
let mut test_cmd_args: Vec<&str> = flags.split_whitespace().collect();
|
||||
test_cmd_args.push("--");
|
||||
test_cmd_args.push("true");
|
||||
|
||||
// Build the test command based on whether tini is available
|
||||
// Note: --fork should already be in the flags for proper namespace setup
|
||||
if let Some(tini_path) = TINI_AVAILABLE.as_ref() {
|
||||
// Test with tini: unshare <flags> -- tini -s -- true
|
||||
test_cmd_args.push("--");
|
||||
test_cmd_args.push(tini_path.as_str());
|
||||
test_cmd_args.push("-s");
|
||||
test_cmd_args.push("--");
|
||||
test_cmd_args.push("true");
|
||||
} else {
|
||||
// Fallback without tini: unshare <flags> -- true
|
||||
test_cmd_args.push("--");
|
||||
test_cmd_args.push("true");
|
||||
}
|
||||
|
||||
let test_result = std::process::Command::new("unshare")
|
||||
.args(&test_cmd_args)
|
||||
@@ -332,7 +389,11 @@ lazy_static::lazy_static! {
|
||||
|
||||
match test_result {
|
||||
Ok(output) if output.status.success() => {
|
||||
tracing::info!("PID namespace isolation enabled. Flags: {}", flags);
|
||||
if TINI_AVAILABLE.is_some() {
|
||||
tracing::info!("PID namespace isolation enabled with tini. Flags: {}", flags);
|
||||
} else {
|
||||
tracing::info!("PID namespace isolation enabled. Flags: {}", flags);
|
||||
}
|
||||
Some("unshare".to_string())
|
||||
},
|
||||
Ok(output) => {
|
||||
@@ -1140,12 +1201,6 @@ pub async fn run_worker(
|
||||
if *ENABLE_UNSHARE_PID {
|
||||
// Access UNSHARE_PATH to trigger lazy_static initialization and test
|
||||
let _ = &*UNSHARE_PATH;
|
||||
|
||||
tracing::info!(
|
||||
worker = %worker_name, hostname = %hostname,
|
||||
"PID namespace isolation enabled via unshare with flags: {}",
|
||||
UNSHARE_ISOLATION_FLAGS.as_str()
|
||||
);
|
||||
}
|
||||
|
||||
let start_time = Instant::now();
|
||||
|
||||
Reference in New Issue
Block a user