Skip to content

Commit 0862fc2

Browse files
committed
refactor(compute): simplify compiled driver boundary
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent 75af6cc commit 0862fc2

16 files changed

Lines changed: 479 additions & 265 deletions

File tree

architecture/compute-runtimes.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,10 @@ registry. Adding or removing a compiled driver therefore changes registration
105105
rather than the server's selection flow. Alternate gateway binaries can install
106106
their own `ComputeDriverFactory` registrations and hand the completed registry
107107
to `run_cli_with_compute_drivers`; factories receive merged driver config and
108-
finish through the same in-process runtime adapter. A configured UDS endpoint
109-
still takes precedence over a compiled registration with the same name.
108+
return either an in-process driver or a gateway-managed remote endpoint. The
109+
server constructs the common runtime adapter and snapshots `GetCapabilities`
110+
for either result. A configured UDS endpoint still takes precedence over a
111+
compiled registration with the same name.
110112

111113
The `openshell-gateway` composition crate groups first-party registrations
112114
behind the `in-tree-compute-drivers` feature. `openshell-server` has no compute
@@ -402,13 +404,13 @@ image-pull Secrets in every operator-managed namespace.
402404

403405
**Operator** uses pre-provisioned namespaces discovered through two optional
404406
sources: a K8s label selector (`operator_namespace_label`) and a drop-in
405-
allowlist file (`operator_namespace_file`). At least one must be configured.
406-
The `OperatorNamespaceAllowlist` (`Arc<RwLock<BTreeSet<String>>>`) is populated
407-
at runtime by background watchers and read by the namespace resolver. Sandbox
408-
creation fails closed if the workspace is not in the current allowlist. Platform
409-
teams manage namespace lifecycle externally. RBAC uses the same ClusterRole as
410-
managed mode but without namespace `create`/`delete` or ServiceAccount
411-
permissions.
407+
allowlist file (`operator_namespace_file`). Exactly one must be configured.
408+
The compute driver and the gateway's ServiceAccount authenticator independently
409+
watch that public config source; no in-process driver state crosses into the
410+
server. Sandbox creation and token bootstrap fail closed if the workspace is
411+
not in the current allowlist. Platform teams manage namespace lifecycle
412+
externally. RBAC uses the same ClusterRole as managed mode but without namespace
413+
`create`/`delete` or ServiceAccount permissions.
412414

413415
### Watching and Querying
414416

crates/openshell-core/src/driver_utils.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,9 @@ pub fn read_upstream_proxy_credential_file(path: &str) -> Result<String, String>
352352
///
353353
/// The resulting path is `$XDG_STATE_HOME/openshell/<driver_subdir>[/<namespace>]/<sandbox_id>/sandbox.jwt`.
354354
///
355-
/// `driver_subdir` is driver-specific, e.g. `"docker-sandbox-tokens"` or
356-
/// `"podman-sandbox-tokens"`. When `namespace` is `Some`, it is appended as
357-
/// an additional path component (with `/` and `\` replaced by `-`).
355+
/// `driver_subdir` is driver-specific. When `namespace` is `Some`, it is
356+
/// appended as an additional path component (with `/` and `\` replaced by
357+
/// `-`).
358358
///
359359
/// # Errors
360360
/// Returns an error if the XDG state directory cannot be resolved.
@@ -405,7 +405,7 @@ pub fn sandbox_log_level(sandbox: &DriverSandbox, default_level: &str) -> String
405405
}
406406

407407
// ---------------------------------------------------------------------------
408-
// Supervisor image helpers (shared by Docker and Podman drivers)
408+
// Supervisor image helpers shared by container-backed drivers
409409
// ---------------------------------------------------------------------------
410410

411411
/// Return the tag portion of a supervisor image reference, or `None` if the
@@ -440,7 +440,7 @@ pub fn supervisor_image_should_refresh(image: &str) -> bool {
440440
}
441441

442442
// ---------------------------------------------------------------------------
443-
// Supervisor binary extraction helpers (shared by Docker and Podman drivers)
443+
// Supervisor binary extraction helpers shared by container-backed drivers
444444
// ---------------------------------------------------------------------------
445445

446446
#[cfg(feature = "driver-extraction")]
@@ -517,8 +517,7 @@ pub fn write_cache_binary_atomic(final_path: &Path, bytes: &[u8]) -> Result<(),
517517
/// Return the host-side cache path for an extracted supervisor binary.
518518
///
519519
/// The path is `$XDG_DATA_HOME/openshell/<driver_subdir>/<sanitized-digest>/openshell-sandbox`.
520-
/// `driver_subdir` distinguishes caches across drivers (e.g. `"docker-supervisor"`,
521-
/// `"podman-supervisor"`).
520+
/// `driver_subdir` distinguishes caches across drivers.
522521
pub fn supervisor_cache_path(driver_subdir: &str, digest: &str) -> Result<PathBuf, String> {
523522
let base = crate::paths::xdg_data_dir()
524523
.map_err(|err| format!("failed to resolve XDG data dir: {err}"))?;

crates/openshell-core/src/error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ impl Error {
106106

107107
/// Error type shared by all compute driver implementations.
108108
///
109-
/// Both the Podman and Kubernetes drivers map their backend-specific
110-
/// errors into these variants before crossing crate boundaries.
109+
/// Drivers map backend-specific errors into these variants before crossing
110+
/// crate boundaries.
111111
#[derive(Debug, Error)]
112112
pub enum ComputeDriverError {
113113
/// The requested sandbox already exists.

crates/openshell-core/src/telemetry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ mod disabled_tests {
760760
1,
761761
false,
762762
SandboxTemplateSource::Default,
763-
TelemetryComputeDriver::Docker,
763+
TelemetryComputeDriver::from_raw("test-driver"),
764764
);
765765
emit_policy_decision(
766766
PolicyDecisionOperation::Approve,

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use openshell_core::proto::compute::v1::{
5252
use openshell_core::proto_struct::{
5353
deserialize_optional_non_empty_string_list, struct_to_json_value,
5454
};
55-
use openshell_core::{Config, Error, Result as CoreResult};
55+
use openshell_core::{Error, Result as CoreResult};
5656
use std::collections::{HashMap, HashSet};
5757
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
5858
use std::path::{Path, PathBuf};
@@ -416,7 +416,11 @@ pub fn is_available() -> bool {
416416
}
417417

418418
impl DockerComputeDriver {
419-
pub async fn new(config: &Config, docker_config: &DockerComputeConfig) -> CoreResult<Self> {
419+
pub async fn new(
420+
gateway_bind_address: SocketAddr,
421+
gateway_log_level: &str,
422+
docker_config: &DockerComputeConfig,
423+
) -> CoreResult<Self> {
420424
let socket_path = docker_config
421425
.socket_path
422426
.clone()
@@ -446,7 +450,7 @@ impl DockerComputeDriver {
446450
let cdi_gpu_inventory = docker_cdi_gpu_inventory(&info);
447451
let allow_all_default_gpu = docker_info_reports_wsl2(&info);
448452
validate_sandbox_pids_limit(docker_config.sandbox_pids_limit)?;
449-
let gateway_port = config.bind_address.port();
453+
let gateway_port = gateway_bind_address.port();
450454
if gateway_port == 0 {
451455
return Err(Error::config(
452456
"docker compute driver requires a fixed non-zero gateway bind port",
@@ -458,7 +462,7 @@ impl DockerComputeDriver {
458462
let gateway_route =
459463
docker_gateway_route(&info, bridge_gateway_ip, gateway_port, host_gateway_ip);
460464
let gateway_callback_bind_address =
461-
docker_gateway_callback_bind_address(&gateway_route, config.bind_address);
465+
docker_gateway_callback_bind_address(&gateway_route, gateway_bind_address);
462466
let mut docker_config = docker_config.clone();
463467
if docker_config.grpc_endpoint.trim().is_empty() {
464468
let scheme = if docker_guest_tls_configured(&docker_config) {
@@ -490,7 +494,7 @@ impl DockerComputeDriver {
490494
gateway_callback_bind_address,
491495
ssh_socket_path: docker_config.ssh_socket_path.clone(),
492496
stop_timeout_secs: DEFAULT_STOP_TIMEOUT_SECS,
493-
log_level: config.log_level.clone(),
497+
log_level: gateway_log_level.to_string(),
494498
supervisor_bin,
495499
guest_tls,
496500
daemon_version: version.version.unwrap_or_else(|| "unknown".to_string()),

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use std::path::PathBuf;
66

77
use clap::Parser;
88
use miette::{IntoDiagnostic, Result};
9+
use openshell_core::VERSION;
910
use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer;
10-
use openshell_core::{Config, VERSION};
1111
use openshell_driver_docker::{DockerComputeConfig, DockerComputeDriver};
1212
use tracing::info;
1313
use tracing_subscriber::EnvFilter;
@@ -46,8 +46,7 @@ async fn main() -> Result<()> {
4646

4747
let config_source = std::fs::read_to_string(&args.config).into_diagnostic()?;
4848
let docker_config: DockerComputeConfig = toml::from_str(&config_source).into_diagnostic()?;
49-
let gateway_config = Config::new(None).with_bind_address(args.gateway_bind);
50-
let driver = DockerComputeDriver::new(&gateway_config, &docker_config)
49+
let driver = DockerComputeDriver::new(args.gateway_bind, &args.log_level, &docker_config)
5150
.await
5251
.into_diagnostic()?;
5352

crates/openshell-driver-podman/src/watcher.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,12 @@ fn deleted_event(sandbox_id: String) -> WatchSandboxesEvent {
121121
/// drops (daemon restart, socket error, or clean shutdown), the stream
122122
/// terminates with a final error item and stops producing events.
123123
///
124-
/// Callers are responsible for reconnecting by calling [`start_watch`] again.
125-
/// The server's `ComputeRuntime::watch_loop` in `openshell-server` provides
126-
/// this behaviour with a 2-second backoff: when the stream terminates with an
127-
/// error, `watch_loop` sleeps and then calls `watch_sandboxes()` again, which
128-
/// ultimately calls `start_watch()` again and re-syncs state.
124+
/// Callers are responsible for reconnecting by calling [`start_watch`] again
125+
/// and re-synchronizing state.
129126
///
130127
/// **Do not add reconnection logic inside this function.** A local reconnect
131-
/// would race with `watch_loop`'s retry and produce duplicate initial-sync
132-
/// events that corrupt the server's sandbox index.
128+
/// would race with the consumer's retry and produce duplicate initial-sync
129+
/// events.
133130
pub async fn start_watch(
134131
client: PodmanClient,
135132
lifecycle_event_fences: LifecycleEventFences,

crates/openshell-gateway/src/lib.rs

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl openshell_server::ComputeDriverFactory for KubernetesFactory {
110110
async fn build(
111111
&self,
112112
context: openshell_server::ComputeDriverBuildContext<'_>,
113-
) -> openshell_core::Result<openshell_server::ComputeDriverBuildOutput> {
113+
) -> openshell_core::Result<openshell_server::ComputeDriverInstance> {
114114
let mut config: openshell_driver_kubernetes::KubernetesComputeConfig =
115115
context.driver_config()?;
116116
if let Ok(size) = std::env::var("OPENSHELL_K8S_WORKSPACE_DEFAULT_STORAGE_SIZE") {
@@ -119,27 +119,16 @@ impl openshell_server::ComputeDriverFactory for KubernetesFactory {
119119
if let Ok(storage_class) = std::env::var("OPENSHELL_K8S_WORKSPACE_STORAGE_CLASS") {
120120
config.workspace_storage_class = storage_class;
121121
}
122-
if context
123-
.gateway_config()
124-
.gateway_jwt
125-
.as_ref()
126-
.is_some_and(|jwt| jwt.ttl_secs == 0)
127-
{
128-
tracing::warn!(
129-
"Kubernetes gateway configured with non-expiring sandbox JWTs; set gateway_jwt.ttl_secs > 0 for shared deployments"
130-
);
131-
}
132122
let driver = openshell_driver_kubernetes::KubernetesComputeDriver::new(
133123
config,
134124
context.shutdown_receiver(),
135125
)
136126
.await
137127
.map_err(|error| openshell_core::Error::execution(error.to_string()))?;
138-
let allowlist = driver.operator_allowlist().cloned();
139128
let driver = openshell_driver_kubernetes::ComputeDriverService::new(driver);
140-
context
141-
.finish_in_process_with_allowlist(std::sync::Arc::new(driver), allowlist)
142-
.await
129+
Ok(openshell_server::ComputeDriverInstance::InProcess(
130+
std::sync::Arc::new(driver),
131+
))
143132
}
144133
}
145134

@@ -153,19 +142,24 @@ impl openshell_server::ComputeDriverFactory for DockerFactory {
153142
async fn build(
154143
&self,
155144
context: openshell_server::ComputeDriverBuildContext<'_>,
156-
) -> openshell_core::Result<openshell_server::ComputeDriverBuildOutput> {
145+
) -> openshell_core::Result<openshell_server::ComputeDriverInstance> {
157146
let mut config: openshell_driver_docker::DockerComputeConfig = context.driver_config()?;
158147
apply_guest_tls(
159148
&mut config.guest_tls_ca,
160149
&mut config.guest_tls_cert,
161150
&mut config.guest_tls_key,
162151
context.guest_tls_paths(),
163152
);
164-
let driver =
165-
openshell_driver_docker::DockerComputeDriver::new(context.gateway_config(), &config)
166-
.await
167-
.map_err(|error| openshell_core::Error::execution(error.to_string()))?;
168-
context.finish_in_process(std::sync::Arc::new(driver)).await
153+
let driver = openshell_driver_docker::DockerComputeDriver::new(
154+
context.gateway_bind_address(),
155+
context.gateway_log_level(),
156+
&config,
157+
)
158+
.await
159+
.map_err(|error| openshell_core::Error::execution(error.to_string()))?;
160+
Ok(openshell_server::ComputeDriverInstance::InProcess(
161+
std::sync::Arc::new(driver),
162+
))
169163
}
170164
}
171165

@@ -179,7 +173,7 @@ impl openshell_server::ComputeDriverFactory for PodmanFactory {
179173
async fn build(
180174
&self,
181175
context: openshell_server::ComputeDriverBuildContext<'_>,
182-
) -> openshell_core::Result<openshell_server::ComputeDriverBuildOutput> {
176+
) -> openshell_core::Result<openshell_server::ComputeDriverInstance> {
183177
let mut config: openshell_driver_podman::PodmanComputeConfig = context.driver_config()?;
184178
config.gateway_port = context.gateway_port();
185179
if let Ok(path) = std::env::var("OPENSHELL_PODMAN_SOCKET") {
@@ -201,7 +195,9 @@ impl openshell_server::ComputeDriverFactory for PodmanFactory {
201195
.await
202196
.map_err(|error| openshell_core::Error::execution(error.to_string()))?;
203197
let driver = openshell_driver_podman::ComputeDriverService::new(driver);
204-
context.finish_in_process(std::sync::Arc::new(driver)).await
198+
Ok(openshell_server::ComputeDriverInstance::InProcess(
199+
std::sync::Arc::new(driver),
200+
))
205201
}
206202
}
207203

@@ -215,7 +211,7 @@ impl openshell_server::ComputeDriverFactory for VmFactory {
215211
async fn build(
216212
&self,
217213
context: openshell_server::ComputeDriverBuildContext<'_>,
218-
) -> openshell_core::Result<openshell_server::ComputeDriverBuildOutput> {
214+
) -> openshell_core::Result<openshell_server::ComputeDriverInstance> {
219215
let mut config: vm::VmComputeConfig = context.driver_config()?;
220216
if config.state_dir.as_os_str().is_empty() {
221217
config.state_dir = vm::VmComputeConfig::default_state_dir();
@@ -236,8 +232,11 @@ impl openshell_server::ComputeDriverFactory for VmFactory {
236232
&mut config.guest_tls_key,
237233
context.guest_tls_paths(),
238234
);
239-
let endpoint = vm::spawn(context.gateway_config(), &config, context.otlp_config()).await?;
240-
context.finish_remote(endpoint).await
235+
let endpoint =
236+
vm::spawn(context.gateway_log_level(), &config, context.otlp_config()).await?;
237+
Ok(openshell_server::ComputeDriverInstance::ManagedRemote(
238+
endpoint,
239+
))
241240
}
242241
}
243242

crates/openshell-gateway/src/vm.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use hyper_util::rt::TokioIo;
3535
use openshell_core::proto::compute::v1::{
3636
GetCapabilitiesRequest, compute_driver_client::ComputeDriverClient,
3737
};
38-
use openshell_core::{Config, Error, Result};
38+
use openshell_core::{Error, Result};
3939
#[cfg(unix)]
4040
use openshell_otel::TraceContextInterceptor;
4141
use openshell_server::AcquiredRemoteDriverEndpoint;
@@ -453,7 +453,7 @@ pub fn compute_driver_guest_tls_paths(
453453
/// kills the subprocess and removes the socket on drop.
454454
#[cfg(unix)]
455455
pub async fn spawn(
456-
config: &Config,
456+
gateway_log_level: &str,
457457
vm_config: &VmComputeConfig,
458458
otlp_config: Option<&OtlpConfig>,
459459
) -> Result<AcquiredRemoteDriverEndpoint> {
@@ -477,7 +477,7 @@ pub async fn spawn(
477477
command
478478
.arg("--expected-peer-pid")
479479
.arg(std::process::id().to_string());
480-
command.arg("--log-level").arg(&config.log_level);
480+
command.arg("--log-level").arg(gateway_log_level);
481481
append_otlp_args(&mut command, otlp_config);
482482
command
483483
.arg("--openshell-endpoint")
@@ -527,7 +527,7 @@ fn append_otlp_args(command: &mut Command, otlp_config: Option<&OtlpConfig>) {
527527

528528
#[cfg(not(unix))]
529529
pub async fn spawn(
530-
_config: &Config,
530+
_gateway_log_level: &str,
531531
_vm_config: &VmComputeConfig,
532532
_otlp_config: Option<&OtlpConfig>,
533533
) -> Result<AcquiredRemoteDriverEndpoint> {

0 commit comments

Comments
 (0)