Skip to content

Commit 6ea68c6

Browse files
committed
refactor(compute): make sandbox readiness gateway-owned across all drivers
Introduce compute_phase_components and apply_readiness_conditions to centralise the gateway's phase composition logic. The public SandboxPhase is now determined by combining the backend phase reported by the driver with supervisor session presence, independent of the driver implementation. Remove SupervisorReadiness from the driver contract. Running containers always report BackendReady; the gateway owns the Ready decision. Rename the dispatcher match to three arms so that status-less events for existing sandboxes are a documented no-op rather than a silent pass-through. Drop backend_ready_no_session and the SupervisorNotConnected condition. The BackendReady driver condition plus the Provisioning phase already communicates that the backend is up but the supervisor has not connected. The redundant condition added noise without new information. Closes #1951 Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent dd62c85 commit 6ea68c6

5 files changed

Lines changed: 431 additions & 137 deletions

File tree

architecture/compute-runtimes.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,60 @@ Each runtime receives a sandbox spec from the gateway and is responsible for:
1616
- Reporting lifecycle and platform events back to the gateway.
1717
- Cleaning up runtime-owned resources.
1818

19+
Drivers report **backend state only**. A driver snapshot with `Ready=True` means
20+
the underlying compute resource (container, pod, VM) is healthy and running —
21+
nothing more. Drivers must not gate on supervisor session state or hold
22+
references to gateway-internal types. The gateway owns the public
23+
`SandboxPhase::Ready` decision. This applies equally to extension drivers
24+
implementing `ComputeDriver` out of tree.
25+
1926
Drivers own runtime-specific platform event interpretation. When an event should
2027
drive client provisioning UI, the driver attaches the shared
2128
`openshell.progress.*` metadata defined in `openshell-core` instead of requiring
2229
clients to parse Kubernetes reasons, VM cache states, or other driver-local
2330
reason strings.
2431

32+
## Sandbox Readiness Composition
33+
34+
The gateway composes driver backend state with supervisor session presence to
35+
produce the public `SandboxPhase`. This composition is gateway-owned and applied
36+
uniformly across all drivers:
37+
38+
```
39+
backend_phase = derive_phase(driver_status)
40+
41+
public_phase =
42+
if backend_phase in {Error, Deleting}: → pass through (terminal precedence)
43+
if backend_phase == Ready && session connected: → Ready
44+
if backend_phase == Ready && no session: → Provisioning
45+
if backend_phase in {Provisioning, Unknown} && session: → Ready
46+
if backend_phase in {Provisioning, Unknown} && no session: → Provisioning
47+
```
48+
49+
When `public_phase == Ready` the sandbox is usable through the gateway — both the
50+
backend resource is healthy and a supervisor session is registered. A sandbox whose
51+
backend reports ready but has no supervisor session yet holds `Provisioning`; the
52+
driver's `BackendReady=True` condition is visible in the sandbox status for operators
53+
who need to distinguish that state from a sandbox still provisioning its compute resource.
54+
55+
**Session precedence over lagging driver snapshots:** A supervisor session can only be
56+
established by a running workload. When `set_supervisor_session_state` promotes the
57+
store record to `Ready` on session connect, a driver watch event may still arrive
58+
shortly after carrying a stale `Provisioning` or `Unknown` backend phase. The
59+
composition rule treats a connected session as the stronger signal and keeps `Ready`
60+
in that case, preventing a lagging snapshot from undoing the session-driven promotion.
61+
62+
**HA deployments:** Supervisor sessions are process-local. A gateway replica that
63+
does not own the active supervisor session holds the public phase at `Provisioning`.
64+
The owning replica's `supervisor_session_connected` write propagates through the
65+
shared store and reconcile loop. This is correct behavior — a replica should not
66+
claim `Ready` for a session it does not hold.
67+
68+
**Extension point:** The readiness decision is a safety invariant, not an
69+
operator-configurable hook. The driver contract is the correct extension point for
70+
custom backend readiness semantics. RFC-0010 lifecycle hooks may observe readiness
71+
transitions via `post_commit`; they do not override the composition rule.
72+
2573
The capability RPC reports driver identity, version, and the default sandbox
2674
image used by the gateway. GPU availability stays driver-local and is validated
2775
when a sandbox create request asks for GPU resources.

crates/openshell-driver-docker/src/lib.rs

Lines changed: 10 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -127,17 +127,6 @@ fn resolve_default_docker_supervisor_image_tag(
127127
tag.replace('+', "-")
128128
}
129129

130-
/// Queried by the Docker driver to decide when a sandbox's supervisor
131-
/// relay is live. Implementations return `true` once a sandbox has an
132-
/// active `ConnectSupervisor` session registered.
133-
///
134-
/// The driver cannot observe the supervisor's SSH socket directly (it
135-
/// lives inside the container), so it leans on this signal to flip the
136-
/// Ready condition from `DependenciesNotReady` to `True`.
137-
pub trait SupervisorReadiness: Send + Sync + 'static {
138-
fn is_supervisor_connected(&self, sandbox_id: &str) -> bool;
139-
}
140-
141130
/// Gateway-local configuration for the Docker compute driver.
142131
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
143132
#[serde(default, deny_unknown_fields)]
@@ -254,7 +243,6 @@ pub struct DockerComputeDriver {
254243
config: DockerDriverRuntimeConfig,
255244
events: broadcast::Sender<WatchSandboxesEvent>,
256245
pending: Arc<Mutex<HashMap<String, PendingSandboxRecord>>>,
257-
supervisor_readiness: Arc<dyn SupervisorReadiness>,
258246
gpu_selector: Arc<CdiGpuDefaultSelector>,
259247
}
260248

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

353341
impl DockerComputeDriver {
354-
pub async fn new(
355-
config: &Config,
356-
docker_config: &DockerComputeConfig,
357-
supervisor_readiness: Arc<dyn SupervisorReadiness>,
358-
) -> CoreResult<Self> {
342+
pub async fn new(config: &Config, docker_config: &DockerComputeConfig) -> CoreResult<Self> {
359343
let docker = Docker::connect_with_local_defaults()
360344
.map_err(|err| Error::execution(format!("failed to create Docker client: {err}")))?;
361345
let version = docker.version().await.map_err(|err| {
@@ -423,7 +407,6 @@ impl DockerComputeDriver {
423407
},
424408
events: broadcast::channel(WATCH_BUFFER).0,
425409
pending: Arc::new(Mutex::new(HashMap::new())),
426-
supervisor_readiness,
427410
gpu_selector: Arc::new(CdiGpuDefaultSelector::new(
428411
cdi_gpu_inventory,
429412
allow_all_default_gpu,
@@ -634,9 +617,9 @@ impl DockerComputeDriver {
634617
let container = self
635618
.find_managed_container_summary(sandbox_id, sandbox_name)
636619
.await?;
637-
if let Some(sandbox) = container.and_then(|summary| {
638-
sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref())
639-
}) {
620+
if let Some(sandbox) =
621+
container.and_then(|summary| sandbox_from_container_summary(&summary))
622+
{
640623
return Ok(Some(sandbox));
641624
}
642625

@@ -647,9 +630,7 @@ impl DockerComputeDriver {
647630
let containers = self.list_managed_container_summaries().await?;
648631
let container_sandboxes = containers
649632
.iter()
650-
.filter_map(|summary| {
651-
sandbox_from_container_summary(summary, self.supervisor_readiness.as_ref())
652-
})
633+
.filter_map(sandbox_from_container_summary)
653634
.collect::<Vec<_>>();
654635
let mut by_id = self.pending_snapshot_map().await;
655636
for sandbox in container_sandboxes {
@@ -1151,8 +1132,7 @@ impl DockerComputeDriver {
11511132
if let Some(summary) = self
11521133
.find_managed_container_summary(sandbox_id, sandbox_name)
11531134
.await?
1154-
&& let Some(sandbox) =
1155-
sandbox_from_container_summary(&summary, self.supervisor_readiness.as_ref())
1135+
&& let Some(sandbox) = sandbox_from_container_summary(&summary)
11561136
{
11571137
self.publish_sandbox_snapshot(sandbox);
11581138
}
@@ -2761,10 +2741,7 @@ fn parse_memory_limit(value: &str) -> Result<Option<i64>, Status> {
27612741
Ok(Some((amount * multiplier).round() as i64))
27622742
}
27632743

2764-
fn sandbox_from_container_summary(
2765-
summary: &ContainerSummary,
2766-
readiness: &dyn SupervisorReadiness,
2767-
) -> Option<DriverSandbox> {
2744+
fn sandbox_from_container_summary(summary: &ContainerSummary) -> Option<DriverSandbox> {
27682745
let labels = summary.labels.as_ref()?;
27692746
let id = labels.get(LABEL_SANDBOX_ID)?.clone();
27702747
let name = labels.get(LABEL_SANDBOX_NAME)?.clone();
@@ -2773,27 +2750,21 @@ fn sandbox_from_container_summary(
27732750
.cloned()
27742751
.unwrap_or_default();
27752752

2776-
let supervisor_connected = readiness.is_supervisor_connected(&id);
27772753
Some(DriverSandbox {
27782754
id,
27792755
name: name.clone(),
27802756
namespace,
27812757
spec: None,
2782-
status: Some(driver_status_from_summary(
2783-
summary,
2784-
&name,
2785-
supervisor_connected,
2786-
)),
2758+
status: Some(driver_status_from_summary(summary, &name)),
27872759
})
27882760
}
27892761

27902762
fn driver_status_from_summary(
27912763
summary: &ContainerSummary,
27922764
sandbox_name: &str,
2793-
supervisor_connected: bool,
27942765
) -> DriverSandboxStatus {
27952766
let state = summary.state.unwrap_or(ContainerSummaryStateEnum::EMPTY);
2796-
let (ready, reason, message, deleting) = container_ready_condition(state, supervisor_connected);
2767+
let (ready, reason, message, deleting) = container_ready_condition(state);
27972768

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

28142785
fn container_ready_condition(
28152786
state: ContainerSummaryStateEnum,
2816-
supervisor_connected: bool,
28172787
) -> (&'static str, &'static str, &'static str, bool) {
28182788
match state {
28192789
ContainerSummaryStateEnum::RUNNING => {
2820-
if supervisor_connected {
2821-
(
2822-
"True",
2823-
"SupervisorConnected",
2824-
"Supervisor relay is live",
2825-
false,
2826-
)
2827-
} else {
2828-
(
2829-
"False",
2830-
"DependenciesNotReady",
2831-
"Container is running; waiting for supervisor relay",
2832-
false,
2833-
)
2834-
}
2790+
("True", "BackendReady", "Container is running", false)
28352791
}
28362792
ContainerSummaryStateEnum::CREATED => ("False", "Starting", "Container created", false),
28372793
ContainerSummaryStateEnum::RESTARTING => (

crates/openshell-driver-docker/src/tests.rs

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,6 @@ fn inspected_volume(driver: &str, options: HashMap<String, String>) -> bollard::
169169
}
170170
}
171171

172-
struct DisconnectedSupervisorReadiness;
173-
174-
impl SupervisorReadiness for DisconnectedSupervisorReadiness {
175-
fn is_supervisor_connected(&self, _sandbox_id: &str) -> bool {
176-
false
177-
}
178-
}
179-
180172
fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver {
181173
let allow_all_default_gpu = config.allow_all_default_gpu;
182174
DockerComputeDriver {
@@ -187,7 +179,6 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr
187179
config,
188180
events: broadcast::channel(WATCH_BUFFER).0,
189181
pending: Arc::new(tokio::sync::Mutex::new(HashMap::new())),
190-
supervisor_readiness: Arc::new(DisconnectedSupervisorReadiness),
191182
gpu_selector: Arc::new(CdiGpuDefaultSelector::new(
192183
CdiGpuInventory::default(),
193184
allow_all_default_gpu,
@@ -1723,34 +1714,19 @@ fn driver_status_keeps_running_sandboxes_provisioning_with_stable_message() {
17231714
..running.clone()
17241715
};
17251716

1726-
let running_status = driver_status_from_summary(&running, "demo", false);
1727-
let running_later_status = driver_status_from_summary(&running_later, "demo", false);
1728-
assert_eq!(running_status.conditions[0].status, "False");
1729-
assert_eq!(running_status.conditions[0].reason, "DependenciesNotReady");
1730-
assert_eq!(
1731-
running_status.conditions[0].message,
1732-
"Container is running; waiting for supervisor relay"
1733-
);
1717+
// A running container always emits Ready=True with BackendReady. The gateway
1718+
// composes this with supervisor-session presence to decide public SandboxPhase.
1719+
let running_status = driver_status_from_summary(&running, "demo");
1720+
let running_later_status = driver_status_from_summary(&running_later, "demo");
1721+
assert_eq!(running_status.conditions[0].status, "True");
1722+
assert_eq!(running_status.conditions[0].reason, "BackendReady");
1723+
assert_eq!(running_status.conditions[0].message, "Container is running");
17341724
assert_eq!(running_status.conditions, running_later_status.conditions);
17351725

1736-
let exited_status = driver_status_from_summary(&exited, "demo", false);
1726+
let exited_status = driver_status_from_summary(&exited, "demo");
17371727
assert_eq!(exited_status.conditions[0].status, "False");
17381728
assert_eq!(exited_status.conditions[0].reason, "ContainerExited");
17391729
assert_eq!(exited_status.conditions[0].message, "Container exited");
1740-
1741-
// With a live supervisor session, a RUNNING container flips Ready=True
1742-
// so ExecSandbox and other "sandbox must be ready" gates can proceed.
1743-
let running_connected = driver_status_from_summary(&running, "demo", true);
1744-
assert_eq!(running_connected.conditions[0].status, "True");
1745-
assert_eq!(
1746-
running_connected.conditions[0].reason,
1747-
"SupervisorConnected"
1748-
);
1749-
1750-
// Supervisor readiness is ignored for non-RUNNING states -- an exited
1751-
// container must not report Ready=True.
1752-
let exited_connected = driver_status_from_summary(&exited, "demo", true);
1753-
assert_eq!(exited_connected.conditions[0].status, "False");
17541730
}
17551731

17561732
#[test]
@@ -1768,7 +1744,7 @@ fn driver_status_marks_restarting_sandboxes_as_error() {
17681744
..Default::default()
17691745
};
17701746

1771-
let status = driver_status_from_summary(&restarting, "demo", false);
1747+
let status = driver_status_from_summary(&restarting, "demo");
17721748
assert_eq!(status.conditions[0].status, "False");
17731749
assert_eq!(status.conditions[0].reason, "ContainerRestarting");
17741750
assert_eq!(

0 commit comments

Comments
 (0)