From 34ed80d33c973f660338572824998022f5b22385 Mon Sep 17 00:00:00 2001 From: Evie Howard Date: Fri, 4 Sep 2026 12:08:43 +0100 Subject: [PATCH] refactor(cli): remove local Dockerfile image builds Signed-off-by: Evie Howard --- .../skills/launch-openshell-gator/SKILL.md | 4 +- Cargo.lock | 3 - README.md | 10 +- crates/openshell-bootstrap/Cargo.toml | 5 - crates/openshell-bootstrap/src/build.rs | 677 ------------------ .../openshell-bootstrap/src/build_windows.rs | 23 - crates/openshell-bootstrap/src/lib.rs | 5 - crates/openshell-cli/src/main.rs | 29 +- crates/openshell-cli/src/run.rs | 341 ++++----- crates/openshell-core/src/driver_utils.rs | 11 + crates/openshell-driver-docker/src/lib.rs | 22 +- crates/openshell-driver-docker/src/tests.rs | 32 +- crates/openshell-driver-podman/src/watcher.rs | 24 + crates/openshell-driver-vm/README.md | 6 +- crates/openshell-sandbox/src/main.rs | 20 +- deploy/rpm/TROUBLESHOOTING.md | 8 +- docs/reference/sandbox-compute-drivers.mdx | 2 +- docs/sandboxes/manage-sandboxes.mdx | 28 +- e2e/rust/src/harness/container.rs | 69 ++ e2e/rust/tests/custom_image.rs | 30 +- e2e/rust/tests/driver_config_volume.rs | 63 +- e2e/rust/tests/live_policy_update.rs | 26 +- examples/bring-your-own-container/README.md | 20 +- rfc/0013-native-windows-mxc/README.md | 3 +- scripts/agents/README.md | 7 +- scripts/agents/gator/README.md | 9 +- scripts/agents/run.sh | 33 +- skills/openshell-cli/SKILL.md | 13 +- 28 files changed, 450 insertions(+), 1073 deletions(-) delete mode 100644 crates/openshell-bootstrap/src/build.rs delete mode 100644 crates/openshell-bootstrap/src/build_windows.rs diff --git a/.agents/skills/launch-openshell-gator/SKILL.md b/.agents/skills/launch-openshell-gator/SKILL.md index fb9ada09b0..763dc779a2 100644 --- a/.agents/skills/launch-openshell-gator/SKILL.md +++ b/.agents/skills/launch-openshell-gator/SKILL.md @@ -26,7 +26,7 @@ For gator's PR/issue validation policy, load `gator-gate` inside the launched sa |---|---| | `scripts/agents/run.sh` | Manifest-driven OpenShell agent launcher. | | `scripts/agents/gator/agent.yaml` | Gator manifest: immutable payload version, default gateway, harness, providers, runtime, skills, and subagents. | -| `scripts/agents/gator/Dockerfile` | Gator sandbox image source. Local launches build this image through OpenShell. | +| `scripts/agents/gator/Dockerfile` | Gator sandbox image source. Local launches build it in gateway's Docker or Podman image store. | | `scripts/agents/gator/policy.yaml` | Sandbox policy for the gator agent. | | `scripts/agents/gator/bin/gh` | Gator-specific `gh` wrapper and same-SHA duplicate-post guard. | | `scripts/agents/gator/bin/review-feedback-ledger` | Builds tree-aware review scope, durable findings, convergence telemetry, and review-budget state. | @@ -160,7 +160,7 @@ sandbox_name="gator-pr-${pr_number}-supervised" "Review and monitor PR #${pr_number} through the gator-gate workflow. Scope this invocation only to PR #${pr_number}." ``` -The launcher builds the gator sandbox image when needed, stages the immutable payload, imports provider profiles, configures provider credentials and refresh, creates and uploads the sandbox payload, then starts the agent supervisor with `sandbox exec`. It writes a background log under `scripts/agents/gator/logs/`. +The launcher queries gateway's selected compute driver, builds gator image in matching Docker or Podman image store, stages immutable payload, imports provider profiles, configures provider credentials and refresh, creates and uploads sandbox payload, then starts agent supervisor with `sandbox exec`. It writes a background log under `scripts/agents/gator/logs/`. `CONTAINER_ENGINE`, when set, must match gateway driver. ### Launch An Issue Or Issue/PR Pair diff --git a/Cargo.lock b/Cargo.lock index c713ace9ce..fdd622b55c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3763,16 +3763,13 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" name = "openshell-bootstrap" version = "0.0.0" dependencies = [ - "bollard", "bytes", - "futures", "miette", "openshell-core", "rcgen", "serde", "serde_json", "sha2 0.10.9", - "tar", "tempfile", "tokio", "tracing", diff --git a/README.md b/README.md index a7ecf4179e..af54a8540e 100644 --- a/README.md +++ b/README.md @@ -196,14 +196,20 @@ The TUI gives you a live, keyboard-driven view of your gateway and sandboxes. Na ## Community Sandboxes and BYOC -Use `--from` to create sandboxes from the [OpenShell Community](https://github.com/NVIDIA/OpenShell-Community) catalog, a local directory, or a container image: +Use `--from` to create sandboxes from the [OpenShell Community](https://github.com/NVIDIA/OpenShell-Community) catalog or a container image: ```bash openshell sandbox create --from gemini # community catalog -openshell sandbox create --from ./my-sandbox-dir # local Dockerfile +docker build -t my-sandbox:latest ./my-sandbox-dir # Docker gateway +openshell sandbox create --from my-sandbox:latest # Docker built image +podman build -t localhost/my-sandbox:latest ./my-sandbox-dir # Podman gateway +openshell sandbox create --from localhost/my-sandbox:latest # Podman built image openshell sandbox create --from registry.io/img:v1 # container image ``` +Build with the container engine used by your local gateway. For a remote +gateway, push the image to a registry that the gateway can pull from. + See the [OpenShell Community](https://github.com/NVIDIA/OpenShell-Community) catalog and the [BYOC example](https://github.com/NVIDIA/OpenShell/tree/main/examples/bring-your-own-container) for details. ## Use OpenShell with Your Agent diff --git a/crates/openshell-bootstrap/Cargo.toml b/crates/openshell-bootstrap/Cargo.toml index 96a7985085..435024e302 100644 --- a/crates/openshell-bootstrap/Cargo.toml +++ b/crates/openshell-bootstrap/Cargo.toml @@ -12,20 +12,15 @@ rust-version.workspace = true [dependencies] openshell-core = { path = "../openshell-core", default-features = false } bytes = { workspace = true } -futures = { workspace = true } miette = { workspace = true } rcgen = { workspace = true } sha2 = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } -tar = "0.4" tempfile = "3" tokio = { workspace = true } tracing = { workspace = true } -[target.'cfg(not(target_os = "windows"))'.dependencies] -bollard = "0.20" - [dev-dependencies] [lints] diff --git a/crates/openshell-bootstrap/src/build.rs b/crates/openshell-bootstrap/src/build.rs deleted file mode 100644 index f0e03fcd89..0000000000 --- a/crates/openshell-bootstrap/src/build.rs +++ /dev/null @@ -1,677 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -//! Build container images for sandbox runtimes. -//! -//! This module wraps bollard's `build_image()` API to build a container image -//! from a Dockerfile and build context. Package-managed local gateways use the -//! host Docker daemon, so the resulting tag is passed to the gateway directly. - -use std::collections::HashMap; -use std::path::Path; -use std::time::Duration; - -use bollard::Docker; -use bollard::query_parameters::BuildImageOptionsBuilder; -use futures::StreamExt; -use miette::{IntoDiagnostic, Result, WrapErr}; -use tokio::time::timeout; - -/// Maximum gap between Docker build stream events before a build is treated -/// as stuck. -/// -/// Total silence longer than this on under-provisioned container runtimes -/// (e.g. default Colima 2 vCPU / 2 GiB on macOS) reliably indicates a -/// deadlocked builder that will never recover. The default leaves headroom -/// for legitimately quiet steps (a single long `RUN` that produces no output) -/// — override with `OPENSHELL_BUILD_NO_PROGRESS_TIMEOUT_SECS` if a specific -/// build needs more time, or shorter for CI tightening. -const DEFAULT_BUILD_NO_PROGRESS_TIMEOUT_SECS: u64 = 1800; - -async fn docker_daemon_platform(docker: &Docker) -> Result { - let info = docker - .info() - .await - .into_diagnostic() - .wrap_err("failed to query local Docker daemon info")?; - let os = info - .os_type - .as_deref() - .filter(|value| !value.is_empty()) - .ok_or_else(|| miette::miette!("Docker daemon did not report an operating system"))?; - let arch = info - .architecture - .as_deref() - .filter(|value| !value.is_empty()) - .ok_or_else(|| miette::miette!("Docker daemon did not report an architecture"))?; - - normalize_docker_platform(os, arch) -} - -fn normalize_docker_platform(os: &str, arch: &str) -> Result { - // Remote or non-standard daemons may return mixed-case arch strings. - let arch_lower = arch.to_lowercase(); - let normalized_arch = match arch_lower.as_str() { - "amd64" | "x86_64" => "amd64", - "arm64" | "aarch64" => "arm64", - "arm" | "armv7" | "armv7l" | "armhf" => "arm/v7", - "armv6" | "armv6l" => "arm/v6", - "386" | "i386" | "i686" => "386", - "ppc64le" => "ppc64le", - "s390x" => "s390x", - _ => { - return Err(miette::miette!( - "unsupported Docker daemon architecture for local image build: {arch}" - )); - } - }; - Ok(format!("{os}/{normalized_arch}")) -} - -/// Parse `os/arch[/variant]` and reject malformed empty segments. -fn parse_platform(platform: &str) -> Result<(&str, &str, &str)> { - let mut parts = platform.split('/'); - let os = parts - .next() - .filter(|v| !v.is_empty()) - .ok_or_else(|| miette::miette!("platform is missing an operating system"))?; - let arch = parts - .next() - .filter(|v| !v.is_empty()) - .ok_or_else(|| miette::miette!("platform is missing an architecture"))?; - // A trailing slash creates a malformed empty variant segment. - let variant = match parts.next() { - Some("") => { - return Err(miette::miette!( - "platform variant must not be empty (trailing slash?): '{platform}'" - )); - } - Some(v) => v, - None => "", - }; - if parts.next().is_some() { - return Err(miette::miette!( - "platform must be os/arch[/variant], got '{platform}'" - )); - } - Ok((os, arch, variant)) -} - -/// Seed Docker's implicit `BuildKit` platform args without replacing caller values. -/// -/// `target_platform` identifies the output image. `build_platform` identifies -/// the Docker daemon doing the build. Variant args are only injected when the -/// platform string has an explicit variant, which preserves Dockerfile ARG -/// defaults for no-variant platforms. -fn insert_implicit_platform_build_args( - build_args: &mut HashMap, - target_platform: &str, - build_platform: &str, -) -> Result<()> { - let (target_os, target_arch, target_variant) = parse_platform(target_platform) - .wrap_err_with(|| format!("invalid target platform '{target_platform}'"))?; - let (build_os, build_arch, build_variant) = parse_platform(build_platform) - .wrap_err_with(|| format!("invalid build platform '{build_platform}'"))?; - - for (key, val) in [ - ("TARGETPLATFORM", target_platform), - ("TARGETOS", target_os), - ("TARGETARCH", target_arch), - ("BUILDPLATFORM", build_platform), - ("BUILDOS", build_os), - ("BUILDARCH", build_arch), - ] { - build_args - .entry(key.to_string()) - .or_insert_with(|| val.to_string()); - } - if !target_variant.is_empty() { - build_args - .entry("TARGETVARIANT".to_string()) - .or_insert_with(|| target_variant.to_string()); - } - if !build_variant.is_empty() { - build_args - .entry("BUILDVARIANT".to_string()) - .or_insert_with(|| build_variant.to_string()); - } - - Ok(()) -} - -/// Build a container image from a Dockerfile using the local Docker daemon. -/// -/// This is used by `openshell sandbox create --from `. The image -/// remains available in the local Docker daemon so the gateway's active local -/// compute driver can resolve the tag. -#[allow(clippy::implicit_hasher)] -pub async fn build_local_image( - dockerfile_path: &Path, - tag: &str, - context_dir: &Path, - build_args: &HashMap, - on_log: &mut impl FnMut(String), -) -> Result<()> { - on_log(format!( - "Building image {tag} from {}", - dockerfile_path.display() - )); - build_image(dockerfile_path, tag, context_dir, build_args, on_log).await?; - on_log(format!("Built image {tag}")); - Ok(()) -} - -/// Build a container image using the local Docker daemon. -/// -/// Creates a tar archive of `context_dir`, sends it to Docker with the -/// specified Dockerfile path and tag, and streams build output to `on_log`. -async fn build_image( - dockerfile_path: &Path, - tag: &str, - context_dir: &Path, - build_args: &HashMap, - on_log: &mut impl FnMut(String), -) -> Result<()> { - let docker = Docker::connect_with_local_defaults() - .into_diagnostic() - .wrap_err("failed to connect to local Docker daemon")?; - // BUILD* always comes from daemon info. A TARGETPLATFORM override only - // changes the output image platform. - let build_platform = docker_daemon_platform(&docker).await?; - let target_platform = build_args - .get("TARGETPLATFORM") - .map_or_else(|| build_platform.clone(), Clone::clone); - let mut effective_build_args = build_args.clone(); - insert_implicit_platform_build_args( - &mut effective_build_args, - &target_platform, - &build_platform, - )?; - - // Compute the relative path of the Dockerfile within the context. - let dockerfile_relative = dockerfile_path - .strip_prefix(context_dir) - .unwrap_or(dockerfile_path); - let dockerfile_str = dockerfile_relative - .to_str() - .ok_or_else(|| miette::miette!("Dockerfile path is not valid UTF-8"))?; - - // Create a tar archive of the build context, respecting .dockerignore. - let context_tar = create_build_context_tar(context_dir)?; - - let mut builder = BuildImageOptionsBuilder::default() - .dockerfile(dockerfile_str) - .t(tag) - .rm(true) - .platform(&target_platform); - - // Pass build args to Docker. - if !effective_build_args.is_empty() { - builder = builder.buildargs(&effective_build_args); - } - - let options = builder.build(); - - let body = bollard::body_full(bytes::Bytes::from(context_tar)); - let mut stream = docker.build_image(options, None, Some(body)); - let no_progress_secs: u64 = std::env::var("OPENSHELL_BUILD_NO_PROGRESS_TIMEOUT_SECS") - .ok() - .and_then(|s| s.parse().ok()) - .filter(|&n| n > 0) - .unwrap_or(DEFAULT_BUILD_NO_PROGRESS_TIMEOUT_SECS); - let no_progress_timeout = Duration::from_secs(no_progress_secs); - - loop { - let next = match timeout(no_progress_timeout, stream.next()).await { - Ok(Some(result)) => result, - Ok(None) => break, - Err(_) => { - return Err(miette::miette!( - "Docker build produced no output for {}s. This usually means the container \ - runtime is under-provisioned (CPU/memory) and the builder has deadlocked; \ - check `docker info` (NCPU, MemTotal) and increase Colima/Docker Desktop \ - resources before retrying. If a legitimate build step is just quiet, raise \ - the threshold with OPENSHELL_BUILD_NO_PROGRESS_TIMEOUT_SECS=.", - no_progress_timeout.as_secs() - )); - } - }; - - let info = next - .into_diagnostic() - .wrap_err("Docker build stream error")?; - - // Forward build output lines. - if let Some(stream_line) = &info.stream { - let trimmed = stream_line.trim_end(); - if !trimmed.is_empty() { - on_log(trimmed.to_string()); - } - } - - // Check for build errors. - if let Some(error_detail) = &info.error_detail { - let msg = error_detail - .message - .as_deref() - .unwrap_or("unknown build error"); - return Err(miette::miette!("Docker build failed: {msg}")); - } - } - - Ok(()) -} - -/// Create a tar archive of a directory for use as a Docker build context. -/// -/// Walks `context_dir` recursively, respects a `.dockerignore` file if present, -/// and adds matching files with paths relative to the context root. -fn create_build_context_tar(context_dir: &Path) -> Result> { - let ignore_patterns = load_dockerignore(context_dir); - - let mut builder = tar::Builder::new(Vec::new()); - - // Walk the directory tree and add entries, skipping ignored paths. - walk_and_add(context_dir, context_dir, &ignore_patterns, &mut builder)?; - - builder - .into_inner() - .into_diagnostic() - .wrap_err("failed to finalize build context tar") -} - -/// Recursively walk a directory and add entries to a tar archive, -/// skipping paths that match `.dockerignore` patterns. -fn walk_and_add( - root: &Path, - current: &Path, - ignore_patterns: &[IgnorePattern], - builder: &mut tar::Builder>, -) -> Result<()> { - let entries = std::fs::read_dir(current) - .into_diagnostic() - .wrap_err_with(|| format!("failed to read directory: {}", current.display()))?; - - for entry in entries { - let entry = entry - .into_diagnostic() - .wrap_err("failed to read directory entry")?; - let path = entry.path(); - let relative = path - .strip_prefix(root) - .unwrap_or(&path) - .to_string_lossy() - .to_string(); - - // Normalize to forward slashes for pattern matching. - let relative_normalized = relative.replace('\\', "/"); - - if is_ignored(&relative_normalized, path.is_dir(), ignore_patterns) { - continue; - } - - if path.is_dir() { - walk_and_add(root, &path, ignore_patterns, builder)?; - } else { - // Use append_path_with_name which handles GNU LongName extensions - // for paths exceeding 100 bytes (the POSIX tar name field limit). - builder - .append_path_with_name(&path, &relative_normalized) - .into_diagnostic() - .wrap_err_with(|| format!("failed to add file to tar: {relative_normalized}"))?; - } - } - - Ok(()) -} - -/// A parsed `.dockerignore` pattern. -#[derive(Debug, Clone)] -struct IgnorePattern { - /// The glob pattern (may contain `*`, `**`, `?`). - pattern: String, - /// Whether this is a negation pattern (starts with `!`). - negated: bool, -} - -/// Load and parse a `.dockerignore` file from the context directory. -/// -/// Returns an empty list if no `.dockerignore` exists. -fn load_dockerignore(context_dir: &Path) -> Vec { - let dockerignore_path = context_dir.join(".dockerignore"); - let Ok(contents) = std::fs::read_to_string(&dockerignore_path) else { - return Vec::new(); - }; - - contents - .lines() - .map(str::trim) - .filter(|line| !line.is_empty() && !line.starts_with('#')) - .map(|line| { - line.strip_prefix('!').map_or_else( - || IgnorePattern { - pattern: line.to_string(), - negated: false, - }, - |rest| IgnorePattern { - pattern: rest.trim().to_string(), - negated: true, - }, - ) - }) - .collect() -} - -/// Check whether a relative path should be ignored based on `.dockerignore` patterns. -/// -/// Uses a simple glob-matching approach: patterns are matched against the -/// full relative path. A leading `/` anchors to the context root. The last -/// matching pattern wins (negation patterns re-include files). -fn is_ignored(relative_path: &str, is_dir: bool, patterns: &[IgnorePattern]) -> bool { - let mut ignored = false; - - for pat in patterns { - let pattern = pat.pattern.trim_start_matches('/'); - - // Check if the pattern matches. - let matches = glob_match(pattern, relative_path, is_dir); - - if matches { - ignored = !pat.negated; - } - } - - ignored -} - -/// Simple glob matching supporting `*`, `**`, and `?`. -/// -/// This is intentionally simple — it covers the common `.dockerignore` cases -/// without pulling in a full glob crate. For complex patterns, Docker's own -/// builder handles them during the build step anyway; this is just for -/// reducing the context tar size. -fn glob_match(pattern: &str, path: &str, is_dir: bool) -> bool { - // Handle ** prefix (match any number of directories) - if let Some(rest) = pattern.strip_prefix("**/") { - // Match against the path itself and any suffix after a / - if glob_match(rest, path, is_dir) { - return true; - } - for (idx, _) in path.match_indices('/') { - if let Some(suffix) = path.get(idx + 1..) - && glob_match(rest, suffix, is_dir) - { - return true; - } - } - return false; - } - - // Handle pattern that is just a name (no slashes) — match against any - // path component or as a prefix directory match. - if !pattern.contains('/') { - // Match the final component of the path. - let basename = path.rsplit('/').next().unwrap_or(path); - if simple_glob_match(pattern, basename) { - return true; - } - // Also match as a directory prefix: pattern "node_modules" should - // match "node_modules/foo/bar". - if let Some(first) = path.split('/').next() - && simple_glob_match(pattern, first) - { - return true; - } - return false; - } - - // Pattern contains slashes — match against the full path. - simple_glob_match(pattern, path) || (is_dir && path.starts_with(pattern.trim_end_matches('/'))) -} - -/// Match a simple glob pattern (with `*` and `?` but not `**`) against a string. -fn simple_glob_match(pattern: &str, text: &str) -> bool { - let mut p_star: Option = None; - let mut t_star: Option = None; - - let p_bytes: Vec = pattern.chars().collect(); - let t_bytes: Vec = text.chars().collect(); - let mut pi = 0; - let mut ti = 0; - - while ti < t_bytes.len() { - if pi < p_bytes.len() && (p_bytes[pi] == '?' || p_bytes[pi] == t_bytes[ti]) { - pi += 1; - ti += 1; - } else if pi < p_bytes.len() && p_bytes[pi] == '*' { - p_star = Some(pi); - t_star = Some(ti); - pi += 1; - } else if let Some(ps) = p_star { - pi = ps + 1; - t_star = Some(t_star.unwrap() + 1); - ti = t_star.unwrap(); - } else { - return false; - } - } - - while pi < p_bytes.len() && p_bytes[pi] == '*' { - pi += 1; - } - - pi == p_bytes.len() -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - - #[test] - fn test_create_build_context_tar() { - let dir = tempfile::tempdir().unwrap(); - let dir_path = dir.path(); - - // Create a simple Dockerfile and a file. - fs::write(dir_path.join("Dockerfile"), "FROM ubuntu:24.04\n").unwrap(); - fs::write(dir_path.join("hello.txt"), "hello world\n").unwrap(); - fs::create_dir(dir_path.join("subdir")).unwrap(); - fs::write(dir_path.join("subdir/nested.txt"), "nested\n").unwrap(); - - let tar_bytes = create_build_context_tar(dir_path).unwrap(); - assert!(!tar_bytes.is_empty()); - - // Verify the tar contains the expected entries. - let mut archive = tar::Archive::new(tar_bytes.as_slice()); - let entries: Vec = archive - .entries() - .unwrap() - .filter_map(std::result::Result::ok) - .map(|e| e.path().unwrap().to_string_lossy().to_string()) - .collect(); - - assert!(entries.iter().any(|e| e.contains("Dockerfile"))); - assert!(entries.iter().any(|e| e.contains("hello.txt"))); - assert!(entries.iter().any(|e| e.contains("subdir/nested.txt"))); - } - - #[test] - fn test_dockerignore_excludes_files() { - let dir = tempfile::tempdir().unwrap(); - let dir_path = dir.path(); - - fs::write(dir_path.join("Dockerfile"), "FROM ubuntu:24.04\n").unwrap(); - fs::write(dir_path.join("hello.txt"), "hello\n").unwrap(); - fs::write(dir_path.join("secret.env"), "SECRET=foo\n").unwrap(); - fs::create_dir(dir_path.join("node_modules")).unwrap(); - fs::write(dir_path.join("node_modules/pkg.js"), "module\n").unwrap(); - fs::write(dir_path.join(".dockerignore"), "*.env\nnode_modules\n").unwrap(); - - let tar_bytes = create_build_context_tar(dir_path).unwrap(); - let mut archive = tar::Archive::new(tar_bytes.as_slice()); - let entries: Vec = archive - .entries() - .unwrap() - .filter_map(std::result::Result::ok) - .map(|e| e.path().unwrap().to_string_lossy().to_string()) - .collect(); - - assert!(entries.iter().any(|e| e.contains("Dockerfile"))); - assert!(entries.iter().any(|e| e.contains("hello.txt"))); - assert!(!entries.iter().any(|e| e.contains("secret.env"))); - assert!(!entries.iter().any(|e| e.contains("node_modules"))); - } - - #[test] - fn test_dockerignore_negation() { - let dir = tempfile::tempdir().unwrap(); - let dir_path = dir.path(); - - fs::write(dir_path.join("Dockerfile"), "FROM ubuntu:24.04\n").unwrap(); - fs::write(dir_path.join("a.log"), "log\n").unwrap(); - fs::write(dir_path.join("important.log"), "keep\n").unwrap(); - fs::write(dir_path.join(".dockerignore"), "*.log\n!important.log\n").unwrap(); - - let tar_bytes = create_build_context_tar(dir_path).unwrap(); - let mut archive = tar::Archive::new(tar_bytes.as_slice()); - let entries: Vec = archive - .entries() - .unwrap() - .filter_map(std::result::Result::ok) - .map(|e| e.path().unwrap().to_string_lossy().to_string()) - .collect(); - - assert!(!entries.iter().any(|e| e.contains("a.log"))); - assert!(entries.iter().any(|e| e.contains("important.log"))); - } - - #[test] - fn test_long_path_exceeding_100_bytes() { - let dir = tempfile::tempdir().unwrap(); - let dir_path = dir.path(); - - // Build a nested path that exceeds 100 bytes when relative to root. - let deep_dir = dir_path.join( - "a/deeply/nested/directory/path/that/exceeds/one/hundred/bytes/total/from/the/build/context/root", - ); - fs::create_dir_all(&deep_dir).unwrap(); - fs::write(deep_dir.join("file.txt"), "deep content\n").unwrap(); - fs::write(dir_path.join("Dockerfile"), "FROM ubuntu:24.04\n").unwrap(); - - let tar_bytes = create_build_context_tar(dir_path).unwrap(); - let mut archive = tar::Archive::new(tar_bytes.as_slice()); - let entries: Vec = archive - .entries() - .unwrap() - .filter_map(std::result::Result::ok) - .map(|e| e.path().unwrap().to_string_lossy().to_string()) - .collect(); - - let long_entry = entries.iter().find(|e| e.contains("file.txt")); - assert!( - long_entry.is_some(), - "tar should contain deeply nested file; entries: {entries:?}" - ); - assert!( - long_entry.unwrap().len() > 100, - "path should exceed 100 bytes to exercise GNU LongName handling" - ); - } - - #[test] - fn test_simple_glob_match() { - assert!(simple_glob_match("*.txt", "hello.txt")); - assert!(!simple_glob_match("*.txt", "hello.rs")); - assert!(simple_glob_match("test?", "test1")); - assert!(!simple_glob_match("test?", "test12")); - assert!(simple_glob_match("*", "anything")); - assert!(simple_glob_match("foo*bar", "fooXYZbar")); - } - - #[test] - fn test_glob_match_double_star() { - assert!(glob_match("**/*.log", "a/b/c.log", false)); - assert!(glob_match("**/*.log", "c.log", false)); - assert!(!glob_match("**/*.log", "c.txt", false)); - assert!(glob_match("**/föö.log", "a/b/föö.log", false)); - } - - #[test] - fn test_is_ignored_directory_prefix() { - let patterns = vec![IgnorePattern { - pattern: "node_modules".to_string(), - negated: false, - }]; - assert!(is_ignored("node_modules", true, &patterns)); - assert!(is_ignored("node_modules/foo.js", false, &patterns)); - } - - #[test] - fn test_normalize_docker_platform_maps_docker_desktop_arch() { - assert_eq!( - normalize_docker_platform("linux", "aarch64").unwrap(), - "linux/arm64" - ); - assert_eq!( - normalize_docker_platform("linux", "x86_64").unwrap(), - "linux/amd64" - ); - } - - #[test] - fn test_normalize_docker_platform_case_insensitive_arch() { - // Some remote or non-standard Docker daemons return mixed-case arch strings. - assert_eq!( - normalize_docker_platform("linux", "ARM64").unwrap(), - "linux/arm64" - ); - assert_eq!( - normalize_docker_platform("linux", "X86_64").unwrap(), - "linux/amd64" - ); - } - - #[test] - fn test_insert_implicit_platform_build_args_native() { - // Native builds use one platform for both target and build args. - let mut build_args = HashMap::new(); - insert_implicit_platform_build_args(&mut build_args, "linux/arm/v7", "linux/arm/v7") - .unwrap(); - - assert_eq!(build_args.get("TARGETPLATFORM").unwrap(), "linux/arm/v7"); - assert_eq!(build_args.get("TARGETOS").unwrap(), "linux"); - assert_eq!(build_args.get("TARGETARCH").unwrap(), "arm"); - assert_eq!(build_args.get("TARGETVARIANT").unwrap(), "v7"); - assert_eq!(build_args.get("BUILDPLATFORM").unwrap(), "linux/arm/v7"); - assert_eq!(build_args.get("BUILDOS").unwrap(), "linux"); - assert_eq!(build_args.get("BUILDARCH").unwrap(), "arm"); - assert_eq!(build_args.get("BUILDVARIANT").unwrap(), "v7"); - } - - #[test] - fn test_insert_implicit_platform_build_args_cross_compile() { - // Cross-builds keep daemon BUILD* args separate from target args. - let mut build_args = HashMap::new(); - insert_implicit_platform_build_args(&mut build_args, "linux/arm64", "linux/amd64").unwrap(); - - assert_eq!(build_args.get("TARGETPLATFORM").unwrap(), "linux/arm64"); - assert_eq!(build_args.get("TARGETARCH").unwrap(), "arm64"); - assert_eq!(build_args.get("BUILDPLATFORM").unwrap(), "linux/amd64"); - assert_eq!(build_args.get("BUILDARCH").unwrap(), "amd64"); - // Leaving variant args absent preserves Dockerfile ARG defaults. - assert!(!build_args.contains_key("TARGETVARIANT")); - assert!(!build_args.contains_key("BUILDVARIANT")); - } - - #[test] - fn test_insert_implicit_platform_build_args_trailing_slash_rejected() { - let mut build_args = HashMap::new(); - let result = - insert_implicit_platform_build_args(&mut build_args, "linux/arm64/", "linux/amd64"); - assert!( - result.is_err(), - "trailing slash should be rejected as a malformed variant segment" - ); - } -} diff --git a/crates/openshell-bootstrap/src/build_windows.rs b/crates/openshell-bootstrap/src/build_windows.rs deleted file mode 100644 index 93af1345d3..0000000000 --- a/crates/openshell-bootstrap/src/build_windows.rs +++ /dev/null @@ -1,23 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -//! Windows stub for local Dockerfile image builds. - -use std::collections::HashMap; -use std::path::Path; - -use miette::Result; - -// Keep this stub's signature aligned with the supported-platform implementation. -#[allow(clippy::implicit_hasher)] -pub async fn build_local_image( - _dockerfile_path: &Path, - _tag: &str, - _context_dir: &Path, - _build_args: &HashMap, - _on_log: &mut impl FnMut(String), -) -> Result<()> { - Err(miette::miette!( - "local Dockerfile sandbox sources are unsupported on Windows" - )) -} diff --git a/crates/openshell-bootstrap/src/lib.rs b/crates/openshell-bootstrap/src/lib.rs index 5598d524ee..36ab7a8c73 100644 --- a/crates/openshell-bootstrap/src/lib.rs +++ b/crates/openshell-bootstrap/src/lib.rs @@ -1,11 +1,6 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -#[cfg(not(target_os = "windows"))] -pub mod build; -#[cfg(target_os = "windows")] -#[path = "build_windows.rs"] -pub mod build; pub mod edge_token; pub mod jwt; pub mod oidc_token; diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 142a520068..a07083862c 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1363,18 +1363,17 @@ enum SandboxCommands { #[arg(long, conflicts_with_all = ["from", "gpu", "cpu", "memory", "driver_config_json", "envs"])] template: Option, - /// Sandbox source: a community sandbox name (e.g., `ollama`), a path - /// to a Dockerfile or directory containing one, a rootfs tar archive - /// (`.tar`, `.tar.gz`, or `.tgz`), or a full container image reference - /// (e.g., `myregistry.com/img:tag`). + /// Sandbox source: a community sandbox name (e.g., `ollama`), a rootfs + /// tar archive (`.tar`, `.tar.gz`, or `.tgz`), or a full container + /// image reference (e.g., `myregistry.com/img:tag`). /// /// Community names are resolved to /// `ghcr.io/nvidia/openshell-community/sandboxes/:latest` /// (override the prefix with `OPENSHELL_COMMUNITY_REGISTRY`). /// - /// When given a Dockerfile or directory, the image is built into the - /// local Docker daemon before creating the sandbox. When given a - /// rootfs tar, it is passed directly to the VM compute driver. + /// To use a local Dockerfile, build and tag it with the container + /// engine used by your local gateway, then pass the resulting image + /// reference here. A rootfs tar is staged for the VM compute driver. #[arg(long, value_hint = ValueHint::AnyPath)] from: Option, @@ -4239,11 +4238,6 @@ mod tests { 4, "Dockerfile", ), - ( - vec!["openshell", "sandbox", "create", "--from", "Do"], - 4, - "Dockerfile", - ), ( vec![ "openshell", @@ -4469,20 +4463,12 @@ mod tests { } #[test] - fn sandbox_create_and_download_use_path_value_hints() { + fn sandbox_download_uses_path_value_hint() { let cmd = Cli::command(); let sandbox = cmd .get_subcommands() .find(|c| c.get_name() == "sandbox") .expect("missing sandbox subcommand"); - let create = sandbox - .get_subcommands() - .find(|c| c.get_name() == "create") - .expect("missing create subcommand"); - let from = create - .get_arguments() - .find(|arg| arg.get_id() == "from") - .expect("missing from argument"); let download = sandbox .get_subcommands() .find(|c| c.get_name() == "download") @@ -4492,7 +4478,6 @@ mod tests { .find(|arg| arg.get_id() == "dest") .expect("missing dest argument"); - assert_eq!(from.get_value_hint(), ValueHint::AnyPath); assert_eq!(dest.get_value_hint(), ValueHint::AnyPath); } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 18c8ee7366..3326ead90c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -525,10 +525,9 @@ pub async fn sandbox_create( )); } - // Resolve the --from flag into a container image reference, building from - // a Dockerfile first if necessary, or staging a rootfs tar on the gateway - // and carrying back its staging token. Template creates resolve workload - // shape on the gateway and skip local image handling. + // Resolve the --from flag into a container image reference, or stage a + // rootfs tar on the gateway and retain its staging token. Template creates + // resolve workload shape on the gateway and skip --from. let (image, rootfs_tar_token): (Option, Option) = if template.is_some() { (None, None) } else { @@ -537,14 +536,6 @@ pub async fn sandbox_create( let resolved = resolve_from(val)?; match resolved { ResolvedSource::Image(img) => (Some(img), None), - ResolvedSource::Dockerfile { - dockerfile, - context, - } => { - let tag = - build_from_dockerfile(&dockerfile, &context, gateway_name).await?; - (Some(tag), None) - } ResolvedSource::RootfsTar { path } => { let token = stage_rootfs_tar(gateway_name, &mut client, workspace, &path).await?; @@ -1157,118 +1148,66 @@ pub async fn sandbox_create( enum ResolvedSource { /// A ready-to-use container image reference. Image(String), - /// A Dockerfile that must be built before creating the sandbox. - Dockerfile { - dockerfile: PathBuf, - context: PathBuf, - }, - /// A flat rootfs tar archive (`.tar`, `.tar.gz`, `.tgz`) to pass directly - /// to the VM compute driver. + /// A flat rootfs tar archive (`.tar`, `.tar.gz`, `.tgz`) to stage for the + /// VM compute driver. RootfsTar { path: PathBuf }, } -/// Classify the `--from` value into an image reference, a Dockerfile that -/// needs building, or a rootfs tar to pass to the VM driver. +/// Classify the `--from` value into an image reference or a rootfs tar to stage +/// for the VM driver. /// /// Resolution order: -/// 1. Existing file whose name contains "dockerfile" → build from Dockerfile. -/// 2. Existing directory that contains a `Dockerfile` → build from directory. -/// 3. Existing file with `.tar`, `.tar.gz`, or `.tgz` extension → rootfs tar archive. -/// 4. Other existing local paths → error. -/// 5. Non-existent path-like values (`./…`, `../…`, `/…`, `~/…`) → local -/// error, so they don't reach the gateway as broken image-pull requests. -/// 6. Value contains `/`, `:`, or `.` → treat as a full image reference. -/// 7. Otherwise → community sandbox name, expanded via the registry prefix. +/// 1. Existing file with `.tar`, `.tar.gz`, or `.tgz` extension → rootfs tar archive. +/// 2. Local Dockerfile and directory paths → an actionable build-and-tag error. +/// 3. Other explicit local paths → an actionable error. +/// 4. Full image reference or community sandbox name → resolve as an image. fn resolve_from(value: &str) -> Result { let path = Path::new(value); - // 1. Existing file that looks like a Dockerfile. - if path.is_file() { - if filename_looks_like_dockerfile(path) { - let dockerfile = path - .canonicalize() - .into_diagnostic() - .wrap_err_with(|| format!("failed to resolve path: {}", path.display()))?; - let context = dockerfile - .parent() - .ok_or_else(|| miette::miette!("Dockerfile has no parent directory"))? - .to_path_buf(); - return Ok(ResolvedSource::Dockerfile { - dockerfile, - context, - }); - } - - if filename_looks_like_rootfs_tar(path) { - let tar_path = path - .canonicalize() - .into_diagnostic() - .wrap_err_with(|| format!("failed to resolve path: {}", path.display()))?; - return Ok(ResolvedSource::RootfsTar { path: tar_path }); - } + if path.is_file() && filename_looks_like_rootfs_tar(path) { + let tar_path = path + .canonicalize() + .into_diagnostic() + .wrap_err_with(|| format!("failed to resolve path: {}", path.display()))?; + return Ok(ResolvedSource::RootfsTar { path: tar_path }); + } - if value_looks_like_local_source(value) { + if value_looks_like_local_path(value) { + if !path.exists() && filename_looks_like_rootfs_tar(path) { return Err(miette::miette!( - "local --from file is not a Dockerfile or rootfs tar (.tar/.tar.gz/.tgz): {}", + "local --from path does not exist: {}", path.display() )); } - } - - // 2. Existing directory containing a Dockerfile. - if path.is_dir() { - let candidate = path.join("Dockerfile"); - if candidate.is_file() { - let context = path - .canonicalize() - .into_diagnostic() - .wrap_err_with(|| format!("failed to resolve path: {}", path.display()))?; - let dockerfile = context.join("Dockerfile"); - return Ok(ResolvedSource::Dockerfile { - dockerfile, - context, - }); - } - return Err(miette::miette!( - "No Dockerfile found in directory: {}", - path.display() - )); - } - if path.exists() { - return Err(miette::miette!( - "local --from path is not a regular file or directory: {}", - path.display() - )); - } - - // 3. Missing explicit local paths should fail locally. Otherwise values - // like `./Dockerfile` reach the gateway as image references and fail as - // Docker pull errors. - if value_looks_like_local_source(value) { + let build_context = if path.is_dir() { + path.display().to_string() + } else { + path.parent() + .map(|p| p.display().to_string()) + .filter(|p| !p.is_empty()) + .unwrap_or_else(|| ".".to_string()) + }; return Err(miette::miette!( - "local --from path does not exist: {}\n\ - Use an existing Dockerfile, directory containing Dockerfile, rootfs tar (.tar/.tar.gz/.tgz), or a container image reference.", - path.display() + "'--from' no longer builds local Dockerfiles or directories: {}\n\ + Build and tag the image with the container engine used by the gateway, then pass the resulting image reference:\n \ + docker build -t {} # Docker gateway\n \ + podman build -t {} # Podman gateway\n \ + openshell sandbox create --from \n\ + If the gateway cannot access the selected engine's local image store (for example, a remote or Kubernetes gateway), push the image to a registry that the gateway can pull from.", + path.display(), + build_context, + build_context, )); } - // 4. Full image reference or community sandbox name — delegate to shared - // resolution in openshell-core. + // Full image reference or community sandbox name — delegate to shared + // resolution in openshell-core. Ok(ResolvedSource::Image( openshell_core::image::resolve_community_image(value), )) } -fn filename_looks_like_dockerfile(path: &Path) -> bool { - let name = path - .file_name() - .map(|n| n.to_string_lossy()) - .unwrap_or_default(); - let lower = name.to_lowercase(); - lower.contains("dockerfile") -} - #[allow(clippy::case_sensitive_file_extension_comparisons)] // already lowercased fn filename_looks_like_rootfs_tar(path: &Path) -> bool { let name = path @@ -1279,85 +1218,28 @@ fn filename_looks_like_rootfs_tar(path: &Path) -> bool { lower.ends_with(".tar.gz") || lower.ends_with(".tar") || lower.ends_with(".tgz") } -fn value_looks_like_local_source(value: &str) -> bool { - value_is_explicit_local_path(value) || value_looks_like_bare_dockerfile_name(value) -} - -fn value_is_explicit_local_path(value: &str) -> bool { +fn value_looks_like_local_path(value: &str) -> bool { let path = Path::new(value); path.is_absolute() || matches!(value, "." | "..") || value.starts_with("./") || value.starts_with("../") || value.starts_with("~/") + || value_looks_like_bare_dockerfile_name(value) } fn value_looks_like_bare_dockerfile_name(value: &str) -> bool { - !value.contains('/') && !value.contains(':') && filename_looks_like_dockerfile(Path::new(value)) + !value.contains('/') + && !value.contains(':') + && Path::new(value) + .file_name() + .is_some_and(|name| name.to_string_lossy().to_lowercase().contains("dockerfile")) } -fn dockerfile_sources_supported_for_gateway(metadata: Option<&GatewayMetadata>) -> bool { +fn rootfs_tar_sources_supported_for_gateway(metadata: Option<&GatewayMetadata>) -> bool { !metadata.is_some_and(|metadata| metadata.is_remote) } -/// Build a Dockerfile and return the local Docker tag. -/// -/// Package-managed local gateways use the same Docker daemon that the CLI -/// builds into, so the tag is passed through directly and the active compute -/// driver resolves it. -async fn build_from_dockerfile( - dockerfile: &Path, - context: &Path, - gateway_name: &str, -) -> Result { - let metadata = get_gateway_metadata(gateway_name); - if !dockerfile_sources_supported_for_gateway(metadata.as_ref()) { - return Err(miette!( - "local Dockerfile sources are only supported for local gateways; gateway '{}' is remote", - gateway_name - )); - } - - let timestamp = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .unwrap_or_default() - .as_secs(); - let tag = format!("openshell/sandbox-from:{timestamp}"); - - eprintln!( - "Building image {} from {}", - tag.cyan(), - dockerfile.display() - ); - eprintln!(" {} {}", "Context:".dimmed(), context.display()); - eprintln!(" {} {}", "Gateway:".dimmed(), gateway_name); - eprintln!(); - - let mut on_log = |msg: String| { - eprintln!(" {msg}"); - }; - - openshell_bootstrap::build::build_local_image( - dockerfile, - &tag, - context, - &HashMap::new(), - &mut on_log, - ) - .await?; - - eprintln!(); - eprintln!( - "{} Image {} is available in the local Docker daemon for gateway '{}'.", - "✓".green().bold(), - tag.cyan(), - gateway_name, - ); - eprintln!(); - - Ok(tag) -} - /// Ask the gateway for a staging slot, then copy the archive into it. /// /// The gateway owns the destination: it allocates a request-scoped directory @@ -1371,7 +1253,7 @@ async fn stage_rootfs_tar( tar_path: &Path, ) -> Result { let metadata = get_gateway_metadata(gateway_name); - if !dockerfile_sources_supported_for_gateway(metadata.as_ref()) { + if !rootfs_tar_sources_supported_for_gateway(metadata.as_ref()) { return Err(miette!( "local rootfs tar sources are only supported for local gateways; gateway '{}' is remote", gateway_name @@ -6155,25 +6037,26 @@ fn format_endpoint(endpoint: &openshell_core::proto::NetworkEndpoint) -> String #[cfg(test)] mod tests { use super::{ - PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, - dockerfile_sources_supported_for_gateway, format_endpoint, format_log_line, git_sync_files, - has_main_process_result, parse_cli_setting_value, parse_credential_expiry_cli_value, - parse_driver_config_json, parse_secret_material_env_pairs, policy_revision_list_json, - policy_revision_to_json, provisioning_timeout_message, ready_false_condition_message, - resolve_from, sandbox_should_persist, sandbox_upload_plan, service_endpoint_to_json, - service_expose_status_error, service_url_for_gateway, workspace_member_to_json, + PolicyGetView, ProvisioningStep, build_sandbox_resource_limits, format_endpoint, + format_log_line, git_sync_files, has_main_process_result, parse_cli_setting_value, + parse_credential_expiry_cli_value, parse_driver_config_json, + parse_secret_material_env_pairs, policy_revision_list_json, policy_revision_to_json, + provisioning_timeout_message, ready_false_condition_message, resolve_from, + rootfs_tar_sources_supported_for_gateway, sandbox_should_persist, sandbox_upload_plan, + service_endpoint_to_json, service_expose_status_error, service_url_for_gateway, + workspace_member_to_json, }; use crate::TEST_ENV_LOCK; use crate::commands::common::{ parse_credential_expiry_pairs, parse_credential_pairs, progress_step_from_metadata, }; use crate::test_utils::EnvVarGuard; + use openshell_bootstrap::GatewayMetadata; use std::fs; use std::path::Path; use std::process::Command; use tonic::Status; - use openshell_bootstrap::GatewayMetadata; use openshell_core::progress::{ PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX, @@ -6676,58 +6559,86 @@ mod tests { } #[test] - fn resolve_from_classifies_existing_dockerfile_path() { + fn resolve_from_rejects_existing_dockerfile_path() { let temp = tempfile::tempdir().expect("failed to create tempdir"); let dockerfile = temp.path().join("Dockerfile"); fs::write(&dockerfile, "FROM scratch\n").expect("failed to write Dockerfile"); - match resolve_from(dockerfile.to_str().expect("temp path is not UTF-8")) - .expect("expected Dockerfile source") - { - super::ResolvedSource::Dockerfile { - dockerfile: resolved, - context, - } => { - assert_eq!( - resolved, - dockerfile - .canonicalize() - .expect("failed to canonicalize Dockerfile") - ); - assert_eq!( - context, - temp.path() - .canonicalize() - .expect("failed to canonicalize context") - ); - } - other => { - panic!("expected Dockerfile source, got {other:?}"); - } - } + let err = resolve_from(dockerfile.to_str().expect("temp path is not UTF-8")) + .expect_err("expected local Dockerfile path to be rejected"); + + assert!( + err.to_string() + .contains("no longer builds local Dockerfiles or directories"), + "unexpected error: {err}" + ); + assert!( + err.to_string().contains("docker build -t "), + "expected actionable build guidance: {err}" + ); + assert!( + err.to_string().contains("podman build -t "), + "expected Podman build guidance: {err}" + ); } #[test] - fn resolve_from_rejects_missing_explicit_dockerfile_path() { + fn resolve_from_rejects_missing_explicit_local_path() { let temp = tempfile::tempdir().expect("failed to create tempdir"); let missing = temp.path().join("Dockerfile"); let err = resolve_from(missing.to_str().expect("temp path is not UTF-8")) - .expect_err("expected missing Dockerfile path to be rejected"); + .expect_err("expected missing explicit local path to be rejected"); assert!( - err.to_string().contains("local --from path does not exist"), + err.to_string() + .contains("no longer builds local Dockerfiles or directories"), "unexpected error: {err}" ); } + #[test] + fn resolve_from_rejects_bare_dockerfile_name() { + let err = resolve_from("Dockerfile").expect_err("expected bare Dockerfile to be rejected"); + + assert!( + err.to_string() + .contains("no longer builds local Dockerfiles or directories"), + "unexpected error: {err}" + ); + } + + #[test] + fn resolve_from_keeps_bare_community_name_when_local_directory_matches() { + let _lock = TEST_ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let temp = tempfile::tempdir().expect("create tempdir"); + fs::create_dir(temp.path().join("python")).expect("create matching directory"); + let original_dir = std::env::current_dir().expect("read current directory"); + std::env::set_current_dir(temp.path()).expect("enter tempdir"); + + let result = resolve_from("python"); + + std::env::set_current_dir(original_dir).expect("restore current directory"); + match result.expect("bare community name should not be a local path") { + super::ResolvedSource::Image(image) => assert_eq!( + image, + "ghcr.io/nvidia/openshell-community/sandboxes/python:latest" + ), + other @ super::ResolvedSource::RootfsTar { .. } => { + panic!("expected image source, got {other:?}"); + } + } + } + #[test] fn resolve_from_keeps_dockerfile_named_image_refs_as_images() { let image_ref = "ghcr.io/acme/dockerfile-runner:latest"; match resolve_from(image_ref).expect("expected image source") { super::ResolvedSource::Image(image) => assert_eq!(image, image_ref), - other => { + other @ super::ResolvedSource::RootfsTar { .. } => { panic!("expected image ref, got {other:?}"); } } @@ -6750,7 +6661,9 @@ mod tests { .expect("failed to canonicalize archive") ); } - other => panic!("expected RootfsTar source, got {other:?}"), + other @ super::ResolvedSource::Image(_) => { + panic!("expected RootfsTar source, got {other:?}"); + } } } @@ -6771,7 +6684,9 @@ mod tests { .expect("failed to canonicalize archive") ); } - other => panic!("expected RootfsTar source, got {other:?}"), + other @ super::ResolvedSource::Image(_) => { + panic!("expected RootfsTar source, got {other:?}"); + } } } @@ -6792,7 +6707,9 @@ mod tests { .expect("failed to canonicalize archive") ); } - other => panic!("expected RootfsTar source, got {other:?}"), + other @ super::ResolvedSource::Image(_) => { + panic!("expected RootfsTar source, got {other:?}"); + } } } @@ -6909,7 +6826,7 @@ mod tests { } #[test] - fn dockerfile_sources_are_rejected_for_remote_gateways() { + fn rootfs_tar_sources_are_rejected_for_remote_gateways() { let metadata = GatewayMetadata { name: "remote".to_string(), gateway_endpoint: "https://gateway.example.com".to_string(), @@ -6924,11 +6841,11 @@ mod tests { ..Default::default() }; - assert!(!dockerfile_sources_supported_for_gateway(Some(&metadata))); + assert!(!rootfs_tar_sources_supported_for_gateway(Some(&metadata))); } #[test] - fn dockerfile_sources_are_allowed_for_local_gateways() { + fn rootfs_tar_sources_are_allowed_for_local_gateways() { let metadata = GatewayMetadata { name: "local".to_string(), gateway_endpoint: "http://127.0.0.1:8080".to_string(), @@ -6943,8 +6860,8 @@ mod tests { ..Default::default() }; - assert!(dockerfile_sources_supported_for_gateway(Some(&metadata))); - assert!(dockerfile_sources_supported_for_gateway(None)); + assert!(rootfs_tar_sources_supported_for_gateway(Some(&metadata))); + assert!(rootfs_tar_sources_supported_for_gateway(None)); } #[test] diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index 74871751da..ad1f42db6c 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -52,6 +52,17 @@ pub fn openshell_sandbox_label_selector() -> String { /// being relaunched. pub const CONDITION_EXITED: &str = "ContainerExited"; +/// Ready-condition reason when the supervisor rejects an image-provided OCI +/// working directory because the sandbox identity lacks the required access. +pub const CONDITION_WORKSPACE_VALIDATION_FAILED: &str = "WorkspaceValidationFailed"; + +/// Supervisor exit status reserved for OCI workspace validation failures. +/// +/// Local container drivers translate this status into +/// [`CONDITION_WORKSPACE_VALIDATION_FAILED`] so users receive the specific +/// provisioning failure rather than a generic container exit. +pub const SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED: i32 = 78; + /// Ready-condition reason when a container was terminated by an external signal. /// /// SIGKILL/SIGTERM (exit 137/143) is what a Podman/Docker machine or daemon diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index df5aee4750..4566d5c171 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -24,8 +24,9 @@ use futures::{Stream, StreamExt}; use openshell_core::config::{DEFAULT_SANDBOX_PIDS_LIMIT, DEFAULT_STOP_TIMEOUT_SECS}; use openshell_core::driver_mounts; use openshell_core::driver_utils::{ - CONDITION_EXITED, CONDITION_RUNTIME_RESTART, LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, - LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE, + CONDITION_EXITED, CONDITION_RUNTIME_RESTART, CONDITION_WORKSPACE_VALIDATION_FAILED, + LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, + LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE, SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED, SUPERVISOR_IMAGE_BINARY_PATH, extract_first_tar_entry, supervisor_image_should_refresh, temp_extract_container_name, validate_linux_elf_binary, write_cache_binary_atomic, }; @@ -3496,8 +3497,10 @@ fn driver_status_from_summary( /// Refine an exited Docker sandbox's `Ready` condition from inspected state. /// -/// A signal kill (exit 137/143 = SIGKILL/SIGTERM, not OOM) is the signature of -/// a machine/daemon restart terminating a running container. Reclassify it from +/// A workspace-validation exit is reported distinctly so users can repair the +/// OCI working directory rather than diagnose a generic crash. A signal kill +/// (exit 137/143 = SIGKILL/SIGTERM, not OOM) is the signature of a +/// machine/daemon restart terminating a running container. Reclassify it from /// the generic terminal `ContainerExited` to the recoverable /// `ContainerRuntimeRestart` so gateway startup can revive it. OOM kills and /// ordinary application exits stay `ContainerExited` and terminal. @@ -3505,7 +3508,7 @@ fn apply_docker_exit_classification(sandbox: &mut DriverSandbox, state: &Contain if state.oom_killed == Some(true) { return; } - let Some(code) = state.exit_code.filter(|&code| matches!(code, 137 | 143)) else { + let Some(code) = state.exit_code else { return; }; let Some(condition) = sandbox @@ -3518,8 +3521,13 @@ fn apply_docker_exit_classification(sandbox: &mut DriverSandbox, state: &Contain if condition.reason != CONDITION_EXITED { return; } - condition.reason = CONDITION_RUNTIME_RESTART.to_string(); - condition.message = format!("Container terminated by signal (exit code {code})"); + if code == i64::from(SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED) { + condition.reason = CONDITION_WORKSPACE_VALIDATION_FAILED.to_string(); + condition.message = "OCI WorkingDir is not usable by the sandbox identity".to_string(); + } else if matches!(code, 137 | 143) { + condition.reason = CONDITION_RUNTIME_RESTART.to_string(); + condition.message = format!("Container terminated by signal (exit code {code})"); + } } fn container_ready_condition( diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index aff5ba17f8..010d50c20e 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -4,8 +4,9 @@ use super::*; use openshell_core::config::DEFAULT_SERVER_PORT; use openshell_core::driver_utils::{ - LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, - LABEL_SANDBOX_NAMESPACE, supervisor_cache_path_with_base, + CONDITION_WORKSPACE_VALIDATION_FAILED, LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, + LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, LABEL_SANDBOX_NAMESPACE, + SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED, supervisor_cache_path_with_base, }; use openshell_core::progress::{ PROGRESS_ACTIVE_DETAIL_KEY, PROGRESS_ACTIVE_STEP_KEY, PROGRESS_COMPLETE_LABEL_KEY, @@ -3300,6 +3301,15 @@ fn ready_reason(sandbox: &DriverSandbox) -> &str { .expect("Ready condition present") } +fn ready_message(sandbox: &DriverSandbox) -> &str { + sandbox + .status + .as_ref() + .and_then(|status| status.conditions.iter().find(|c| c.r#type == "Ready")) + .map(|c| c.message.as_str()) + .expect("Ready condition present") +} + #[test] fn docker_signal_kill_reclassified_as_runtime_restart() { // 137 (128+SIGKILL) and 143 (128+SIGTERM) mark an external termination — @@ -3337,6 +3347,24 @@ fn docker_ordinary_exit_stays_terminal() { assert_eq!(ready_reason(&sandbox), CONDITION_EXITED); } +#[test] +fn docker_workspace_validation_exit_is_reported_explicitly() { + let mut sandbox = exited_sandbox_with_ready_reason(CONDITION_EXITED); + let state = ContainerState { + status: Some(ContainerStateStatusEnum::EXITED), + exit_code: Some(i64::from(SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED)), + ..Default::default() + }; + + apply_docker_exit_classification(&mut sandbox, &state); + + assert_eq!( + ready_reason(&sandbox), + CONDITION_WORKSPACE_VALIDATION_FAILED + ); + assert!(ready_message(&sandbox).contains("WorkingDir")); +} + #[test] fn docker_oom_kill_stays_terminal_despite_137() { // An OOM kill reports exit 137 but must NOT be treated as a recoverable diff --git a/crates/openshell-driver-podman/src/watcher.rs b/crates/openshell-driver-podman/src/watcher.rs index c3903a0067..57ea45e6e4 100644 --- a/crates/openshell-driver-podman/src/watcher.rs +++ b/crates/openshell-driver-podman/src/watcher.rs @@ -28,6 +28,7 @@ const CONDITION_RUNNING: &str = "ContainerRunning"; const CONDITION_STARTING: &str = "ContainerStarting"; use openshell_core::driver_utils::{ CONDITION_EXITED, CONDITION_RUNTIME_RESTART, CONDITION_STOPPED, + CONDITION_WORKSPACE_VALIDATION_FAILED, SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED, }; pub type WatchStream = @@ -453,6 +454,11 @@ fn condition_from_state(state: &ContainerState) -> DriverCondition { "OOMKilled", "Container was killed by the OOM killer".to_string(), ) + } else if state.exit_code == i64::from(SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED) { + ( + CONDITION_WORKSPACE_VALIDATION_FAILED, + "OCI WorkingDir is not usable by the sandbox identity".to_string(), + ) } else if matches!(state.exit_code, 137 | 143) { ( CONDITION_RUNTIME_RESTART, @@ -612,6 +618,24 @@ mod tests { assert!(cond.message.contains("code 1")); } + #[test] + fn condition_workspace_validation_exit_is_reported_explicitly() { + let state = ContainerState { + status: "exited".to_string(), + running: false, + exit_code: i64::from(SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED), + oom_killed: false, + health: None, + started_at: None, + finished_at: Some("2026-04-14T12:00:00Z".to_string()), + }; + + let cond = condition_from_state(&state); + + assert_eq!(cond.reason, CONDITION_WORKSPACE_VALIDATION_FAILED); + assert!(cond.message.contains("WorkingDir")); + } + #[test] fn condition_signal_kill_is_runtime_restart() { // 137 (128+SIGKILL) and 143 (128+SIGTERM) are external terminations — diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index 5c61ae1823..05e6df454b 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -262,9 +262,9 @@ Each table is created atomically via `nft -f` on VM start and torn down atomical - Matching rustup target: `rustup target add aarch64-unknown-linux-gnu` (or `x86_64-unknown-linux-gnu` for an amd64 guest) - `cargo install --locked cargo-zigbuild` and `brew install zig` (or distro equivalent). `vm:supervisor` uses `cargo zigbuild` to cross-compile the in-VM `openshell-sandbox` supervisor binary. - [mise](https://mise.jdx.dev/) task runner -- Docker or Podman socket on the local CLI/gateway host when using - `openshell sandbox create --from ./Dockerfile` or `--from ./dir`; the CLI - builds the image and the VM driver exports it via the local container engine. +- Docker or Podman socket on the local CLI/gateway host when building an image + before `openshell sandbox create --from `; the VM driver exports the + image via the local container engine. Docker is tried first; if unavailable, the driver falls back to the Podman socket. On Linux, enable the Podman API socket with `systemctl --user start podman.socket` diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 1ad69e1070..e67b9b48a7 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -10,7 +10,7 @@ use std::sync::atomic::AtomicBool; use clap::Parser; use miette::{IntoDiagnostic, Result}; use openshell_ocsf::{OcsfJsonlLayer, OcsfShorthandLayer}; -use tracing::{info, warn}; +use tracing::{error, info, warn}; use tracing_subscriber::EnvFilter; use tracing_subscriber::filter::LevelFilter; use tracing_subscriber::{Layer, layer::SubscriberExt, util::SubscriberInitExt}; @@ -577,7 +577,7 @@ fn main() -> Result<()> { .build() .into_diagnostic()?; - let exit_code = runtime.block_on(async move { + let result = runtime.block_on(async move { // Install rustls crypto provider before any TLS connections (including log push). let _ = rustls::crypto::ring::default_provider().install_default(); @@ -725,7 +725,21 @@ fn main() -> Result<()> { upstream_proxy_args, ) .await - })?; + }); + + let exit_code = match result { + Ok(exit_code) => exit_code, + Err(error) + if error + .to_string() + .contains("image workspace validation failed") => + { + error!(%error, "Image workspace validation failed"); + eprintln!("{error:?}"); + openshell_core::driver_utils::SUPERVISOR_EXIT_WORKSPACE_VALIDATION_FAILED + } + Err(error) => return Err(error), + }; std::process::exit(exit_code); } diff --git a/deploy/rpm/TROUBLESHOOTING.md b/deploy/rpm/TROUBLESHOOTING.md index 103ce3bf9d..5ea6f24bf5 100644 --- a/deploy/rpm/TROUBLESHOOTING.md +++ b/deploy/rpm/TROUBLESHOOTING.md @@ -47,13 +47,11 @@ systemd commands directly: ### Building from local Dockerfiles -`openshell sandbox create --from ./Dockerfile` builds via the local -Docker daemon. With the RPM Podman driver, build the image with Podman -and reference it directly: +Build the image with Podman, then reference it directly: ```shell -podman build -t my-sandbox ./my-dir -openshell sandbox create --from localhost/my-sandbox +podman build -t localhost/my-sandbox:latest ./my-dir +openshell sandbox create --from localhost/my-sandbox:latest ``` ## Remote CLI access diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 5e9789f4fe..9da7ce05e1 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -166,7 +166,7 @@ to `0.0.0.0` solely to make sandbox callbacks reachable. [Docker](https://www.docker.com/get-started/)-backed sandboxes run as containers on the gateway host. Use Docker for local development, single-machine gateways, and hosts that already use Docker Desktop or Docker Engine. -The gateway talks to the Docker daemon to create sandbox containers. Docker is also required for local image builds from directories or Dockerfiles. +The gateway talks to the Docker daemon to create sandbox containers. Docker Desktop and compatible macOS runtimes route `host.openshell.internal` through an IPv4 host-gateway alias. The gateway reuses an IPv4 primary listener diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 40b35e4237..6246d87f6c 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -144,21 +144,39 @@ openshell sandbox create \ ### Custom Containers -Use `--from` to create a sandbox from the base image, another pre-built sandbox name, a local directory, a rootfs tar archive, or a container image: +Use `--from` to create a sandbox from the base image, another pre-built sandbox name, a rootfs tar archive, or a container image: ```shell openshell sandbox create --from base openshell sandbox create --from ollama -openshell sandbox create --from ./my-sandbox-dir openshell sandbox create --from ./rootfs.tar openshell sandbox create --from my-registry.example.com/my-image:latest ``` Bare names such as `base` and `ollama` resolve to images under `ghcr.io/nvidia/openshell-community/sandboxes`. Set `OPENSHELL_COMMUNITY_REGISTRY` when you need to use an internal mirror. -Local directories and Dockerfiles require a local gateway because the CLI -builds images through the local Docker daemon. Use a registry image reference -for remote gateways. +`--from` does not build local Dockerfiles or directories. Build and tag the +image with the container engine used by your local gateway, then pass the +resulting image reference: + +```shell +# Docker gateway +docker build -t my-image:latest . +openshell sandbox create --from my-image:latest + +# Podman gateway +podman build -t localhost/my-image:latest . +openshell sandbox create --from localhost/my-image:latest +``` + +For a remote gateway, push the image to a registry that the gateway can pull +from and use that registry image reference. + + +**Pre-0.1.0 breaking change:** `openshell sandbox create --from ./Dockerfile` +and directory sources no longer build images. Build and tag the image before +you create the sandbox. + #### Rootfs Tar Archives diff --git a/e2e/rust/src/harness/container.rs b/e2e/rust/src/harness/container.rs index 034cfca78e..542b9db8e6 100644 --- a/e2e/rust/src/harness/container.rs +++ b/e2e/rust/src/harness/container.rs @@ -72,6 +72,75 @@ impl ContainerEngine { } } +static NEXT_IMAGE_ID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +/// Builds a container image from a local Dockerfile via the active container +/// engine and removes it on drop. +/// +/// `openshell sandbox create --from` no longer builds local Dockerfiles +/// itself (see the pre-0.1.0 breaking-change notes), so e2e tests that need a +/// custom image build it out-of-band with this guard and pass the resulting +/// `tag()` to `--from`. +pub struct ImageGuard { + engine: ContainerEngine, + tag: String, +} + +impl ImageGuard { + pub fn build(label: &str, dockerfile: &Path, context: &Path) -> Result { + let engine = ContainerEngine::from_env()?; + let unique = NEXT_IMAGE_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let timestamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + let tag = format!("localhost/openshell-e2e-{label}-{timestamp}-{unique}:latest"); + let output = engine + .command() + .args([ + "build", + "--file", + dockerfile + .to_str() + .ok_or_else(|| "Dockerfile path must be UTF-8".to_string())?, + "--tag", + &tag, + context + .to_str() + .ok_or_else(|| "image context path must be UTF-8".to_string())?, + ]) + .output() + .map_err(|err| format!("run {} build: {err}", engine.name()))?; + if !output.status.success() { + return Err(format!( + "{} build failed (exit {:?}):\n{}{}", + engine.name(), + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + )); + } + Ok(Self { engine, tag }) + } + + #[must_use] + pub fn tag(&self) -> &str { + &self.tag + } +} + +impl Drop for ImageGuard { + fn drop(&mut self) { + let _ = self + .engine + .command() + .args(["image", "rm", "--force", &self.tag]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status(); + } +} + #[must_use] pub fn e2e_network_name() -> Option { std::env::var("OPENSHELL_E2E_NETWORK_NAME") diff --git a/e2e/rust/tests/custom_image.rs b/e2e/rust/tests/custom_image.rs index 5652a0011e..94d7ec7e6f 100644 --- a/e2e/rust/tests/custom_image.rs +++ b/e2e/rust/tests/custom_image.rs @@ -12,6 +12,7 @@ use std::{fs, io::Write}; +use openshell_e2e::harness::container::ImageGuard; use openshell_e2e::harness::output::strip_ansi; use openshell_e2e::harness::sandbox::SandboxGuard; use serial_test::serial; @@ -66,7 +67,7 @@ const MARKER: &str = "custom-image-e2e-marker"; /// already grants that authority; existing content retains its ownership. #[tokio::test] #[serial(custom_image)] -async fn sandbox_from_custom_dockerfile() { +async fn sandbox_from_custom_image() { // Step 1: Write a temporary Dockerfile. let tmpdir = tempfile::tempdir().expect("create tmpdir"); let dockerfile_path = tmpdir.path().join("Dockerfile"); @@ -76,10 +77,13 @@ async fn sandbox_from_custom_dockerfile() { .expect("write Dockerfile"); } - // Step 2: Create a sandbox from the Dockerfile. - let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); + // Step 2: Build the image out-of-band and create a sandbox from it. + // `--from` no longer builds local Dockerfiles itself (pre-0.1.0 + // breaking change); tests build explicitly and pass the resulting tag. + let image = ImageGuard::build("custom-dockerfile", &dockerfile_path, tmpdir.path()) + .expect("build custom image with selected container engine"); let mut guard = SandboxGuard::create_keep_with_args( - &["--from", dockerfile_str, "--no-tty"], + &["--from", image.tag(), "--no-tty"], &[ "sh", "-c", @@ -92,7 +96,7 @@ async fn sandbox_from_custom_dockerfile() { "Ready", ) .await - .expect("sandbox create from Dockerfile"); + .expect("sandbox create from custom image"); // Step 3: Verify the marker file content appears in the output. let clean_output = strip_ansi(&guard.create_output); @@ -194,10 +198,11 @@ async fn sandbox_from_passwd_less_numeric_oci_user() { .expect("write Dockerfile"); } - let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); + let image = ImageGuard::build("passwd-less-numeric", &dockerfile_path, tmpdir.path()) + .expect("build numeric OCI image with selected container engine"); let mut guard = SandboxGuard::create(&[ "--from", - dockerfile_str, + image.tag(), "--", "sh", "-c", @@ -222,10 +227,11 @@ async fn sandbox_rejects_image_workdir_that_would_require_new_authority() { let tmpdir = tempfile::tempdir().expect("create tmpdir"); let dockerfile_path = tmpdir.path().join("Dockerfile"); fs::write(&dockerfile_path, UNWRITABLE_WORKDIR_DOCKERFILE_CONTENT).expect("write Dockerfile"); - let dockerfile_str = dockerfile_path.to_str().expect("Dockerfile path is UTF-8"); + let image = ImageGuard::build("unwritable-workdir", &dockerfile_path, tmpdir.path()) + .expect("build unwritable-workdir image with selected container engine"); let result = SandboxGuard::create_keep_with_args( - &["--from", dockerfile_str, "--no-tty"], + &["--from", image.tag(), "--no-tty"], &["sh", "-c", "echo should-not-run"], "should-not-run", ) @@ -239,9 +245,7 @@ async fn sandbox_rejects_image_workdir_that_would_require_new_authority() { }; let message = error.to_string(); assert!( - message.contains("WorkingDir") - || message.contains("workspace") - || message.contains("readiness"), - "expected workspace authority failure, got: {message}" + message.contains("WorkspaceValidationFailed") && message.contains("WorkingDir"), + "expected rejected image to fail provisioning, got: {message}" ); } diff --git a/e2e/rust/tests/driver_config_volume.rs b/e2e/rust/tests/driver_config_volume.rs index 2d8789edce..1350fc1040 100644 --- a/e2e/rust/tests/driver_config_volume.rs +++ b/e2e/rust/tests/driver_config_volume.rs @@ -7,7 +7,6 @@ use std::fs; use std::io::Write; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; -use std::process::Stdio; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -18,7 +17,7 @@ use bollard::query_parameters::{ RemoveVolumeOptionsBuilder, StartContainerOptions, WaitContainerOptions, }; use futures_util::TryStreamExt; -use openshell_e2e::harness::container::{ContainerEngine, e2e_driver}; +use openshell_e2e::harness::container::{ImageGuard, e2e_driver}; use openshell_e2e::harness::sandbox::SandboxGuard; use serde_json::{Map, Value}; @@ -46,56 +45,6 @@ struct VolumeGuard { name: String, } -struct ImageGuard { - engine: ContainerEngine, - tag: String, -} - -impl ImageGuard { - fn build(driver: &str, dockerfile: &Path, context: &Path) -> Result { - let engine = ContainerEngine::from_env()?; - let tag = format!("localhost/{}-oci-user:latest", unique_volume_name(driver)); - let output = engine - .command() - .args([ - "build", - "--file", - dockerfile - .to_str() - .ok_or_else(|| "Dockerfile path must be UTF-8".to_string())?, - "--tag", - &tag, - context - .to_str() - .ok_or_else(|| "image context path must be UTF-8".to_string())?, - ]) - .output() - .map_err(|err| format!("run {} build: {err}", engine.name()))?; - if !output.status.success() { - return Err(format!( - "{} build failed (exit {:?}):\n{}{}", - engine.name(), - output.status.code(), - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - )); - } - Ok(Self { engine, tag }) - } -} - -impl Drop for ImageGuard { - fn drop(&mut self) { - let _ = self - .engine - .command() - .args(["image", "rm", "--force", &self.tag]) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status(); - } -} - impl VolumeGuard { async fn create(driver: &str) -> Result { let name = unique_volume_name(driver); @@ -184,8 +133,12 @@ async fn oci_workspace_preparation_skips_nested_volume_ownership() { let image_context = tempfile::tempdir().expect("create OCI image context"); let dockerfile = image_context.path().join("Dockerfile"); fs::write(&dockerfile, OCI_USER_DOCKERFILE).expect("write OCI image Dockerfile"); - let image = ImageGuard::build(&driver, &dockerfile, image_context.path()) - .expect("build OCI-user image with selected container engine"); + let image = ImageGuard::build( + &format!("{driver}-oci-user"), + &dockerfile, + image_context.path(), + ) + .expect("build OCI-user image with selected container engine"); let driver_config = format!( r#"{{"{driver}":{{"mounts":[{{"type":"volume","source":"{}","target":"{OCI_VOLUME_TARGET}","read_only":false}}]}}}}"#, @@ -194,7 +147,7 @@ async fn oci_workspace_preparation_skips_nested_volume_ownership() { let mut sandbox = SandboxGuard::create_keep_with_args( &[ "--from", - &image.tag, + image.tag(), "--driver-config-json", &driver_config, "--no-tty", diff --git a/e2e/rust/tests/live_policy_update.rs b/e2e/rust/tests/live_policy_update.rs index 7a1e12923a..7618696c62 100644 --- a/e2e/rust/tests/live_policy_update.rs +++ b/e2e/rust/tests/live_policy_update.rs @@ -147,9 +147,16 @@ landlock: } #[cfg(feature = "e2e-docker")] -fn write_local_override_image() -> Result { +fn write_local_override_image() -> Result< + ( + tempfile::TempDir, + openshell_e2e::harness::container::ImageGuard, + ), + String, +> { let dir = tempfile::tempdir().map_err(|e| format!("create image context: {e}"))?; - std::fs::write(dir.path().join("Dockerfile"), LOCAL_OVERRIDE_DOCKERFILE) + let dockerfile = dir.path().join("Dockerfile"); + std::fs::write(&dockerfile, LOCAL_OVERRIDE_DOCKERFILE) .map_err(|e| format!("write local override Dockerfile: {e}"))?; std::fs::write(dir.path().join("local-policy.rego"), LOCAL_OVERRIDE_REGO) .map_err(|e| format!("write local override Rego policy: {e}"))?; @@ -181,7 +188,12 @@ network_policies: {} ", ) .map_err(|e| format!("write local override policy data: {e}"))?; - Ok(dir) + let image = openshell_e2e::harness::container::ImageGuard::build( + "local-override", + &dockerfile, + dir.path(), + )?; + Ok((dir, image)) } // --------------------------------------------------------------------------- @@ -588,11 +600,7 @@ async fn initial_sparse_policy_is_acknowledged_as_loaded() { #[cfg(feature = "e2e-docker")] #[tokio::test] async fn local_policy_override_survives_gateway_policy_polls() { - let image_context = write_local_override_image().expect("write local override image"); - let dockerfile = image_context.path().join("Dockerfile"); - let dockerfile = dockerfile - .to_str() - .expect("Dockerfile path should be utf-8"); + let (_image_context, image) = write_local_override_image().expect("write local override image"); let gateway_policy_a_file = write_policy(&["example.com"]).expect("write gateway policy A"); let gateway_policy_a_path = gateway_policy_a_file @@ -613,7 +621,7 @@ async fn local_policy_override_survives_gateway_policy_polls() { "--name", "e2e-lcl-pol-ovrd", "--from", - dockerfile, + image.tag(), "--policy", &gateway_policy_a_path, "--no-tty", diff --git a/examples/bring-your-own-container/README.md b/examples/bring-your-own-container/README.md index c79e571f51..a496b0c7a9 100644 --- a/examples/bring-your-own-container/README.md +++ b/examples/bring-your-own-container/README.md @@ -7,7 +7,7 @@ your local machine through port forwarding. ## Prerequisites - A running OpenShell gateway (`mise run gateway:docker` for local development) -- Docker daemon running +- Docker or Podman running, matching the gateway driver ## What's in this example @@ -18,17 +18,27 @@ your local machine through port forwarding. ## Quick start -### 1. Create a sandbox from the Dockerfile with port forwarding +### 1. Build the image and create a sandbox with port forwarding ```bash +# Docker gateway +docker build -t openshell-byoc:latest examples/bring-your-own-container openshell sandbox create \ - --from examples/bring-your-own-container/Dockerfile \ + --from openshell-byoc:latest \ + --forward 8080 \ + -- python /sandbox/app.py + +# Podman gateway +podman build -t localhost/openshell-byoc:latest examples/bring-your-own-container +openshell sandbox create \ + --from localhost/openshell-byoc:latest \ --forward 8080 \ -- python /sandbox/app.py ``` -The `--from` flag accepts a Dockerfile path. The CLI builds the image, -pushes it into the cluster, and creates the sandbox in one step. +Build the Dockerfile with the container engine used by your local gateway +before creating the sandbox, then pass its image reference to `--from`. For a +remote gateway, push the image to a registry that the gateway can pull from. The `--forward 8080` flag opens an SSH tunnel so `localhost:8080` on your machine reaches the REST API inside the sandbox. diff --git a/rfc/0013-native-windows-mxc/README.md b/rfc/0013-native-windows-mxc/README.md index 2a95a6a80d..515e7387dd 100644 --- a/rfc/0013-native-windows-mxc/README.md +++ b/rfc/0013-native-windows-mxc/README.md @@ -205,8 +205,7 @@ trait — there is no separate binary, no surrogate, and no tonic adapter. #### Workload and software availability -MXC does not consume the `openshell sandbox create --from Dockerfile` / OCI image -model used by Linux container runtimes. For current support, the sandbox runs Windows +MXC does not consume the OCI image model used by Linux container runtimes. For current support, the sandbox runs Windows software already present on the host or made available through explicit MXC filesystem grants, with the driver supplying the agent command, working directory, environment, credentials, and policy-derived MXC configuration. diff --git a/scripts/agents/README.md b/scripts/agents/README.md index a718eea78b..32dac5f2d4 100644 --- a/scripts/agents/README.md +++ b/scripts/agents/README.md @@ -73,8 +73,9 @@ Manifest paths support these prefixes: 8. Render the prompt template with runtime values such as `{{HARNESS}}`, `{{RUN_MODE}}`, `{{POLL_INTERVAL_SECONDS}}`, `{{USER_PROMPT}}`, and manifest-declared subagent variables such as `{{REVIEWER_COMMAND}}`. -9. Build a temporary Docker context that bakes the rendered payload into - `/etc/openshell/agent-payload`. +9. Query the gateway's compute driver, then build a temporary image context + with its Docker or Podman image store. The image bakes the rendered payload + into `/etc/openshell/agent-payload`. 10. Apply manifest-declared gateway settings. 11. Resolve provider profile IDs by scanning `profile_paths` in order. 12. Import each provider profile into the gateway. If an active profile already @@ -85,7 +86,7 @@ Manifest paths support these prefixes: to the sandbox. 15. Configure and rotate refresh-backed provider credentials when declared by the manifest. -16. Run `openshell sandbox create` from that temporary Dockerfile source. +16. Run `openshell sandbox create` from the resulting image reference. 17. Inside the sandbox, run `/etc/openshell/agent-payload/runtime/entrypoint.sh`. 18. The runtime entrypoint starts `/etc/openshell/agent-payload/runtime/supervisor.sh`. diff --git a/scripts/agents/gator/README.md b/scripts/agents/gator/README.md index c64bb068f9..b1b8219633 100644 --- a/scripts/agents/gator/README.md +++ b/scripts/agents/gator/README.md @@ -7,7 +7,8 @@ Launch a headless sandbox agent that runs the `gator-gate` skill against OpenShe - `gh` is authenticated on the host and has access to `NVIDIA/OpenShell` and `NVIDIA/OpenShell-Community`. - For `--harness codex`, `codex login` has created `$HOME/.codex/auth.json`. - For `--harness codex`, local Codex auth must include an access token, refresh token, and account ID. -- A local gateway is available when using the default local Dockerfile source. +- A local gateway and either Docker or Podman are available to build the + default sandbox image. ## Usage @@ -19,7 +20,11 @@ Launch a headless sandbox agent that runs the `gator-gate` skill against OpenShe "Run gator on PR 1536 and keep watching until it closes or merges." ``` -By default the launcher uses `scripts/agents/gator/Dockerfile` as the sandbox source. Local gateways build `scripts/agents/gator/` as the image context, so gator-specific image files such as `policy.yaml` and `bin/gh` stay with the gator agent. The launcher bakes rendered prompts, skills, subagents, and shared runtime files into `/etc/openshell/agent-payload`, so `--from` must point to a local Dockerfile or directory containing a Dockerfile. +By default the launcher uses `scripts/agents/gator/Dockerfile` as the sandbox image source. It builds `scripts/agents/gator/` as the image context, so gator-specific image files such as `policy.yaml` and `bin/gh` stay with the gator agent. The launcher bakes rendered prompts, skills, subagents, and shared runtime files into `/etc/openshell/agent-payload`, then passes the resulting image reference to `openshell sandbox create`. + +The launcher queries the selected gateway and builds with its Docker or Podman +compute driver. If `CONTAINER_ENGINE` is set, it must match that driver. Other +gateway drivers cannot run this local-image launcher. Use `--harness codex` to select Codex explicitly. Other harness names are rejected until their support is added to `agent.yaml` and `scripts/agents/runtime/harnesses//`. Agent directories do not carry their own harness implementations; they provide prompt templates and optional skills or subagents for the shared runtime to inject. diff --git a/scripts/agents/run.sh b/scripts/agents/run.sh index 4bd868ea3d..5bf05a7117 100755 --- a/scripts/agents/run.sh +++ b/scripts/agents/run.sh @@ -683,7 +683,38 @@ File.open(dockerfile_path, "a") do |file| end RUBY - SANDBOX_FROM="$build_dockerfile" + # Build into the local engine selected by the gateway. Without this, + # auto-detection can choose Podman while the gateway uses Docker (or vice + # versa), leaving the image unavailable to the gateway. + local gateway_info + if ! gateway_info="$("$OPENSHELL_BIN" --gateway "$GATEWAY" gateway info --output json)"; then + fail "failed to determine compute driver for gateway '$GATEWAY'" + fi + local gateway_engine + if ! gateway_engine="$(printf '%s' "$gateway_info" | ruby -rjson -e ' + drivers = JSON.parse(STDIN.read).fetch("compute_drivers", []).map { |driver| driver.fetch("name") } + abort "gateway must report exactly one compute driver" unless drivers.length == 1 + puts drivers.first.downcase + ')"; then + fail "gateway '$GATEWAY' did not report exactly one compute driver" + fi + case "$gateway_engine" in + docker|podman) ;; + *) fail "gateway '$GATEWAY' uses compute driver '$gateway_engine'; agent launcher local image builds require Docker or Podman" ;; + esac + if [[ -n "${CONTAINER_ENGINE:-}" ]] && [[ "$(printf '%s' "$CONTAINER_ENGINE" | tr '[:upper:]' '[:lower:]')" != "$gateway_engine" ]]; then + fail "CONTAINER_ENGINE=$CONTAINER_ENGINE conflicts with gateway '$GATEWAY' compute driver '$gateway_engine'" + fi + CONTAINER_ENGINE="$gateway_engine" + export CONTAINER_ENGINE + + # Source after setting CONTAINER_ENGINE so the helper validates the + # gateway-selected engine instead of auto-detecting another engine. + source "$ROOT_DIR/tasks/scripts/container-engine.sh" + local image_tag="openshell/agent-${AGENT_ID}:$(date +%s)" + log "Building sandbox image '$image_tag' with $CONTAINER_ENGINE." + ce_build --load --file "$build_dockerfile" --tag "$image_tag" "$build_context" + SANDBOX_FROM="$image_tag" } log "Staging immutable sandbox payload from '$SANDBOX_FROM'." diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index e080fd4001..9d82e442bc 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -582,15 +582,16 @@ Review the proposed scope, candidate hash, prover findings, and application erro Build a custom container image and run it as a sandbox. -### Create a sandbox from a Dockerfile +### Create a sandbox from a pre-built image ```bash -openshell sandbox create --from ./Dockerfile --name my-app +docker build -t my-app:latest . +openshell sandbox create --from my-app:latest --name my-app ``` -The `--from` flag accepts a Dockerfile path, a directory containing a Dockerfile, a full image reference such as `myregistry.com/img:tag`, or a community sandbox name such as `ollama`. +The `--from` flag accepts an existing full image reference such as `myregistry.com/img:tag`, or a community sandbox name such as `ollama`. Build local Dockerfiles first with the same container engine as the local gateway, then pass the image tag. -Local Dockerfile and directory builds require a local gateway because the CLI builds through the local Docker daemon. Use a registry image reference for remote gateways. Bare community names resolve under `ghcr.io/nvidia/openshell-community/sandboxes` unless `OPENSHELL_COMMUNITY_REGISTRY` overrides the prefix. +Use `docker build -t my-app:latest` for Docker gateways. For Podman gateways, use `podman build -t localhost/my-app:latest` and pass `localhost/my-app:latest` to `--from`. For remote gateways, push the image to a registry reachable by the gateway. Bare community names resolve under `ghcr.io/nvidia/openshell-community/sandboxes` unless `OPENSHELL_COMMUNITY_REGISTRY` overrides the prefix. For Docker and Podman gateways, custom images should declare a non-root OCI `USER`. Each explicit `process.run_as_user` or `process.run_as_group` policy @@ -619,7 +620,7 @@ Manage or iterate on the sandbox: openshell forward list openshell forward stop 8080 my-app openshell sandbox delete my-app -openshell sandbox create --from ./Dockerfile --name my-app --forward 8080 +openshell sandbox create --from my-app:latest --name my-app --forward 8080 ``` Use structured output when automation needs the tracked forward metadata and @@ -636,7 +637,7 @@ the forwarded socket. Create and forward in one command: ```bash -openshell sandbox create --from ./Dockerfile --forward 8080 -- ./start-server.sh +openshell sandbox create --from my-app:latest --forward 8080 -- ./start-server.sh ``` The `--forward` flag starts a background port forward before the command runs.