From f61cf1633b0f6a9056f8a5ec22f3951ffe0c072c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 4 Jun 2026 14:11:20 +0000 Subject: [PATCH] refactor(worker): per-job podman for all docker jobs, drop the shared service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalize the per-job rootless podman instance to every docker job (any sandbox mode) instead of only nsjail, and remove the long-lived shared service: - Drop start_container_runtime_maybe() + its worker-startup call + the global DOCKER_HOST mutation. Each docker job now spins up its own ephemeral per-job podman (start_per_job_podman) and tears it down at job end — so no container outlives any job, and there's no standing shared socket on the worker. - per_job_podman is gated strictly on container_runtime=podman. The legacy path (externally-provided DOCKER_HOST or a mounted /var/run/docker.sock, without container_runtime=podman) is unchanged: connect_docker(None)/forwarded env, and Windmill only manages the $WM_JOB_ID container as before. - docker_host handed to the script: the in-jail /tmp/podman.sock under nsjail (bind-mounted), else the per-job host socket directly (unshare/none). - The "no Docker daemon reachable" hint no longer fires when container_runtime=podman (per-job provides the daemon). - monitor: drop the container_runtime killpill — with no startup service the config is read live per job, no worker restart needed on change. Verified e2e against rootless podman: docker jobs work + confine extra containers under nsjail AND unshare; legacy host-socket jobs run against the host daemon with no per-job podman and extras persisting (unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/src/monitor.rs | 5 -- backend/windmill-worker/src/bash_executor.rs | 37 +++++--- backend/windmill-worker/src/worker.rs | 91 -------------------- 3 files changed, 25 insertions(+), 108 deletions(-) diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 5567eaae11..14403533a7 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -3005,11 +3005,6 @@ pub async fn reload_worker_config(db: &DB, tx: KillpillSender, kill_if_change: b let _ = tx.send(); } - if wc.container_runtime != config.container_runtime { - tracing::info!("Container runtime config changed, sending killpill. Expecting to be restarted by supervisor."); - let _ = tx.send(); - } - if wc.cache_clear != config.cache_clear { tracing::info!("Cache clear changed, sending killpill. Expecting to be restarted by supervisor."); let _ = tx.send(); diff --git a/backend/windmill-worker/src/bash_executor.rs b/backend/windmill-worker/src/bash_executor.rs index 2835304f3b..4fdbc8f840 100644 --- a/backend/windmill-worker/src/bash_executor.rs +++ b/backend/windmill-worker/src/bash_executor.rs @@ -98,9 +98,16 @@ pub async fn handle_bash_job( logs1.push_str("docker mode\n"); // If neither DOCKER_HOST nor the host socket is available, the docker CLI // in the script would fail with a generic "cannot connect to the Docker - // daemon" error. Surface a Windmill-specific hint instead. + // daemon" error. Surface a Windmill-specific hint instead. Skipped when + // container_runtime=podman is set: a per-job rootless podman is started + // below and provides the daemon. if std::env::var("DOCKER_HOST").is_err() && !std::path::Path::new("/var/run/docker.sock").exists() + && windmill_common::worker::WORKER_CONFIG + .load() + .container_runtime + .as_deref() + != Some("podman") { logs1.push_str( "WARNING: docker mode is set but no Docker daemon is reachable from this worker \ @@ -207,13 +214,14 @@ exit $exit_status // Use nsjail if globally enabled OR if script has #sandbox annotation let nsjail = (is_sandboxing_enabled() || annotation.sandbox) && is_regular_job; - // Under nsjail (untrusted), a docker job gets its OWN rootless podman instance so - // no container it spawns can outlive the job — it is torn down when this guard - // drops at the end of the function. Trusted (unshare) jobs keep using the shared - // worker DOCKER_HOST. Only applies when container_runtime=podman is configured. + // Every docker job gets its OWN ephemeral rootless podman instance (started here, + // torn down when this guard drops at the end of the function) so no container it + // spawns can outlive the job, in any sandbox mode. Gated strictly on + // container_runtime=podman: legacy docker access (an externally-provided + // DOCKER_HOST or a mounted /var/run/docker.sock, without container_runtime=podman) + // is left exactly as before. #[cfg(feature = "dind")] let per_job_podman: Option = if annotation.docker - && nsjail && windmill_common::worker::WORKER_CONFIG .load() .container_runtime @@ -225,14 +233,19 @@ exit $exit_status None }; - // In-jail socket path the script's docker CLI connects to (bind-mounted from the - // per-job podman host socket via {DOCKER_SOCK_MOUNT} below). + // DOCKER_HOST handed to the script's docker CLI when using per-job podman: under + // nsjail the per-job socket is bind-mounted into the jail at /tmp/podman.sock + // (via {DOCKER_SOCK_MOUNT}); otherwise the script reaches the host socket directly. let docker_host_for_script: Option = { #[cfg(feature = "dind")] { - per_job_podman - .as_ref() - .map(|_| "unix:///tmp/podman.sock".to_string()) + per_job_podman.as_ref().map(|p| { + if nsjail { + "unix:///tmp/podman.sock".to_string() + } else { + p.docker_host.clone() + } + }) } #[cfg(not(feature = "dind"))] { @@ -241,7 +254,7 @@ exit $exit_status }; // Forward DOCKER_HOST to the bash script in docker mode: the per-job podman - // socket under nsjail, else the shared DOCKER_HOST / dind sidecar. + // socket when container_runtime=podman, else the legacy DOCKER_HOST / docker socket. let docker_envs: Vec<(&str, String)> = if annotation.docker { if let Some(dh) = &docker_host_for_script { vec![("DOCKER_HOST", dh.clone())] diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 97dbb12b6f..7fb0dee9f5 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -2244,10 +2244,6 @@ pub async fn run_worker( ) = (HashMap::new(), vec![]); if i_worker == 1 { - // Start the configured container runtime (rootless podman) before any job - // runs, so docker-mode jobs have a Docker-API-compatible daemon and - // DOCKER_HOST is set process-wide. No-op unless container_runtime is set. - start_container_runtime_maybe().await; // Initialize runtime asset inserter for batched database inserts if let Connection::Sql(db) = conn { init_runtime_asset_loop(db.clone(), killpill_rx.resubscribe()); @@ -3239,93 +3235,6 @@ pub async fn run_worker( tracing::info!(worker = %worker_name, hostname = %hostname, "number of jobs executed: {}", jobs_executed); } -// Holds the managed podman service child so it is not dropped (and thus not -// reaped) for the lifetime of the worker process. -static PODMAN_SERVICE: std::sync::Mutex> = - std::sync::Mutex::new(None); - -/// Start the container runtime configured on this worker group (currently only -/// rootless "podman") so docker-mode jobs have a Docker-API-compatible daemon to -/// talk to — without a privileged dind sidecar or the host Docker socket. Spawns -/// a rootless `podman system service` on a unix socket and points DOCKER_HOST at -/// it, which both `connect_docker()` (bollard) and the docker CLI forwarded to -/// scripts then use. No-op unless `container_runtime` is set on the group. -async fn start_container_runtime_maybe() { - let runtime = match WORKER_CONFIG.load().container_runtime.clone() { - Some(r) => r, - None => return, - }; - if runtime != "podman" { - tracing::error!( - "Unknown container_runtime '{runtime}' (expected 'podman'); not starting a container runtime." - ); - return; - } - // Respect an explicitly-provided DOCKER_HOST (e.g. a legacy dind/host socket). - if std::env::var("DOCKER_HOST").is_ok() { - tracing::info!( - "container_runtime=podman but DOCKER_HOST is already set; using the existing daemon and not starting the managed podman runtime." - ); - return; - } - let podman_ok = tokio::process::Command::new("podman") - .arg("--version") - .output() - .await - .map(|o| o.status.success()) - .unwrap_or(false); - if !podman_ok { - tracing::error!( - "container_runtime=podman but the 'podman' binary was not found on this worker. Use a windmill *-full image, or install podman via a worker-group init script. Docker-mode jobs will fail until podman is available." - ); - return; - } - - let sock_dir = "/tmp/windmill"; - let _ = tokio::fs::create_dir_all(sock_dir).await; - let sock_path = format!("{sock_dir}/podman.sock"); - let _ = tokio::fs::remove_file(&sock_path).await; // clear any stale socket - let docker_host = format!("unix://{sock_path}"); - - match tokio::process::Command::new("podman") - .args(["system", "service", "--time=0", &docker_host]) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .spawn() - { - Ok(child) => { - if let Ok(mut guard) = PODMAN_SERVICE.lock() { - *guard = Some(child); - } - // Wait (up to ~10s) for the rootless podman service socket to appear - // so the first docker job doesn't race startup. - let mut ready = false; - for _ in 0..50 { - if tokio::fs::metadata(&sock_path).await.is_ok() { - ready = true; - break; - } - tokio::time::sleep(std::time::Duration::from_millis(200)).await; - } - std::env::set_var("DOCKER_HOST", &docker_host); - if ready { - tracing::info!( - "Started rootless podman container runtime, DOCKER_HOST={docker_host}" - ); - } else { - tracing::warn!( - "Started rootless podman container runtime but socket {sock_path} did not appear within 10s; DOCKER_HOST set to {docker_host} anyway." - ); - } - } - Err(e) => { - tracing::error!( - "Failed to start podman container runtime: {e:#}. Docker-mode jobs will fail." - ); - } - } -} - async fn queue_init_bash_maybe<'c>( conn: &Connection, same_worker_tx: SameWorkerSender,