Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions crates/openshell-core/src/driver_utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,6 +45,30 @@ pub fn openshell_sandbox_label_selector() -> String {
format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}")
}

// ---------------------------------------------------------------------------

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made constants public and reusable.

// Sandbox condition reason strings set by compute drivers.
// ---------------------------------------------------------------------------

/// Ready-condition reason when a container exits on its own.
///
/// Covers an ordinary application exit or crash (exit 0, a non-zero error code,
/// or an uncaught fault). This is a terminal reason: gateway startup does NOT
/// auto-restart it, so a genuine failure keeps its error signal instead of
/// being relaunched.
pub const CONDITION_EXITED: &str = "ContainerExited";

/// 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
/// restart does to running containers. Distinct from `CONDITION_EXITED` so
/// gateway startup can recover machine-restart victims while leaving ordinary
/// application exits terminal.
pub const CONDITION_RUNTIME_RESTART: &str = "ContainerRuntimeRestart";

/// Ready-condition reason when a container is explicitly stopped via the
/// runtime API (e.g. `podman stop`, gateway-initiated shutdown).
pub const CONDITION_STOPPED: &str = "ContainerStopped";

// ---------------------------------------------------------------------------

/// Path to the sandbox supervisor binary inside the container image.
Expand Down
75 changes: 64 additions & 11 deletions crates/openshell-driver-docker/src/lib.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,10 +24,10 @@ use openshell_core::config::{
};
use openshell_core::driver_mounts;
use openshell_core::driver_utils::{
LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME,
LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE, SUPERVISOR_IMAGE_BINARY_PATH,
extract_first_tar_entry, supervisor_image_should_refresh, temp_extract_container_name,
validate_linux_elf_binary, write_cache_binary_atomic,
CONDITION_EXITED, CONDITION_RUNTIME_RESTART, LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE,
LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE,
SUPERVISOR_IMAGE_BINARY_PATH, extract_first_tar_entry, supervisor_image_should_refresh,
temp_extract_container_name, validate_linux_elf_binary, write_cache_binary_atomic,
};
use openshell_core::gpu::{
CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements,
Expand DownExpand Up@@ -688,10 +688,37 @@ impl DockerComputeDriver {

async fn current_snapshots(&self) -> Result<Vec<DriverSandbox>, Status> {
let containers = self.list_managed_container_summaries().await?;
let container_sandboxes = containers
.iter()
.filter_map(sandbox_from_container_summary)
.collect::<Vec<_>>();
let mut container_sandboxes = Vec::with_capacity(containers.len());
for summary in &containers {
let Some(mut sandbox) = sandbox_from_container_summary(summary) else {
continue;
};
// Docker's list summary carries no exit code, so an exited
// container is reported as the generic terminal `ContainerExited`.
// Inspect it to tell a machine/daemon-restart signal kill apart
// from an ordinary application exit, mirroring the Podman driver,
// so startup recovery can revive restart victims while leaving
// crashes terminal.
if summary.state == Some(ContainerSummaryStateEnum::EXITED)
&& let Some(container_id) = summary.id.as_deref()
{
match self.docker.inspect_container(container_id, None).await {
Ok(inspected) => {
if let Some(state) = inspected.state.as_ref() {
apply_docker_exit_classification(&mut sandbox, state);
}
}
Err(err) => {
debug!(
container_id,
error = %err,
"Could not inspect exited Docker container to classify its exit"
);
}
}
}
container_sandboxes.push(sandbox);
}
let mut by_id = self.pending_snapshot_map().await;
for sandbox in container_sandboxes {
by_id.insert(sandbox.id.clone(), sandbox);
Expand DownExpand Up@@ -3092,6 +3119,34 @@ 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
/// the generic terminal `ContainerExited` to the recoverable
/// `ContainerRuntimeRestart` so gateway startup can revive it. OOM kills and
/// ordinary application exits stay `ContainerExited` and terminal.
fn apply_docker_exit_classification(sandbox: &mut DriverSandbox, state: &ContainerState) {
if state.oom_killed == Some(true) {
return;
}
let Some(code) = state.exit_code.filter(|&code| matches!(code, 137 | 143)) else {
return;
};
let Some(condition) = sandbox
.status
.as_mut()
.and_then(|status| status.conditions.iter_mut().find(|c| c.r#type == "Ready"))
else {
return;
};
if condition.reason != CONDITION_EXITED {
return;
}
condition.reason = CONDITION_RUNTIME_RESTART.to_string();
condition.message = format!("Container terminated by signal (exit code {code})");
}

fn container_ready_condition(
state: ContainerSummaryStateEnum,
) -> (&'static str, &'static str, &'static str, bool) {
Expand All@@ -3115,9 +3170,7 @@ fn container_ready_condition(
ContainerSummaryStateEnum::PAUSED => {
("False", "ContainerPaused", "Container is paused", false)
}
ContainerSummaryStateEnum::EXITED => {
("False", "ContainerExited", "Container exited", false)
}
ContainerSummaryStateEnum::EXITED => ("False", CONDITION_EXITED, "Container exited", false),
ContainerSummaryStateEnum::DEAD => ("False", "ContainerDead", "Container is dead", false),
}
}
Expand Down
85 changes: 85 additions & 0 deletions crates/openshell-driver-docker/src/tests.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -2647,3 +2647,88 @@ fn lifecycle_fence_rejects_polled_exit_from_before_restart() {
fences.remove("sandbox-1");
assert!(fences.previous_exit("sandbox-1").is_none());
}

fn exited_sandbox_with_ready_reason(reason: &str) -> DriverSandbox {
DriverSandbox {
id: "sbx-exit".to_string(),
name: "demo".to_string(),
namespace: String::new(),
spec: None,
status: Some(DriverSandboxStatus {
sandbox_name: "demo".to_string(),
instance_id: "container-1".to_string(),
agent_fd: String::new(),
sandbox_fd: String::new(),
conditions: vec![DriverCondition {
r#type: "Ready".to_string(),
status: "False".to_string(),
reason: reason.to_string(),
message: "Container exited".to_string(),
last_transition_time: String::new(),
}],
deleting: false,
}),
workspace: String::new(),
}
}

fn ready_reason(sandbox: &DriverSandbox) -> &str {
sandbox
.status
.as_ref()
.and_then(|status| status.conditions.iter().find(|c| c.r#type == "Ready"))
.map(|c| c.reason.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 —
// the signature of a machine/daemon restart — and become recoverable
// `ContainerRuntimeRestart`.
for exit_code in [137, 143] {
let mut sandbox = exited_sandbox_with_ready_reason(CONDITION_EXITED);
let state = ContainerState {
status: Some(ContainerStateStatusEnum::EXITED),
oom_killed: Some(false),
exit_code: Some(exit_code),
..Default::default()
};
apply_docker_exit_classification(&mut sandbox, &state);
assert_eq!(
ready_reason(&sandbox),
CONDITION_RUNTIME_RESTART,
"exit code {exit_code} should reclassify as runtime restart"
);
}
}

#[test]
fn docker_ordinary_exit_stays_terminal() {
// An application exit (non-zero error code) stays `ContainerExited` so its
// failure signal survives instead of being relaunched on startup.
let mut sandbox = exited_sandbox_with_ready_reason(CONDITION_EXITED);
let state = ContainerState {
status: Some(ContainerStateStatusEnum::EXITED),
oom_killed: Some(false),
exit_code: Some(1),
..Default::default()
};
apply_docker_exit_classification(&mut sandbox, &state);
assert_eq!(ready_reason(&sandbox), CONDITION_EXITED);
}

#[test]
fn docker_oom_kill_stays_terminal_despite_137() {
// An OOM kill reports exit 137 but must NOT be treated as a recoverable
// restart — it is a genuine failure and stays terminal.
let mut sandbox = exited_sandbox_with_ready_reason(CONDITION_EXITED);
let state = ContainerState {
status: Some(ContainerStateStatusEnum::EXITED),
oom_killed: Some(true),
exit_code: Some(137),
..Default::default()
};
apply_docker_exit_classification(&mut sandbox, &state);
assert_eq!(ready_reason(&sandbox), CONDITION_EXITED);
}
61 changes: 51 additions & 10 deletions crates/openshell-driver-podman/src/watcher.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,8 +26,9 @@ use tracing::{debug, info, warn};
// Condition reason constants shared across event-building paths.
const CONDITION_RUNNING: &str = "ContainerRunning";
const CONDITION_STARTING: &str = "ContainerStarting";
const CONDITION_EXITED: &str = "ContainerExited";
const CONDITION_STOPPED: &str = "ContainerStopped";
use openshell_core::driver_utils::{
CONDITION_EXITED, CONDITION_RUNTIME_RESTART, CONDITION_STOPPED,
};

pub type WatchStream =
Pin<Box<dyn Stream<Item = Result<WatchSandboxesEvent, ComputeDriverError>> + Send>>;
Expand DownExpand Up@@ -445,15 +446,29 @@ fn condition_from_state(state: &ContainerState) -> DriverCondition {
},
"created" => ("False", "ContainerCreated", String::new()),
"exited" | "stopped" => {
let msg = if state.oom_killed {
"Container was killed by the OOM killer".to_string()
} else {
format!("Container exited with code {}", state.exit_code)
};
let reason = if state.oom_killed {
"OOMKilled"
// Exit codes 137 (128+SIGKILL) and 143 (128+SIGTERM) mean the
// container was terminated by an external signal rather than
// exiting on its own — the signature of a machine/daemon restart
// killing running containers. Those are recoverable at gateway
// startup; ordinary application exits (0, non-zero, faults) are not.
let (reason, msg) = if state.oom_killed {
(
"OOMKilled",
"Container was killed by the OOM killer".to_string(),
)
} else if matches!(state.exit_code, 137 | 143) {
(
CONDITION_RUNTIME_RESTART,
format!(
"Container terminated by signal (exit code {})",
state.exit_code
),
)
} else {
CONDITION_EXITED
(
CONDITION_EXITED,
format!("Container exited with code {}", state.exit_code),
)
};
("False", reason, msg)
}
Expand DownExpand Up@@ -600,6 +615,32 @@ mod tests {
assert!(cond.message.contains("code 1"));
}

#[test]
fn condition_signal_kill_is_runtime_restart() {
// 137 (128+SIGKILL) and 143 (128+SIGTERM) are external terminations —
// the signature of a machine/daemon restart. They classify as
// recoverable `ContainerRuntimeRestart`, distinct from an ordinary
// application exit.
for exit_code in [137, 143] {
let state = ContainerState {
status: "exited".to_string(),
running: false,
exit_code,
oom_killed: false,
health: None,
started_at: None,
finished_at: Some("2026-04-14T12:30:00Z".to_string()),
};
let cond = condition_from_state(&state);
assert_eq!(cond.status, "False");
assert_eq!(
cond.reason, "ContainerRuntimeRestart",
"exit code {exit_code} should classify as runtime restart"
);
assert!(cond.message.contains(&format!("code {exit_code}")));
}
}

#[test]
fn short_id_truncates() {
assert_eq!(short_id("abc123def456789"), "abc123def456");
Expand Down
Loading
Loading