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
48 changes: 48 additions & 0 deletions architecture/compute-runtimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,60 @@ Each runtime receives a sandbox spec from the gateway and is responsible for:
- Reporting lifecycle and platform events back to the gateway.
- Cleaning up runtime-owned resources.

Drivers report **backend state only**. A driver snapshot with `Ready=True` means
the underlying compute resource (container, pod, VM) is healthy and running —
nothing more. Drivers must not gate on supervisor session state or hold
references to gateway-internal types. The gateway owns the public
`SandboxPhase::Ready` decision. This applies equally to extension drivers
implementing `ComputeDriver` out of tree.

Drivers own runtime-specific platform event interpretation. When an event should
drive client provisioning UI, the driver attaches the shared
`openshell.progress.*` metadata defined in `openshell-core` instead of requiring
clients to parse Kubernetes reasons, VM cache states, or other driver-local
reason strings.

## Sandbox Readiness Composition

The gateway composes driver backend state with supervisor session presence to
produce the public `SandboxPhase`. This composition is gateway-owned and applied
uniformly across all drivers:

```
backend_phase = derive_phase(driver_status)

public_phase =
if backend_phase in {Error, Deleting}: → pass through (terminal precedence)
if backend_phase == Ready && session connected: → Ready
if backend_phase == Ready && no session: → Provisioning
if backend_phase in {Provisioning, Unknown} && session: → Ready

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is it strange that a non-Ready backend phase can be promoted to Ready if a supervisor session is present?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Checked the reason: It may be that the supervisor connected event arrives before the backend ready event. In this case, the sandbox is assumed ready because it must be running if the supervisor connected.

if backend_phase in {Provisioning, Unknown} && no session: → Provisioning
```

When `public_phase == Ready` the sandbox is usable through the gateway — both the
backend resource is healthy and a supervisor session is registered. A sandbox whose
backend reports ready but has no supervisor session yet holds `Provisioning`; the
driver's `BackendReady=True` condition is visible in the sandbox status for operators
who need to distinguish that state from a sandbox still provisioning its compute resource.

**Session precedence over lagging driver snapshots:** A supervisor session can only be
established by a running workload. When `set_supervisor_session_state` promotes the
store record to `Ready` on session connect, a driver watch event may still arrive
shortly after carrying a stale `Provisioning` or `Unknown` backend phase. The
composition rule treats a connected session as the stronger signal and keeps `Ready`
in that case, preventing a lagging snapshot from undoing the session-driven promotion.

**HA deployments:** Supervisor sessions are process-local. A gateway replica that
does not own the active supervisor session holds the public phase at `Provisioning`.
The owning replica's `supervisor_session_connected` write propagates through the
shared store and reconcile loop. This is correct behavior — a replica should not
claim `Ready` for a session it does not hold.

**Extension point:** The readiness decision is a safety invariant, not an
operator-configurable hook. The driver contract is the correct extension point for
custom backend readiness semantics. RFC-0010 lifecycle hooks may observe readiness
transitions via `post_commit`; they do not override the composition rule.

The capability RPC reports driver identity, version, and the default sandbox
image used by the gateway. GPU availability stays driver-local and is validated
when a sandbox create request asks for GPU resources.
Expand Down
64 changes: 10 additions & 54 deletions crates/openshell-driver-docker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,6 @@ fn resolve_default_docker_supervisor_image_tag(
tag.replace('+', "-")
}

/// Queried by the Docker driver to decide when a sandbox's supervisor
/// relay is live. Implementations return `true` once a sandbox has an
/// active `ConnectSupervisor` session registered.
///
/// The driver cannot observe the supervisor's SSH socket directly (it
/// lives inside the container), so it leans on this signal to flip the
/// Ready condition from `DependenciesNotReady` to `True`.
pub trait SupervisorReadiness: Send + Sync + 'static {
fn is_supervisor_connected(&self, sandbox_id: &str) -> bool;
}

/// Gateway-local configuration for the Docker compute driver.
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
#[serde(default, deny_unknown_fields)]
Expand Down Expand Up @@ -254,7 +243,6 @@ pub struct DockerComputeDriver {
config: DockerDriverRuntimeConfig,
events: broadcast::Sender<WatchSandboxesEvent>,
pending: Arc<Mutex<HashMap<String, PendingSandboxRecord>>>,
supervisor_readiness: Arc<dyn SupervisorReadiness>,
gpu_selector: Arc<CdiGpuDefaultSelector>,
}

Expand Down Expand Up @@ -351,11 +339,7 @@ type WatchStream =
Pin<Box<dyn Stream<Item = Result<WatchSandboxesEvent, Status>> + Send + 'static>>;

impl DockerComputeDriver {
pub async fn new(
config: &Config,
docker_config: &DockerComputeConfig,
supervisor_readiness: Arc<dyn SupervisorReadiness>,
) -> CoreResult<Self> {
pub async fn new(config: &Config, docker_config: &DockerComputeConfig) -> CoreResult<Self> {
let docker = Docker::connect_with_local_defaults()
.map_err(|err| Error::execution(format!("failed to create Docker client: {err}")))?;
let version = docker.version().await.map_err(|err| {
Expand Down Expand Up @@ -423,7 +407,6 @@ impl DockerComputeDriver {
},
events: broadcast::channel(WATCH_BUFFER).0,
pending: Arc::new(Mutex::new(HashMap::new())),
supervisor_readiness,
gpu_selector: Arc::new(CdiGpuDefaultSelector::new(
cdi_gpu_inventory,
allow_all_default_gpu,
Expand Down Expand Up @@ -634,9 +617,9 @@ impl DockerComputeDriver {
let container = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?;
if let Some(sandbox) = container.and_then(|summary| {
sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref())
}) {
if let Some(sandbox) =
container.and_then(|summary| sandbox_from_container_summary(&summary))
{
return Ok(Some(sandbox));
}

Expand All @@ -647,9 +630,7 @@ impl DockerComputeDriver {
let containers = self.list_managed_container_summaries().await?;
let container_sandboxes = containers
.iter()
.filter_map(|summary| {
sandbox_from_container_summary(summary, self.supervisor_readiness.as_ref())
})
.filter_map(sandbox_from_container_summary)
.collect::<Vec<_>>();
let mut by_id = self.pending_snapshot_map().await;
for sandbox in container_sandboxes {
Expand Down Expand Up @@ -1151,8 +1132,7 @@ impl DockerComputeDriver {
if let Some(summary) = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?
&& let Some(sandbox) =
sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref())
&& let Some(sandbox) = sandbox_from_container_summary(&summary)
{
self.publish_sandbox_snapshot(sandbox);
}
Expand Down Expand Up @@ -2761,10 +2741,7 @@ fn parse_memory_limit(value: &str) -> Result<Option<i64>, Status> {
Ok(Some((amount * multiplier).round() as i64))
}

fn sandbox_from_container_summary(
summary: &ContainerSummary,
readiness: &dyn SupervisorReadiness,
) -> Option<DriverSandbox> {
fn sandbox_from_container_summary(summary: &ContainerSummary) -> Option<DriverSandbox> {
let labels = summary.labels.as_ref()?;
let id = labels.get(LABEL_SANDBOX_ID)?.clone();
let name = labels.get(LABEL_SANDBOX_NAME)?.clone();
Expand All @@ -2773,27 +2750,21 @@ fn sandbox_from_container_summary(
.cloned()
.unwrap_or_default();

let supervisor_connected = readiness.is_supervisor_connected(&id);
Some(DriverSandbox {
id,
name: name.clone(),
namespace,
spec: None,
status: Some(driver_status_from_summary(
summary,
&name,
supervisor_connected,
)),
status: Some(driver_status_from_summary(summary, &name)),
})
}

fn driver_status_from_summary(
summary: &ContainerSummary,
sandbox_name: &str,
supervisor_connected: bool,
) -> DriverSandboxStatus {
let state = summary.state.unwrap_or(ContainerSummaryStateEnum::EMPTY);
let (ready, reason, message, deleting) = container_ready_condition(state, supervisor_connected);
let (ready, reason, message, deleting) = container_ready_condition(state);

DriverSandboxStatus {
sandbox_name: summary_container_name(summary).unwrap_or_else(|| sandbox_name.to_string()),
Expand All @@ -2813,25 +2784,10 @@ fn driver_status_from_summary(

fn container_ready_condition(
state: ContainerSummaryStateEnum,
supervisor_connected: bool,
) -> (&'static str, &'static str, &'static str, bool) {
match state {
ContainerSummaryStateEnum::RUNNING => {
if supervisor_connected {
(
"True",
"SupervisorConnected",
"Supervisor relay is live",
false,
)
} else {
(
"False",
"DependenciesNotReady",
"Container is running; waiting for supervisor relay",
false,
)
}
("True", "BackendReady", "Container is running", false)
}
ContainerSummaryStateEnum::CREATED => ("False", "Starting", "Container created", false),
ContainerSummaryStateEnum::RESTARTING => (
Expand Down
42 changes: 9 additions & 33 deletions crates/openshell-driver-docker/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,6 @@ fn inspected_volume(driver: &str, options: HashMap<String, String>) -> bollard::
}
}

struct DisconnectedSupervisorReadiness;

impl SupervisorReadiness for DisconnectedSupervisorReadiness {
fn is_supervisor_connected(&self, _sandbox_id: &str) -> bool {
false
}
}

fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver {
let allow_all_default_gpu = config.allow_all_default_gpu;
DockerComputeDriver {
Expand All @@ -187,7 +179,6 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr
config,
events: broadcast::channel(WATCH_BUFFER).0,
pending: Arc::new(tokio::sync::Mutex::new(HashMap::new())),
supervisor_readiness: Arc::new(DisconnectedSupervisorReadiness),
gpu_selector: Arc::new(CdiGpuDefaultSelector::new(
CdiGpuInventory::default(),
allow_all_default_gpu,
Expand Down Expand Up @@ -1723,34 +1714,19 @@ fn driver_status_keeps_running_sandboxes_provisioning_with_stable_message() {
..running.clone()
};

let running_status = driver_status_from_summary(&running, "demo", false);
let running_later_status = driver_status_from_summary(&running_later, "demo", false);
assert_eq!(running_status.conditions[0].status, "False");
assert_eq!(running_status.conditions[0].reason, "DependenciesNotReady");
assert_eq!(
running_status.conditions[0].message,
"Container is running; waiting for supervisor relay"
);
// A running container always emits Ready=True with BackendReady. The gateway
// composes this with supervisor-session presence to decide public SandboxPhase.
let running_status = driver_status_from_summary(&running, "demo");
let running_later_status = driver_status_from_summary(&running_later, "demo");
assert_eq!(running_status.conditions[0].status, "True");
assert_eq!(running_status.conditions[0].reason, "BackendReady");
assert_eq!(running_status.conditions[0].message, "Container is running");
assert_eq!(running_status.conditions, running_later_status.conditions);

let exited_status = driver_status_from_summary(&exited, "demo", false);
let exited_status = driver_status_from_summary(&exited, "demo");
assert_eq!(exited_status.conditions[0].status, "False");
assert_eq!(exited_status.conditions[0].reason, "ContainerExited");
assert_eq!(exited_status.conditions[0].message, "Container exited");

// With a live supervisor session, a RUNNING container flips Ready=True
// so ExecSandbox and other "sandbox must be ready" gates can proceed.
let running_connected = driver_status_from_summary(&running, "demo", true);
assert_eq!(running_connected.conditions[0].status, "True");
assert_eq!(
running_connected.conditions[0].reason,
"SupervisorConnected"
);

// Supervisor readiness is ignored for non-RUNNING states -- an exited
// container must not report Ready=True.
let exited_connected = driver_status_from_summary(&exited, "demo", true);
assert_eq!(exited_connected.conditions[0].status, "False");
}

#[test]
Expand All @@ -1768,7 +1744,7 @@ fn driver_status_marks_restarting_sandboxes_as_error() {
..Default::default()
};

let status = driver_status_from_summary(&restarting, "demo", false);
let status = driver_status_from_summary(&restarting, "demo");
assert_eq!(status.conditions[0].status, "False");
assert_eq!(status.conditions[0].reason, "ContainerRestarting");
assert_eq!(
Expand Down
Loading
Loading