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
54 changes: 54 additions & 0 deletions architecture/compute-runtimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,66 @@ 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
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` with a
`Ready=False`, `SupervisorNotConnected` condition and the message
`Backend ready; waiting for supervisor session`. This distinguishes it from a sandbox
whose compute resource is still provisioning without exposing contradictory public
readiness signals.

**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.

**Known HA limitation:** Supervisor sessions are process-local while the public
sandbox phase is shared. A replica that reconciles a driver snapshot without owning
the active supervisor session can demote the shared phase to `Provisioning`. The
session-owning replica may not receive another connection event to restore `Ready`,
so a usable sandbox can remain unavailable through the public phase gate. Reliable
HA readiness requires persisted or leased supervisor presence plus routing to the
session-owning replica. That work is deferred to GitHub issue #1868. Until then,
deployments that require reliable readiness composition must run a single gateway
replica.

**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 @@ -79,17 +79,6 @@ const HOST_OPENSHELL_INTERNAL: &str = "host.openshell.internal";
const HOST_DOCKER_INTERNAL: &str = "host.docker.internal";
const DOCKER_NETWORK_DRIVER: &str = "bridge";

/// 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 @@ -212,7 +201,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 @@ -309,11 +297,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 socket_path = docker_config
.socket_path
.clone()
Expand Down Expand Up @@ -395,7 +379,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 @@ -606,9 +589,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 @@ -619,9 +602,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 @@ -1123,8 +1104,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 @@ -2738,10 +2718,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 @@ -2754,28 +2731,22 @@ 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)),
workspace,
})
}

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 @@ -2795,25 +2766,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 @@ -141,14 +141,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 @@ -159,7 +151,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 @@ -1728,34 +1719,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 @@ -1773,7 +1749,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