From 2ad800d378ce28592acc0246ce83614b75a9596d Mon Sep 17 00:00:00 2001 From: G4614 <92488762+G4614@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:27:19 +0800 Subject: [PATCH] fix(python): align sdk errors and metadata --- sdks/python/boxlite/constants.py | 4 +- sdks/python/boxlite/exec.py | 25 ++++++++ sdks/python/boxlite/simplebox.py | 34 +++++++++-- sdks/python/boxlite/sync_api/_simplebox.py | 26 ++++++-- sdks/python/src/runtime.rs | 22 ++++++- sdks/python/tests/test_box_management_mock.py | 11 ++++ .../python/tests/test_exec_timeout_sigalrm.py | 59 ++++++++----------- sdks/python/tests/test_simplebox.py | 31 ++++++++++ sdks/python/tests/test_skillbox.py | 4 +- sdks/python/tests/test_sync_simplebox.py | 15 ++++- sdks/python/tests/test_sync_skillbox.py | 4 +- src/boxlite/src/runtime/constants.rs | 4 +- src/boxlite/src/runtime/rt_impl.rs | 16 ++++- src/boxlite/src/runtime/types.rs | 5 +- src/boxlite/src/vmm/krun/engine.rs | 13 +++- src/boxlite/tests/lifecycle.rs | 23 +++++++- 16 files changed, 231 insertions(+), 65 deletions(-) create mode 100644 sdks/python/tests/test_simplebox.py diff --git a/sdks/python/boxlite/constants.py b/sdks/python/boxlite/constants.py index 27a0bff16..518b619a2 100644 --- a/sdks/python/boxlite/constants.py +++ b/sdks/python/boxlite/constants.py @@ -3,8 +3,8 @@ """ # Default VM resources -DEFAULT_CPUS = 1 -DEFAULT_MEMORY_MIB = 2048 +DEFAULT_CPUS = 4 +DEFAULT_MEMORY_MIB = 4096 # ComputerBox defaults (higher resources for desktop) COMPUTERBOX_CPUS = 2 diff --git a/sdks/python/boxlite/exec.py b/sdks/python/boxlite/exec.py index 576dcd4d6..afc494aba 100644 --- a/sdks/python/boxlite/exec.py +++ b/sdks/python/boxlite/exec.py @@ -28,3 +28,28 @@ class ExecResult: stdout: str stderr: str error_message: str | None = None + + +def _looks_like_command_start_error(message: str) -> bool: + """Return true for errors caused by an invalid user command.""" + lowered = message.lower() + return ( + ("executable" in lowered and "not found" in lowered) + or "not found in $path" in lowered + or "not found in path" in lowered + or "does not have correct permissions" in lowered + or "permission denied" in lowered + ) + + +def _looks_like_timeout_result( + exit_code: int, + *, + timeout: float | None, + elapsed: float, +) -> bool: + if timeout is None: + return False + if exit_code not in {-15, -9}: + return False + return elapsed >= max(0.0, timeout * 0.8) diff --git a/sdks/python/boxlite/simplebox.py b/sdks/python/boxlite/simplebox.py index e6082c4e9..af4b57b9c 100644 --- a/sdks/python/boxlite/simplebox.py +++ b/sdks/python/boxlite/simplebox.py @@ -6,10 +6,12 @@ import asyncio import logging +import time from enum import IntEnum from typing import Optional, TYPE_CHECKING -from .exec import ExecResult +from .errors import ExecError, TimeoutError +from .exec import ExecResult, _looks_like_command_start_error, _looks_like_timeout_result if TYPE_CHECKING: from .boxlite import Boxlite @@ -219,10 +221,19 @@ async def exec( # Convert env dict to list of tuples if provided env_list = list(env.items()) if env else None + command_display = " ".join([cmd, *args]) + started_at = time.monotonic() + # Execute via Rust (returns PyExecution) - execution = await self._box.exec( - cmd, arg_list, env_list, user=user, timeout_secs=timeout, cwd=cwd - ) + try: + execution = await self._box.exec( + cmd, arg_list, env_list, user=user, timeout_secs=timeout, cwd=cwd + ) + except Exception as e: + message = str(e) + if _looks_like_command_start_error(message): + raise ExecError(command_display, 127, message) from e + raise # Get streams from Rust execution try: @@ -285,6 +296,12 @@ async def collect_stderr(): logger.debug(f"exec finish, exit_code: {exit_code}") + elapsed = time.monotonic() - started_at + if _looks_like_timeout_result(exit_code, timeout=timeout, elapsed=elapsed): + raise TimeoutError( + f"Command '{command_display}' timed out after {timeout:g} seconds" + ) + return ExecResult( exit_code=exit_code, stdout=stdout, @@ -292,6 +309,15 @@ async def collect_stderr(): error_message=error_message, ) + async def metrics(self): + """Get box metrics (CPU, memory usage).""" + if not self._started: + raise RuntimeError( + "Box not started. Use 'async with SimpleBox(...) as box:' " + "or call 'await box.start()' first." + ) + return await self._box.metrics() + async def stop(self): """ Stop the box and release resources. diff --git a/sdks/python/boxlite/sync_api/_simplebox.py b/sdks/python/boxlite/sync_api/_simplebox.py index bea4ec7e0..770e69085 100644 --- a/sdks/python/boxlite/sync_api/_simplebox.py +++ b/sdks/python/boxlite/sync_api/_simplebox.py @@ -7,9 +7,11 @@ import asyncio import logging +import time from typing import TYPE_CHECKING, Dict, Optional -from ..exec import ExecResult +from ..errors import ExecError, TimeoutError +from ..exec import ExecResult, _looks_like_command_start_error, _looks_like_timeout_result if TYPE_CHECKING: from ._boxlite import SyncBoxlite @@ -190,11 +192,20 @@ def exec( # Access the underlying async Box directly async_box = self._box._box + command_display = " ".join([cmd, *args]) async def _exec_and_collect(): - execution = await async_box.exec( - cmd, arg_list, env_list, user=user, timeout_secs=timeout, cwd=cwd - ) + started_at = time.monotonic() + + try: + execution = await async_box.exec( + cmd, arg_list, env_list, user=user, timeout_secs=timeout, cwd=cwd + ) + except Exception as e: + message = str(e) + if _looks_like_command_start_error(message): + raise ExecError(command_display, 127, message) from e + raise stdout_lines = [] stderr_lines = [] @@ -245,6 +256,13 @@ async def collect_stderr(): except Exception as e: logger.error(f"failed to wait execution: {e}") exit_code = -1 + error_message = None + + elapsed = time.monotonic() - started_at + if _looks_like_timeout_result(exit_code, timeout=timeout, elapsed=elapsed): + raise TimeoutError( + f"Command '{command_display}' timed out after {timeout:g} seconds" + ) return ExecResult( exit_code=exit_code, diff --git a/sdks/python/src/runtime.rs b/sdks/python/src/runtime.rs index c65fab769..4d75050e1 100644 --- a/sdks/python/src/runtime.rs +++ b/sdks/python/src/runtime.rs @@ -223,8 +223,26 @@ impl PyBoxlite { }) } - fn __enter__(slf: PyRef<'_, Self>) -> PyResult> { - Ok(slf) + fn __aenter__<'py>(&self, py: Python<'py>) -> PyResult> { + let runtime = Arc::clone(&self.runtime); + pyo3_async_runtimes::tokio::future_into_py(py, async move { Ok(Self { runtime }) }) + } + + fn __aexit__<'py>( + &self, + py: Python<'py>, + _exc_type: Py, + _exc_val: Py, + _exc_tb: Py, + ) -> PyResult> { + pyo3_async_runtimes::tokio::future_into_py(py, async move { Ok(false) }) + } + + fn __enter__(&self) -> PyResult<()> { + Err(pyo3::exceptions::PyRuntimeError::new_err( + "Boxlite is an async runtime; use 'async with boxlite.Boxlite(...)' \ + or use 'with boxlite.SyncBoxlite(...)' for the synchronous API", + )) } fn __exit__( diff --git a/sdks/python/tests/test_box_management_mock.py b/sdks/python/tests/test_box_management_mock.py index ba974ed26..133d5de28 100644 --- a/sdks/python/tests/test_box_management_mock.py +++ b/sdks/python/tests/test_box_management_mock.py @@ -137,6 +137,17 @@ def cls(self): def test_method_exists(self, cls, method): assert hasattr(cls, method), f"Boxlite missing method: {method}" + def test_sync_context_manager_points_to_sync_api(self, tmp_path): + runtime = boxlite.Boxlite(boxlite.Options(home_dir=str(tmp_path))) + with pytest.raises(RuntimeError, match="SyncBoxlite"): + with runtime: + pass + + @pytest.mark.asyncio + async def test_async_context_manager(self, tmp_path): + async with boxlite.Boxlite(boxlite.Options(home_dir=str(tmp_path))) as runtime: + assert isinstance(runtime, boxlite.Boxlite) + def test_rest_runtime_images_unsupported(self): runtime = boxlite.Boxlite.rest( boxlite.BoxliteRestOptions(url="http://localhost:1") diff --git a/sdks/python/tests/test_exec_timeout_sigalrm.py b/sdks/python/tests/test_exec_timeout_sigalrm.py index c36080147..dc0d5557a 100644 --- a/sdks/python/tests/test_exec_timeout_sigalrm.py +++ b/sdks/python/tests/test_exec_timeout_sigalrm.py @@ -7,11 +7,9 @@ completion, and ``ExecResult.exit_code`` comes back as 0 — the deadline is bypassed. -The Python SDK does NOT raise ``boxlite.TimeoutError`` on exec timeout; it -returns an ``ExecResult`` whose ``exit_code`` reflects how the process died -(non-zero / signal). The PoC at ``boxlite_poc/poc_sigalrm_bypass.py`` -confirms this: it inspects ``exit_code`` and elapsed wall-time, never an -exception. +The Python convenience SDK raises ``boxlite.TimeoutError`` on exec timeout. +This test still checks elapsed wall-time so timeout signaling regressions do +not hide behind the typed exception. Requirements: - make dev:python (build the Python SDK with native extension) @@ -66,9 +64,9 @@ async def test_exec_timeout_kills_sigalrm_ignoring_process(shared_runtime): Two-pronged assertion — both must hold: - 1) ``exit_code != 0`` — the workload must NOT have completed normally. - With the SIGALRM bug, SIG_IGN absorbs the timeout signal, the loop - finishes, and Python exits cleanly with 0. + 1) ``TimeoutError`` is raised — the workload must NOT have completed + normally. With the SIGALRM bug, SIG_IGN absorbs the timeout signal, + the loop finishes, and Python exits cleanly with 0. 2) ``elapsed < TIMEOUT_S + 2.0`` — termination must happen near the configured deadline, not at the workload's natural end. Even if @@ -82,23 +80,18 @@ async def test_exec_timeout_kills_sigalrm_ignoring_process(shared_runtime): image="python:3-alpine", runtime=shared_runtime ) as box: t0 = time.time() - result = await box.exec( - "python3", - "-c", - IGNORE_SIGALRM, - str(WORKLOAD_S), - timeout=TIMEOUT_S, - ) + with pytest.raises(boxlite.TimeoutError): + await box.exec( + "python3", + "-c", + IGNORE_SIGALRM, + str(WORKLOAD_S), + timeout=TIMEOUT_S, + ) elapsed = time.time() - t0 - assert result.exit_code != 0, ( - f"exec returned exit_code=0 after {elapsed:.2f}s — the {WORKLOAD_S}s " - f"workload completed normally despite timeout={TIMEOUT_S}s. The " - f"guest's timeout watcher is sending a catchable signal that the " - f"workload absorbs via SIG_IGN; the kill must use SIGKILL." - ) assert elapsed < TIMEOUT_S + 2.0, ( - f"exec returned after {elapsed:.2f}s with exit_code={result.exit_code} " + f"exec returned after {elapsed:.2f}s " f"— expected termination near {TIMEOUT_S}s. The timeout watcher " f"is not killing the process promptly." ) @@ -125,22 +118,16 @@ async def test_exec_timeout_sigkill_fallback_when_sigterm_ignored(shared_runtime image="python:3-alpine", runtime=shared_runtime ) as box: t0 = time.time() - result = await box.exec( - "python3", - "-c", - IGNORE_TERM_AND_ALRM, - str(WORKLOAD_S), - timeout=TIMEOUT_S, - ) + with pytest.raises(boxlite.TimeoutError): + await box.exec( + "python3", + "-c", + IGNORE_TERM_AND_ALRM, + str(WORKLOAD_S), + timeout=TIMEOUT_S, + ) elapsed = time.time() - t0 - assert result.exit_code != 0, ( - f"stage-3 SIGKILL fallback broken: exec returned exit_code=0 " - f"after {elapsed:.2f}s. The workload ignores SIGTERM AND was not " - f"SIGKILL'd by the watcher — either grace expired without sending " - f"SIGKILL, or stage-3 escalation was removed from " - f"src/guest/src/service/exec/timeout.rs." - ) # Expected ~ TIMEOUT_S + GRACE_S = 5.0s. Allow +2.0s headroom for VM # / SDK overhead. If elapsed approaches WORKLOAD_S, the watcher is # not enforcing the deadline at all. diff --git a/sdks/python/tests/test_simplebox.py b/sdks/python/tests/test_simplebox.py new file mode 100644 index 000000000..6b0202fdf --- /dev/null +++ b/sdks/python/tests/test_simplebox.py @@ -0,0 +1,31 @@ +"""Integration tests for the async SimpleBox convenience wrapper.""" + +from __future__ import annotations + +import pytest + +import boxlite + +pytestmark = [pytest.mark.integration, pytest.mark.asyncio] + + +async def test_simplebox_metrics(shared_runtime): + """Async SimpleBox exposes box metrics like SyncSimpleBox.""" + async with boxlite.SimpleBox( + image="alpine:latest", runtime=shared_runtime + ) as box: + await box.exec("echo", "test") + metrics = await box.metrics() + assert metrics is not None + assert metrics.commands_executed_total >= 1 + + +async def test_simplebox_command_not_found_raises_exec_error(shared_runtime): + """Direct command start failures raise ExecError, not bare RuntimeError.""" + async with boxlite.SimpleBox( + image="alpine:latest", runtime=shared_runtime + ) as box: + with pytest.raises(boxlite.ExecError) as exc: + await box.exec("definitely-not-a-boxlite-command-xyz") + assert exc.value.exit_code == 127 + assert "not found" in exc.value.stderr.lower() diff --git a/sdks/python/tests/test_skillbox.py b/sdks/python/tests/test_skillbox.py index 27834db39..c4c1f3744 100644 --- a/sdks/python/tests/test_skillbox.py +++ b/sdks/python/tests/test_skillbox.py @@ -71,8 +71,8 @@ def test_default_values(self, shared_sync_runtime, oauth_token): info = box.info() # Default image is node:20-alpine assert "node" in info.image.lower() - # Default memory is 2048 MiB - assert info.memory_mib == 2048 + # Default memory is 4096 MiB + assert info.memory_mib == 4096 def test_custom_name(self, shared_sync_runtime, oauth_token): """Test SkillBox with custom name.""" diff --git a/sdks/python/tests/test_sync_simplebox.py b/sdks/python/tests/test_sync_simplebox.py index 928cfd031..f87928432 100644 --- a/sdks/python/tests/test_sync_simplebox.py +++ b/sdks/python/tests/test_sync_simplebox.py @@ -11,6 +11,7 @@ # Try to import sync API - skip if greenlet not installed try: + import boxlite from boxlite import SyncSimpleBox SYNC_AVAILABLE = True @@ -82,6 +83,14 @@ def test_metrics(self, shared_sync_runtime): assert metrics is not None assert metrics.commands_executed_total >= 1 + def test_command_not_found_raises_exec_error(self, shared_sync_runtime): + """Direct command start failures raise ExecError, not bare RuntimeError.""" + with SyncSimpleBox(image="alpine:latest", runtime=shared_sync_runtime) as box: + with pytest.raises(boxlite.ExecError) as exc: + box.exec("definitely-not-a-boxlite-command-xyz") + assert exc.value.exit_code == 127 + assert "not found" in exc.value.stderr.lower() + def test_custom_working_dir(self, shared_sync_runtime): """Can set custom working directory.""" with SyncSimpleBox( @@ -115,10 +124,10 @@ def test_exec_with_user(self, shared_sync_runtime): assert "nobody" in result.stdout def test_exec_with_timeout(self, shared_sync_runtime): - """Per-exec timeout kills long-running commands.""" + """Per-exec timeout raises a typed TimeoutError.""" with SyncSimpleBox(image="alpine:latest", runtime=shared_sync_runtime) as box: - result = box.exec("sleep", "60", timeout=2) - assert result.exit_code != 0 + with pytest.raises(boxlite.TimeoutError): + box.exec("sleep", "60", timeout=2) def test_exec_combined_options(self, shared_sync_runtime): """Per-exec cwd and user can be combined.""" diff --git a/sdks/python/tests/test_sync_skillbox.py b/sdks/python/tests/test_sync_skillbox.py index 69c234dde..9383fde67 100644 --- a/sdks/python/tests/test_sync_skillbox.py +++ b/sdks/python/tests/test_sync_skillbox.py @@ -69,8 +69,8 @@ def test_default_values(self, shared_sync_runtime, oauth_token): info = box.info() # Default image is node:20-alpine assert "node" in info.image.lower() - # Default memory is 2048 MiB - assert info.memory_mib == 2048 + # Default memory is 4096 MiB + assert info.memory_mib == 4096 def test_custom_name(self, shared_sync_runtime, oauth_token): """Test SyncSkillBox with custom name.""" diff --git a/src/boxlite/src/runtime/constants.rs b/src/boxlite/src/runtime/constants.rs index 31dc08b6a..d348576d2 100644 --- a/src/boxlite/src/runtime/constants.rs +++ b/src/boxlite/src/runtime/constants.rs @@ -60,10 +60,10 @@ pub mod fs_options { /// Virtual machine resource defaults pub mod vm_defaults { /// Default number of CPUs allocated to a Box - pub const DEFAULT_CPUS: u8 = 1; + pub const DEFAULT_CPUS: u8 = 4; /// Default memory in MiB allocated to a Box - pub const DEFAULT_MEMORY_MIB: u32 = 2048; + pub const DEFAULT_MEMORY_MIB: u32 = 4096; /// Default disk size in GB for the container rootfs (sparse, grows as needed) pub const DEFAULT_DISK_SIZE_GB: u64 = 10; diff --git a/src/boxlite/src/runtime/rt_impl.rs b/src/boxlite/src/runtime/rt_impl.rs index 19d16bcaa..78747bbf3 100644 --- a/src/boxlite/src/runtime/rt_impl.rs +++ b/src/boxlite/src/runtime/rt_impl.rs @@ -27,6 +27,10 @@ fn litebox_from_impl(box_impl: SharedBoxImpl) -> LiteBox { LiteBox::new(box_backend, snapshot_backend) } +fn looks_like_local_box_id(value: &str) -> bool { + value.len() == 12 && value.bytes().all(|b| b.is_ascii_alphanumeric()) +} + /// Archive the active exit file as `exit.previous`, freeing the canonical /// slot for the next crash. Single-slot rotation — any prior `exit.previous` /// is overwritten. NotFound on source is tolerated. @@ -491,7 +495,17 @@ impl RuntimeImpl { /// Remove a box completely by ID or name. pub fn remove(&self, id_or_name: &str, force: bool) -> BoxliteResult<()> { - let box_id = self.resolve_id(id_or_name)?; + let box_id = match self.resolve_id(id_or_name) { + Ok(box_id) => box_id, + Err(BoxliteError::NotFound(_)) if looks_like_local_box_id(id_or_name) => { + tracing::debug!( + box_id = %id_or_name, + "Remove called for an already-missing local box id; treating as cleanup success" + ); + return Ok(()); + } + Err(err) => return Err(err), + }; self.remove_box(&box_id, force) } diff --git a/src/boxlite/src/runtime/types.rs b/src/boxlite/src/runtime/types.rs index d81ca292f..6f16bf86d 100644 --- a/src/boxlite/src/runtime/types.rs +++ b/src/boxlite/src/runtime/types.rs @@ -320,6 +320,7 @@ pub struct BoxInfo { impl BoxInfo { /// Create BoxInfo from config and state. pub fn new(config: &crate::litebox::config::BoxConfig, state: &BoxState) -> Self { + use crate::runtime::constants::vm_defaults::{DEFAULT_CPUS, DEFAULT_MEMORY_MIB}; use crate::runtime::options::RootfsSpec; Self { @@ -333,8 +334,8 @@ impl BoxInfo { RootfsSpec::Image(r) => r.clone(), RootfsSpec::RootfsPath(p) => format!("rootfs:{}", p), }, - cpus: config.options.cpus.unwrap_or(2), - memory_mib: config.options.memory_mib.unwrap_or(512), + cpus: config.options.cpus.unwrap_or(DEFAULT_CPUS), + memory_mib: config.options.memory_mib.unwrap_or(DEFAULT_MEMORY_MIB), labels: HashMap::new(), health_status: state.health_status, } diff --git a/src/boxlite/src/vmm/krun/engine.rs b/src/boxlite/src/vmm/krun/engine.rs index 66595c859..7ac648db7 100644 --- a/src/boxlite/src/vmm/krun/engine.rs +++ b/src/boxlite/src/vmm/krun/engine.rs @@ -2,6 +2,7 @@ use super::context::KrunContext; use crate::runtime::constants::network; +use crate::runtime::constants::vm_defaults::{DEFAULT_CPUS, DEFAULT_MEMORY_MIB}; use crate::vmm::{InstanceSpec, Vmm, VmmConfig, VmmInstance, engine::VmmInstanceImpl}; use boxlite_shared::errors::{BoxliteError, BoxliteResult}; @@ -238,9 +239,15 @@ impl Vmm for Krun { tracing::debug!("Creating libkrun context"); let mut ctx = KrunContext::create()?; - tracing::debug!("Setting VM config: 4 CPUs, 4096MB memory"); - // Configure VM like chroot_vm example: 4 CPUs and 4096MB memory - ctx.set_vm_config(config.cpus.unwrap_or(4), config.memory_mib.unwrap_or(4096))?; + tracing::debug!( + cpus = config.cpus.unwrap_or(DEFAULT_CPUS), + memory_mib = config.memory_mib.unwrap_or(DEFAULT_MEMORY_MIB), + "Setting VM config" + ); + ctx.set_vm_config( + config.cpus.unwrap_or(DEFAULT_CPUS), + config.memory_mib.unwrap_or(DEFAULT_MEMORY_MIB), + )?; // Configure net from connection info passed by parent process if let Some(connection) = &config.network_backend_endpoint { diff --git a/src/boxlite/tests/lifecycle.rs b/src/boxlite/tests/lifecycle.rs index 22fb7bed2..e1b27b7ae 100644 --- a/src/boxlite/tests/lifecycle.rs +++ b/src/boxlite/tests/lifecycle.rs @@ -248,6 +248,25 @@ async fn remove_nonexistent_returns_not_found() { ); } +#[tokio::test] +async fn remove_auto_removed_local_box_id_is_idempotent() { + let home = boxlite_test_utils::home::PerTestBoxHome::isolated(); + let runtime = BoxliteRuntime::new(BoxliteOptions { + home_dir: home.path.clone(), + image_registries: common::test_registries(), + }) + .expect("create runtime"); + let handle = runtime + .create(common::alpine_opts_auto(), None) + .await + .unwrap(); + let box_id = handle.id().clone(); + + handle.stop().await.unwrap(); + assert!(runtime.get_info(box_id.as_str()).await.unwrap().is_none()); + runtime.remove(box_id.as_str(), false).await.unwrap(); +} + #[tokio::test] async fn remove_stopped_box_succeeds() { let home = boxlite_test_utils::home::PerTestBoxHome::isolated(); @@ -417,8 +436,8 @@ async fn litebox_info_returns_correct_metadata() { .expect("info should be available"); assert_eq!(info.id, box_id); assert_eq!(info.status, BoxStatus::Configured); - assert_eq!(info.cpus, 2); // Default value - assert_eq!(info.memory_mib, 512); // Default value + assert_eq!(info.cpus, 4); // Default value + assert_eq!(info.memory_mib, 4096); // Default value // Cleanup runtime.remove(box_id.as_str(), true).await.unwrap();