Skip to content

Commit 48c9287

Browse files
drewelezar
authored andcommitted
refactor(compute): clarify lifecycle ownership
Signed-off-by: Drew Newberry <[email protected]>
1 parent 299fcd6 commit 48c9287

12 files changed

Lines changed: 59 additions & 77 deletions

File tree

‎architecture/compute-runtimes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ cluster-owned and continue running without gateway shutdown or startup
127127
lifecycle calls.
128128

129129
The driver reports this behavior through
130-
`GetCapabilities.gateway_managed_lifecycle`. The same declaration works for
130+
`GetCapabilities.gateway_manages_lifecycle`. The same declaration works for
131131
in-process and external drivers. Older drivers omit the field and retain the
132132
conservative operator-managed behavior.
133133

‎crates/openshell-core/src/driver_utils.rs‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
66
use std::path::{Path, PathBuf};
77

8-
use crate::proto::compute::v1::{DriverSandbox, GetCapabilitiesResponse};
8+
use crate::proto::compute::v1::DriverSandbox;
99

1010
pub use crate::container_paths::{
1111
SANDBOX_TOKEN_MOUNT_PATH, SUPERVISOR_CONTAINER_BINARY, SUPERVISOR_CONTAINER_DIR,
@@ -372,24 +372,6 @@ pub fn sandbox_token_path(
372372
Ok(path.join(sandbox_id).join("sandbox.jwt"))
373373
}
374374

375-
/// Build a [`GetCapabilitiesResponse`] from the common driver capability fields.
376-
///
377-
/// Every compute driver constructs this response with the same fields. Shared
378-
/// here to avoid repeating the struct literal in each driver crate.
379-
pub fn build_capabilities_response(
380-
driver_name: &str,
381-
driver_version: impl Into<String>,
382-
default_image: impl Into<String>,
383-
gateway_managed_lifecycle: bool,
384-
) -> GetCapabilitiesResponse {
385-
GetCapabilitiesResponse {
386-
driver_name: driver_name.to_string(),
387-
driver_version: driver_version.into(),
388-
default_image: default_image.into(),
389-
gateway_managed_lifecycle,
390-
}
391-
}
392-
393375
/// Return the effective log level for a sandbox.
394376
///
395377
/// Uses the level from the sandbox spec when non-empty, falling back to

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -490,12 +490,12 @@ impl DockerComputeDriver {
490490
}
491491

492492
fn capabilities(&self) -> GetCapabilitiesResponse {
493-
openshell_core::driver_utils::build_capabilities_response(
494-
"docker",
495-
&self.config.daemon_version,
496-
&self.config.default_image,
497-
true,
498-
)
493+
GetCapabilitiesResponse {
494+
driver_name: "docker".to_string(),
495+
driver_version: self.config.daemon_version.clone(),
496+
default_image: self.config.default_image.clone(),
497+
gateway_manages_lifecycle: true,
498+
}
499499
}
500500

501501
#[cfg(test)]

‎crates/openshell-driver-kubernetes/src/driver.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,12 @@ impl KubernetesComputeDriver {
544544
}
545545

546546
pub fn capabilities(&self) -> Result<GetCapabilitiesResponse, String> {
547-
Ok(openshell_core::driver_utils::build_capabilities_response(
548-
"kubernetes",
549-
openshell_core::VERSION,
550-
&self.config.default_image,
551-
false,
552-
))
547+
Ok(GetCapabilitiesResponse {
548+
driver_name: "kubernetes".to_string(),
549+
driver_version: openshell_core::VERSION.to_string(),
550+
default_image: self.config.default_image.clone(),
551+
gateway_manages_lifecycle: false,
552+
})
553553
}
554554

555555
pub fn operator_allowlist(&self) -> Option<&OperatorNamespaceAllowlist> {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -467,12 +467,12 @@ impl PodmanComputeDriver {
467467

468468
/// Report driver capabilities.
469469
pub fn capabilities(&self) -> Result<GetCapabilitiesResponse, ComputeDriverError> {
470-
Ok(openshell_core::driver_utils::build_capabilities_response(
471-
"podman",
472-
openshell_core::VERSION,
473-
&self.config.default_image,
474-
true,
475-
))
470+
Ok(GetCapabilitiesResponse {
471+
driver_name: "podman".to_string(),
472+
driver_version: openshell_core::VERSION.to_string(),
473+
default_image: self.config.default_image.clone(),
474+
gateway_manages_lifecycle: true,
475+
})
476476
}
477477

478478
/// Report the gateway exposure needed by Podman's standard local callback aliases.

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ impl VmDriver {
520520
driver_name: DRIVER_NAME.to_string(),
521521
driver_version: openshell_core::VERSION.to_string(),
522522
default_image: self.config.default_image.clone(),
523-
gateway_managed_lifecycle: true,
523+
gateway_manages_lifecycle: true,
524524
}
525525
}
526526

‎crates/openshell-server/src/cli.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,10 @@ struct RunArgs {
115115
/// implementing `compute_driver.proto`.
116116
///
117117
/// When set, the socket is associated with the single driver name supplied
118-
/// by `--drivers` or `OPENSHELL_DRIVERS`. The endpoint overrides built-in
119-
/// construction when the selected name is Docker, Podman, Kubernetes, or
120-
/// VM.
118+
/// by `--drivers` or `OPENSHELL_DRIVERS` and replaces normal construction
119+
/// for that selected name, including canonical built-in names. The gateway
120+
/// connects to this operator-provided endpoint; it does not provision the
121+
/// remote driver.
121122
#[arg(long, env = "OPENSHELL_COMPUTE_DRIVER_SOCKET")]
122123
compute_driver_socket: Option<PathBuf>,
123124

‎crates/openshell-server/src/compute/mod.rs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ pub struct ComputeDriverInfoSnapshot {
278278
/// Driver-reported implementation version from the startup capability snapshot.
279279
pub driver_version: String,
280280
/// Whether the driver asks the gateway to reconcile compute across restarts.
281-
pub gateway_managed_lifecycle: bool,
281+
pub gateway_manages_lifecycle: bool,
282282
}
283283

284284
/// Interval between store-vs-backend reconciliation sweeps.
@@ -614,7 +614,7 @@ impl ComputeRuntime {
614614
name: driver_name.clone(),
615615
driver_name: capabilities.driver_name,
616616
driver_version: capabilities.driver_version,
617-
gateway_managed_lifecycle: capabilities.gateway_managed_lifecycle,
617+
gateway_manages_lifecycle: capabilities.gateway_manages_lifecycle,
618618
};
619619
let default_image = capabilities.default_image;
620620
let gateway_listener_requirements = match driver
@@ -2047,7 +2047,7 @@ impl ComputeRuntime {
20472047
/// An explicit sandbox stop persists `Stopped`; gateway shutdown does not.
20482048
/// Drivers request this sweep through their startup capability snapshot.
20492049
async fn stop_persisted_sandboxes_on_shutdown(&self) -> Result<(), String> {
2050-
if !self.driver_info.gateway_managed_lifecycle {
2050+
if !self.driver_info.gateway_manages_lifecycle {
20512051
return Ok(());
20522052
}
20532053

@@ -2144,7 +2144,7 @@ impl ComputeRuntime {
21442144
/// so the watch loop sees the post-start state on its first poll.
21452145
pub async fn start_persisted_sandboxes(&self) -> Result<(), String> {
21462146
self.recover_persisted_lifecycle_transitions().await?;
2147-
if !self.driver_info.gateway_managed_lifecycle {
2147+
if !self.driver_info.gateway_manages_lifecycle {
21482148
return Ok(());
21492149
}
21502150

@@ -3943,7 +3943,7 @@ impl ComputeDriver for NoopTestDriver {
39433943
driver_name: "noop-test-driver".to_string(),
39443944
driver_version: "test".to_string(),
39453945
default_image: "openshell/sandbox:test".to_string(),
3946-
gateway_managed_lifecycle: false,
3946+
gateway_manages_lifecycle: false,
39473947
},
39483948
))
39493949
}
@@ -4084,7 +4084,7 @@ pub async fn new_test_runtime_with_driver(
40844084
name: driver_name.to_string(),
40854085
driver_name: driver_name.to_string(),
40864086
driver_version: "test".to_string(),
4087-
gateway_managed_lifecycle: false,
4087+
gateway_manages_lifecycle: false,
40884088
},
40894089
driver_process: None,
40904090
default_image: "openshell/sandbox:test".to_string(),
@@ -4247,7 +4247,7 @@ mod tests {
42474247
driver_name: "test-driver".to_string(),
42484248
driver_version: "test".to_string(),
42494249
default_image: "openshell/sandbox:test".to_string(),
4250-
gateway_managed_lifecycle: false,
4250+
gateway_manages_lifecycle: false,
42514251
}))
42524252
}
42534253

@@ -4565,7 +4565,7 @@ mod tests {
45654565
driver_name: "controlled-test-driver".to_string(),
45664566
driver_version: "test".to_string(),
45674567
default_image: "openshell/sandbox:test".to_string(),
4568-
gateway_managed_lifecycle: false,
4568+
gateway_manages_lifecycle: false,
45694569
}))
45704570
}
45714571

@@ -4768,7 +4768,7 @@ mod tests {
47684768
name: driver_name.to_string(),
47694769
driver_name: driver_name.to_string(),
47704770
driver_version: "test".to_string(),
4771-
gateway_managed_lifecycle: false,
4771+
gateway_manages_lifecycle: false,
47724772
},
47734773
driver_process: None,
47744774
default_image: "openshell/sandbox:test".to_string(),
@@ -4789,7 +4789,7 @@ mod tests {
47894789
driver_name: &str,
47904790
) -> ComputeRuntime {
47914791
let mut runtime = test_runtime_for_driver(driver, driver_name).await;
4792-
runtime.driver_info.gateway_managed_lifecycle = true;
4792+
runtime.driver_info.gateway_manages_lifecycle = true;
47934793
runtime
47944794
}
47954795

@@ -8422,7 +8422,8 @@ mod tests {
84228422
#[tokio::test]
84238423
async fn startup_sweep_rechecks_intent_after_acquiring_gate() {
84248424
let driver = ControlledDriver::new();
8425-
let runtime = test_runtime_for_driver(driver.clone(), "docker").await;
8425+
let runtime =
8426+
test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await;
84268427
runtime
84278428
.store
84288429
.put_message(&sandbox_record("sb-1", "sandbox", SandboxPhase::Ready))
@@ -8458,7 +8459,8 @@ mod tests {
84588459
#[tokio::test]
84598460
async fn lifecycle_sweeps_page_through_all_persisted_sandboxes() {
84608461
let driver = ControlledDriver::new();
8461-
let runtime = test_runtime_for_driver(driver.clone(), "docker").await;
8462+
let runtime =
8463+
test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await;
84628464
let sandbox_count = LIFECYCLE_SWEEP_PAGE_SIZE + 1;
84638465
for index in 0..sandbox_count {
84648466
runtime
@@ -8790,7 +8792,7 @@ mod tests {
87908792
let driver = FakeComputeDriver::new()
87918793
.with_driver_name("fake-remote-driver")
87928794
.with_default_image("openshell/sandbox:remote")
8793-
.with_gateway_managed_lifecycle()
8795+
.with_gateway_manages_lifecycle()
87948796
.with_gateway_listener_requirement(
87958797
"172.19.0.1:17670",
87968798
"external driver managed bridge",

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2700,7 +2700,7 @@ async fn handle_update_config_inner(
27002700
"delete_setting cannot be combined with policy payload",
27012701
));
27022702
}
2703-
let new_policy = req.policy.ok_or_else(|| {
2703+
let mut new_policy = req.policy.ok_or_else(|| {
27042704
Status::invalid_argument("policy is required for global policy update")
27052705
})?;
27062706
clear_provider_credentialed_markers(&mut new_policy);

‎crates/openshell-server/src/lib.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1248,11 +1248,14 @@ fn resolve_configured_compute_driver(
12481248
) -> Result<ConfiguredComputeDriver> {
12491249
let name = openshell_core::config::normalize_compute_driver_name(driver_name)
12501250
.map_err(Error::config)?;
1251-
let driver_kind = builtin_compute_driver(&name);
1251+
// An operator-provided endpoint replaces normal construction for the
1252+
// selected name. The gateway connects to it; it does not provision a
1253+
// remote implementation for canonical built-in names.
12521254
if driver_startup.endpoint_overrides.contains_key(&name) {
12531255
return Ok(ConfiguredComputeDriver::Remote { name });
12541256
}
12551257

1258+
let driver_kind = builtin_compute_driver(&name);
12561259
if let Some(kind) = driver_kind {
12571260
return Ok(ConfiguredComputeDriver::Builtin(kind));
12581261
}

0 commit comments

Comments
 (0)