trending/OpenShell
1
0
Fork 0
mirror of https://github.com/NVIDIA/OpenShell.git synced 2026-10-02 07:34:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat(compute): make driver lifecycle id-only

Browse Source 
Signed-off-by: Evan Lezar <elezar@nvidia.com>
This commit is contained in:
Evan Lezar
2026-09-23 15:46:27 +02:00
parent d3480d2a7e
commit 579ecfe0d1
13 changed files with 213 additions and 626 deletions
Download Patch File Download Diff File Expand all files Collapse all files
architecture/compute-runtimes.md
+7
View File
@@ -195,6 +195,13 @@ same resource. The gateway requires a fresh supervisor session before a
starting sandbox returns to `Ready`; stale driver snapshots and supervisor
sessions cannot promote a `Stopped` row.
The gateway addresses `GetSandbox`, `StopSandbox`, `StartSandbox`, and
`DeleteSandbox` exclusively by the immutable `sandbox_id` assigned at create
time. Drivers reject an empty ID and do not resolve lifecycle requests by
sandbox name. `ListSandboxes` returns the driver's complete, unscoped
inventory; each snapshot has a non-empty, globally unique ID, while its name
and workspace remain descriptive metadata.
Runtime credentials are generation-scoped and memory-only after launch. A
supervisor or Sandbox Runtime process replacement does not resume a running
generation. Planned upgrades stop the sandbox first; the following start mints
crates/openshell-driver-docker/src/lib.rs
+59 -192
View File
@@ -395,44 +395,28 @@ impl DockerLifecycleEventFences {
.contains(sandbox_id)
}
fn request_stop(&self, sandbox_id: &str, sandbox_name: &str) {
let mut state = self
.state
fn request_stop(&self, sandbox_id: &str) {
self.state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
if !sandbox_id.is_empty() {
state.stops_requested.insert(format!("id:{sandbox_id}"));
}
if !sandbox_name.is_empty() {
state.stops_requested.insert(format!("name:{sandbox_name}"));
}
.unwrap_or_else(std::sync::PoisonError::into_inner)
.stops_requested
.insert(sandbox_id.to_string());
}
fn clear_stop(&self, sandbox_id: &str, sandbox_name: &str) {
let mut state = self
.state
fn clear_stop(&self, sandbox_id: &str) {
self.state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
if !sandbox_id.is_empty() {
state.stops_requested.remove(&format!("id:{sandbox_id}"));
}
if !sandbox_name.is_empty() {
state
.stops_requested
.remove(&format!("name:{sandbox_name}"));
}
.unwrap_or_else(std::sync::PoisonError::into_inner)
.stops_requested
.remove(sandbox_id);
}
fn stop_requested(&self, sandbox_id: &str, sandbox_name: &str) -> bool {
let state = self
.state
fn stop_requested(&self, sandbox_id: &str) -> bool {
self.state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
(!sandbox_id.is_empty() && state.stops_requested.contains(&format!("id:{sandbox_id}")))
|| (!sandbox_name.is_empty()
&& state
.stops_requested
.contains(&format!("name:{sandbox_name}")))
.unwrap_or_else(std::sync::PoisonError::into_inner)
.stops_requested
.contains(sandbox_id)
}
fn record_previous_exit(&self, sandbox_id: &str, finished_at: Option<&str>) {
@@ -461,17 +445,14 @@ impl DockerLifecycleEventFences {
.cloned()
}
fn remove(&self, sandbox_id: &str, sandbox_name: &str) {
fn remove(&self, sandbox_id: &str) {
let mut state = self
.state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
state.previous_finished_at.remove(sandbox_id);
state.starts_in_progress.remove(sandbox_id);
state.stops_requested.remove(&format!("id:{sandbox_id}"));
state
.stops_requested
.remove(&format!("name:{sandbox_name}"));
state.stops_requested.remove(sandbox_id);
}
}
@@ -1392,14 +1373,11 @@ impl DockerComputeDriver {
async fn get_sandbox_snapshot(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<DriverSandbox>, Status> {
if let Some(pending) = self.pending_snapshot(sandbox_id, sandbox_name).await? {
if let Some(pending) = self.pending_snapshot(sandbox_id).await {
return Ok(Some(pending));
}
let container = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?;
let container = self.find_managed_container_summary(sandbox_id).await?;
if let Some(mut sandbox) =
container.and_then(|summary| sandbox_from_container_summary(&summary))
{
@@ -1485,7 +1463,7 @@ impl DockerComputeDriver {
.await?;
if self
.find_managed_container_summary(&sandbox.id, &sandbox.name)
.find_managed_container_summary(&sandbox.id)
.await?
.is_some()
{
@@ -1533,10 +1511,7 @@ impl DockerComputeDriver {
match Box::pin(self.provision_sandbox_inner(&sandbox)).await {
Ok(()) => {
self.clear_pending_sandbox(&sandbox.id).await;
if let Err(error) = self
.publish_container_snapshot(&sandbox.id, &sandbox.name)
.await
{
if let Err(error) = self.publish_container_snapshot(&sandbox.id).await {
warn!(
sandbox_id = %sandbox.id,
%error,
@@ -1833,10 +1808,7 @@ impl DockerComputeDriver {
{
Ok(control) => control,
Err(status) => {
if self
.lifecycle_event_fences
.stop_requested(&sandbox.id, &sandbox.name)
{
if self.lifecycle_event_fences.stop_requested(&sandbox.id) {
debug!(
sandbox_id = %sandbox.id,
"Ignoring Docker supervisor startup interruption after an explicit stop"
@@ -1859,10 +1831,7 @@ impl DockerComputeDriver {
));
}
};
if self
.lifecycle_event_fences
.stop_requested(&sandbox.id, &sandbox.name)
{
if self.lifecycle_event_fences.stop_requested(&sandbox.id) {
stop_docker_control_process(control).await;
debug!(
sandbox_id = %sandbox.id,
@@ -2160,14 +2129,8 @@ impl DockerComputeDriver {
Ok(())
}
async fn delete_sandbox_inner(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<bool, Status> {
let pending = self
.remove_pending_sandbox(sandbox_id, sandbox_name)
.await?;
async fn delete_sandbox_inner(&self, sandbox_id: &str) -> Result<bool, Status> {
let pending = self.remove_pending_sandbox(sandbox_id).await;
if let Some(record) = pending.as_ref()
&& let Some(task) = record.task.as_ref()
{
@@ -2179,10 +2142,7 @@ impl DockerComputeDriver {
.await?;
}
let Some(container) = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?
else {
let Some(container) = self.find_managed_container_summary(sandbox_id).await? else {
if let Some(record) = pending {
let container_name = container_name_for_sandbox(&record.sandbox);
match self
@@ -2274,16 +2234,12 @@ impl DockerComputeDriver {
}
}
async fn stop_sandbox_inner(&self, sandbox_id: &str, sandbox_name: &str) -> Result<(), Status> {
let container = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?;
async fn stop_sandbox_inner(&self, sandbox_id: &str) -> Result<(), Status> {
let container = self.find_managed_container_summary(sandbox_id).await?;
// Startup can still be waiting for supervisor readiness after the
// workload container exists. Cancel it before removing the supervisor
// so its failure path cannot delete retained workload storage.
let mut pending = self
.remove_pending_sandbox(sandbox_id, sandbox_name)
.await?;
let mut pending = self.remove_pending_sandbox(sandbox_id).await;
if let Some(task) = pending.as_mut().and_then(|record| record.task.take()) {
task.abort();
let _ = task.await;
@@ -2357,28 +2313,24 @@ impl DockerComputeDriver {
otel.name = "docker.start_sandbox",
otel.status_code = tracing::field::Empty,
sandbox.id = %sandbox_id,
sandbox.name = %sandbox_name,
)
)]
pub async fn start_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
generation_id: &str,
launch_authentication: &[u8],
) -> Result<bool, Status> {
let span_status = openshell_otel::ErrorStatusGuard::current();
require_sandbox_identifier(sandbox_id, sandbox_name)?;
require_sandbox_id(sandbox_id)?;
let generation = openshell_core::sandbox_generation::SandboxGenerationId::parse(
generation_id.to_string(),
)
.map_err(|error| Status::invalid_argument(error.to_string()))?;
self.lifecycle_event_fences
.clear_stop(sandbox_id, sandbox_name);
self.lifecycle_event_fences.clear_stop(sandbox_id);
self.lifecycle_event_fences.begin_start(sandbox_id);
let result = Box::pin(self.start_sandbox_with_lifecycle_fence(
sandbox_id,
sandbox_name,
&generation,
launch_authentication,
))
@@ -2390,14 +2342,10 @@ impl DockerComputeDriver {
async fn start_sandbox_with_lifecycle_fence(
&self,
sandbox_id: &str,
sandbox_name: &str,
generation: &openshell_core::sandbox_generation::SandboxGenerationId,
launch_authentication: &[u8],
) -> Result<bool, Status> {
let Some(container) = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?
else {
let Some(container) = self.find_managed_container_summary(sandbox_id).await? else {
return Ok(false);
};
let Some(target) = summary_container_target(&container) else {
@@ -2569,14 +2517,9 @@ impl DockerComputeDriver {
Ok(())
}
async fn pending_snapshot(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<DriverSandbox>, Status> {
async fn pending_snapshot(&self, sandbox_id: &str) -> Option<DriverSandbox> {
let pending = self.pending.lock().await;
let id = pending_sandbox_record_id(&pending, sandbox_id, sandbox_name)?;
Ok(id.and_then(|id| pending.get(&id).map(|record| record.sandbox.clone())))
pending.get(sandbox_id).map(|record| record.sandbox.clone())
}
async fn pending_snapshot_map(&self) -> HashMap<String, DriverSandbox> {
@@ -2592,16 +2535,9 @@ impl DockerComputeDriver {
pending.remove(sandbox_id);
}
async fn remove_pending_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<PendingSandboxRecord>, Status> {
async fn remove_pending_sandbox(&self, sandbox_id: &str) -> Option<PendingSandboxRecord> {
let mut pending = self.pending.lock().await;
let Some(id) = pending_sandbox_record_id(&pending, sandbox_id, sandbox_name)? else {
return Ok(None);
};
Ok(pending.remove(&id))
pending.remove(sandbox_id)
}
async fn fail_pending_sandbox(
@@ -2638,18 +2574,12 @@ impl DockerComputeDriver {
self.publish_sandbox_snapshot(snapshot);
}
async fn publish_container_snapshot(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<(), Status> {
if let Some(pending) = self.pending_snapshot(sandbox_id, sandbox_name).await? {
async fn publish_container_snapshot(&self, sandbox_id: &str) -> Result<(), Status> {
if let Some(pending) = self.pending_snapshot(sandbox_id).await {
self.publish_sandbox_snapshot(pending);
return Ok(());
}
if let Some(summary) = self
.find_managed_container_summary(sandbox_id, sandbox_name)
.await?
if let Some(summary) = self.find_managed_container_summary(sandbox_id).await?
&& let Some(mut sandbox) = sandbox_from_container_summary(&summary)
{
self.apply_runtime_failure(&mut sandbox).await;
@@ -2841,14 +2771,8 @@ impl DockerComputeDriver {
async fn find_managed_container_summary(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<ContainerSummary>, Status> {
let mut label_filter_values = Vec::new();
if !sandbox_id.is_empty() {
label_filter_values.push(format!("{LABEL_SANDBOX_ID}={sandbox_id}"));
} else if !sandbox_name.is_empty() {
label_filter_values.push(format!("{LABEL_SANDBOX_NAME}={sandbox_name}"));
}
let label_filter_values = vec![format!("{LABEL_SANDBOX_ID}={sandbox_id}")];
let filters =
managed_container_label_filters(&self.config.sandbox_namespace, label_filter_values);
@@ -2870,15 +2794,10 @@ impl DockerComputeDriver {
let namespace_matches = labels
.get(LABEL_SANDBOX_NAMESPACE)
.is_some_and(|value| value == &self.config.sandbox_namespace);
let id_matches = sandbox_id.is_empty()
|| labels
.get(LABEL_SANDBOX_ID)
.is_some_and(|value| value == sandbox_id);
let name_matches = sandbox_name.is_empty()
|| labels
.get(LABEL_SANDBOX_NAME)
.is_some_and(|value| value == sandbox_name);
namespace_matches && id_matches && name_matches
let id_matches = labels
.get(LABEL_SANDBOX_ID)
.is_some_and(|value| value == sandbox_id);
namespace_matches && id_matches
}))
}
@@ -3240,19 +3159,13 @@ impl ComputeDriver for DockerComputeDriver {
request: Request<GetSandboxRequest>,
) -> Result<Response<GetSandboxResponse>, Status> {
let request = request.into_inner();
require_sandbox_identifier(&request.sandbox_id, &request.name)?;
require_sandbox_id(&request.sandbox_id)?;
let sandbox = self
.get_sandbox_snapshot(&request.sandbox_id, &request.name)
.get_sandbox_snapshot(&request.sandbox_id)
.await?
.ok_or_else(|| Status::not_found("sandbox not found"))?;
if !request.sandbox_id.is_empty() && request.sandbox_id != sandbox.id {
return Err(Status::failed_precondition(
"sandbox_id did not match the fetched sandbox",
));
}
Ok(Response::new(GetSandboxResponse {
sandbox: Some(sandbox),
}))
@@ -3297,7 +3210,6 @@ impl ComputeDriver for DockerComputeDriver {
otel.name = "docker.stop_sandbox",
otel.status_code = tracing::field::Empty,
sandbox.id = %request.get_ref().sandbox_id,
sandbox.name = %request.get_ref().name,
)
)]
async fn stop_sandbox(
@@ -3306,24 +3218,16 @@ impl ComputeDriver for DockerComputeDriver {
) -> Result<Response<StopSandboxResponse>, Status> {
let span_status = openshell_otel::ErrorStatusGuard::current();
let request = request.into_inner();
require_sandbox_identifier(&request.sandbox_id, &request.name)?;
require_sandbox_id(&request.sandbox_id)?;
self.lifecycle_event_fences
.request_stop(&request.sandbox_id, &request.name);
if let Err(error) = self
.stop_sandbox_inner(&request.sandbox_id, &request.name)
.await
{
self.lifecycle_event_fences
.clear_stop(&request.sandbox_id, &request.name);
.request_stop(&request.sandbox_id);
if let Err(error) = self.stop_sandbox_inner(&request.sandbox_id).await {
self.lifecycle_event_fences.clear_stop(&request.sandbox_id);
return Err(error);
}
if let Err(error) = self
.publish_container_snapshot(&request.sandbox_id, &request.name)
.await
{
self.lifecycle_event_fences
.clear_stop(&request.sandbox_id, &request.name);
if let Err(error) = self.publish_container_snapshot(&request.sandbox_id).await {
self.lifecycle_event_fences.clear_stop(&request.sandbox_id);
return Err(error);
}
span_status.finish(Ok(Response::new(StopSandboxResponse {})))
@@ -3337,7 +3241,6 @@ impl ComputeDriver for DockerComputeDriver {
if !Box::pin(Self::start_sandbox(
self,
&request.sandbox_id,
&request.name,
&request.generation_id,
&request.launch_authentication,
))
@@ -3345,8 +3248,7 @@ impl ComputeDriver for DockerComputeDriver {
{
return Err(Status::not_found("sandbox not found"));
}
self.publish_container_snapshot(&request.sandbox_id, &request.name)
.await?;
self.publish_container_snapshot(&request.sandbox_id).await?;
Ok(Response::new(StartSandboxResponse::default()))
}
@@ -3357,7 +3259,6 @@ impl ComputeDriver for DockerComputeDriver {
otel.name = "docker.delete_sandbox",
otel.status_code = tracing::field::Empty,
sandbox.id = %request.get_ref().sandbox_id,
sandbox.name = %request.get_ref().name,
)
)]
async fn delete_sandbox(
@@ -3366,14 +3267,11 @@ impl ComputeDriver for DockerComputeDriver {
) -> Result<Response<DeleteSandboxResponse>, Status> {
let span_status = openshell_otel::ErrorStatusGuard::current();
let request = request.into_inner();
require_sandbox_identifier(&request.sandbox_id, &request.name)?;
require_sandbox_id(&request.sandbox_id)?;
let event_sandbox_id = request.sandbox_id.clone();
let deleted = self
.delete_sandbox_inner(&request.sandbox_id, &request.name)
.await?;
self.lifecycle_event_fences
.remove(&event_sandbox_id, &request.name);
let deleted = self.delete_sandbox_inner(&request.sandbox_id).await?;
self.lifecycle_event_fences.remove(&event_sandbox_id);
if deleted && !event_sandbox_id.is_empty() {
let _ = self.events.send(WatchSandboxesEvent {
payload: Some(watch_sandboxes_event::Payload::Deleted(
@@ -3502,30 +3400,6 @@ fn pending_sandbox_snapshot(
}
}
fn pending_sandbox_record_id(
pending: &HashMap<String, PendingSandboxRecord>,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<String>, Status> {
if !sandbox_id.is_empty() {
return Ok(pending
.get(sandbox_id)
.map(|record| record.sandbox.id.clone()));
}
let mut matches = pending
.values()
.filter(|record| !sandbox_name.is_empty() && record.sandbox.name == sandbox_name)
.map(|record| record.sandbox.id.clone());
let first = matches.next();
if first.is_some() && matches.next().is_some() {
return Err(Status::failed_precondition(format!(
"multiple pending Docker sandboxes are named '{sandbox_name}'; use sandbox_id"
)));
}
Ok(first)
}
fn provisioning_condition() -> DriverCondition {
DriverCondition {
r#type: "Ready".to_string(),
@@ -5815,16 +5689,9 @@ fn build_container_create_body_for_image(
})
}
/// Reject driver requests that arrive with neither a sandbox id nor a
/// sandbox name. Without this guard, downstream label filters degenerate
/// to "match every managed container in the namespace", which would let
/// `delete_sandbox`/`stop_sandbox`/`get_sandbox` pick an arbitrary
/// sandbox out of the set the driver manages.
fn require_sandbox_identifier(sandbox_id: &str, sandbox_name: &str) -> Result<(), Status> {
if sandbox_id.is_empty() && sandbox_name.is_empty() {
return Err(Status::invalid_argument(
"sandbox_id or sandbox_name is required",
));
fn require_sandbox_id(sandbox_id: &str) -> Result<(), Status> {
if sandbox_id.is_empty() {
return Err(Status::invalid_argument("sandbox_id is required"));
}
Ok(())
}
crates/openshell-driver-docker/src/tests.rs
+14 -70
View File
@@ -726,7 +726,6 @@ async fn start_sandbox_span_does_not_capture_launch_authentication() {
test_driver_with_config(runtime_config())
.start_sandbox(
"sandbox-1",
"sandbox",
"invalid-generation",
b"secret-launch-authentication",
)
@@ -817,11 +816,9 @@ async fn tracing_direct_start_exports_a_docker_start_span() {
let subscriber = tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider));
let driver = test_driver_with_config(runtime_config());
Box::pin(
DockerComputeDriver::start_sandbox(&driver, "", "", "", &[]).with_subscriber(subscriber),
)
.await
.expect_err("missing identifier should fail");
Box::pin(DockerComputeDriver::start_sandbox(&driver, "", "", &[]).with_subscriber(subscriber))
.await
.expect_err("missing identifier should fail");
provider.force_flush().unwrap();
let spans = exporter.get_finished_spans().unwrap();
@@ -2687,19 +2684,12 @@ fn docker_info_reports_wsl2_rejects_plain_linux() {
}
#[test]
fn require_sandbox_identifier_rejects_when_id_and_name_are_empty() {
// Regression test: `delete_sandbox` (and the other identifier-keyed
// RPCs) must refuse requests where both the id and the name are
// empty. Otherwise the empty filters fed to
// `find_managed_container_summary` match the first managed container
// in the namespace, allowing an arbitrary sandbox to be deleted.
let err = require_sandbox_identifier("", "").unwrap_err();
fn require_sandbox_id_rejects_empty_id() {
let err = require_sandbox_id("").unwrap_err();
assert_eq!(err.code(), tonic::Code::InvalidArgument);
assert!(err.message().contains("sandbox_id or sandbox_name"));
assert!(err.message().contains("sandbox_id"));
require_sandbox_identifier("sbx-1", "").expect("id-only is accepted");
require_sandbox_identifier("", "demo").expect("name-only is accepted");
require_sandbox_identifier("sbx-1", "demo").expect("id and name is accepted");
require_sandbox_id("sbx-1").expect("non-empty id is accepted");
}
#[test]
@@ -2976,22 +2966,6 @@ fn pending_sandbox_snapshot_uses_docker_namespace_and_starting_condition() {
assert_eq!(snapshot.name, "demo");
assert_eq!(snapshot.namespace, "docker-dev");
assert!(snapshot.spec.is_none());
let pending = HashMap::from([(
snapshot.id.clone(),
PendingSandboxRecord {
sandbox: snapshot.clone(),
task: None,
},
)]);
assert_eq!(
pending_sandbox_record_id(&pending, "sbx-123", "wrong-name").unwrap(),
Some("sbx-123".to_string())
);
assert_eq!(
pending_sandbox_record_id(&pending, "", "demo").unwrap(),
Some("sbx-123".to_string())
);
let status = snapshot.status.expect("status");
assert!(!status.deleting);
assert_eq!(status.name, "demo");
@@ -3002,34 +2976,6 @@ fn pending_sandbox_snapshot_uses_docker_namespace_and_starting_condition() {
assert_eq!(status.conditions[0].message, "Docker container is starting");
}
#[test]
fn pending_lookup_is_id_authoritative_and_rejects_ambiguous_names() {
let mut alpha = test_sandbox();
alpha.id = "sbx-alpha".to_string();
alpha.workspace = "workspace-alpha".to_string();
let mut beta = alpha.clone();
beta.id = "sbx-beta".to_string();
beta.workspace = "workspace-beta".to_string();
let pending = [alpha, beta]
.into_iter()
.map(|sandbox| {
(
sandbox.id.clone(),
PendingSandboxRecord {
sandbox,
task: None,
},
)
})
.collect();
assert_eq!(
pending_sandbox_record_id(&pending, "sbx-alpha", "demo").unwrap(),
Some("sbx-alpha".to_string())
);
assert!(pending_sandbox_record_id(&pending, "", "demo").is_err());
}
#[test]
fn workload_mounts_only_the_shared_channel_volume() {
let config = runtime_config();
@@ -3292,12 +3238,11 @@ fn lifecycle_fence_rejects_polled_exit_from_before_restart() {
fences.finish_start("sandbox-1");
assert!(!fences.start_in_progress("sandbox-1"));
fences.request_stop("sandbox-1", "demo");
assert!(fences.stop_requested("sandbox-1", ""));
assert!(fences.stop_requested("", "demo"));
fences.clear_stop("sandbox-1", "demo");
assert!(!fences.stop_requested("sandbox-1", "demo"));
fences.request_stop("sandbox-1", "demo");
fences.request_stop("sandbox-1");
assert!(fences.stop_requested("sandbox-1"));
fences.clear_stop("sandbox-1");
assert!(!fences.stop_requested("sandbox-1"));
fences.request_stop("sandbox-1");
fences.record_previous_exit("sandbox-1", Some("2026-08-12T16:39:13Z"));
assert_eq!(
@@ -3333,10 +3278,9 @@ fn lifecycle_fence_rejects_polled_exit_from_before_restart() {
Some(&new_exit),
));
fences.remove("sandbox-1", "demo");
fences.remove("sandbox-1");
assert!(fences.previous_exit("sandbox-1").is_none());
assert!(!fences.stop_requested("sandbox-1", ""));
assert!(!fences.stop_requested("", "demo"));
assert!(!fences.stop_requested("sandbox-1"));
}
fn exited_sandbox_with_ready_reason(reason: &str) -> DriverSandbox {
crates/openshell-driver-mxc/src/driver.rs
+12 -27
View File
@@ -544,12 +544,9 @@ impl MxcComputeBackend {
self.map_sandbox_policy(&sandbox.id, policy, egress_addr)?;
Ok(())
}
pub async fn get_sandbox(&self, sandbox_name: &str) -> Option<DriverSandbox> {
pub async fn get_sandbox(&self, sandbox_id: &str) -> Option<DriverSandbox> {
let registry = self.registry.lock().await;
registry
.values()
.find(|e| e.sandbox.name == sandbox_name)
.map(|e| e.sandbox.clone())
registry.get(sandbox_id).map(|e| e.sandbox.clone())
}
pub async fn list_sandboxes(&self) -> Vec<DriverSandbox> {
@@ -660,23 +657,20 @@ impl MxcComputeBackend {
Ok(())
}
pub async fn stop_sandbox(&self, sandbox_name: &str) -> Result<(), tonic::Status> {
let (sandbox_id, lifecycle_gate) = {
pub async fn stop_sandbox(&self, sandbox_id: &str) -> Result<(), tonic::Status> {
let lifecycle_gate = {
let registry = self.registry.lock().await;
let entry = registry
.values()
.find(|entry| entry.sandbox.name == sandbox_name)
.ok_or_else(|| {
tonic::Status::not_found(format!("sandbox {sandbox_name} not found"))
})?;
(entry.sandbox.id.clone(), entry.lifecycle_gate.clone())
let entry = registry.get(sandbox_id).ok_or_else(|| {
tonic::Status::not_found(format!("sandbox {sandbox_id} not found"))
})?;
entry.lifecycle_gate.clone()
};
let _lifecycle_guard = lifecycle_gate.lock().await;
let (iso_id, mut isolation_stopped, cancel, monitor_task) = {
let mut registry = self.registry.lock().await;
let entry = registry.get_mut(&sandbox_id).ok_or_else(|| {
tonic::Status::not_found(format!("sandbox {sandbox_name} not found"))
let entry = registry.get_mut(sandbox_id).ok_or_else(|| {
tonic::Status::not_found(format!("sandbox {sandbox_id} not found"))
})?;
(
entry.iso_sandbox_id.clone(),
@@ -703,7 +697,7 @@ impl MxcComputeBackend {
}
let mut registry = self.registry.lock().await;
if let Some(entry) = registry.get_mut(&sandbox_id) {
if let Some(entry) = registry.get_mut(sandbox_id) {
entry.isolation_stopped = isolation_stopped;
entry.host_proxy = None;
entry.phase_state = PhaseState::Stopped;
@@ -724,21 +718,12 @@ impl MxcComputeBackend {
}
Ok(())
}
pub async fn delete_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<bool, tonic::Status> {
pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result<bool, tonic::Status> {
let lifecycle_gate = {
let registry = self.registry.lock().await;
let Some(entry) = registry.get(sandbox_id) else {
return Ok(false);
};
if entry.sandbox.name != sandbox_name {
return Err(tonic::Status::failed_precondition(
"sandbox_id did not match sandbox_name",
));
}
entry.lifecycle_gate.clone()
};
crates/openshell-driver-mxc/src/grpc.rs
+8 -19
View File
@@ -75,19 +75,14 @@ impl ComputeDriver for ComputeDriverService {
request: Request<GetSandboxRequest>,
) -> Result<Response<GetSandboxResponse>, Status> {
let req = request.into_inner();
if req.name.is_empty() {
return Err(Status::invalid_argument("name is required"));
if req.sandbox_id.is_empty() {
return Err(Status::invalid_argument("sandbox_id is required"));
}
let sandbox = self
.backend
.get_sandbox(&req.name)
.get_sandbox(&req.sandbox_id)
.await
.ok_or_else(|| Status::not_found(format!("sandbox {} not found", req.name)))?;
if !req.sandbox_id.is_empty() && req.sandbox_id != sandbox.id {
return Err(Status::failed_precondition(
"sandbox_id did not match the fetched sandbox",
));
}
.ok_or_else(|| Status::not_found(format!("sandbox {} not found", req.sandbox_id)))?;
Ok(Response::new(GetSandboxResponse {
sandbox: Some(sandbox),
}))
@@ -118,10 +113,10 @@ impl ComputeDriver for ComputeDriverService {
request: Request<StopSandboxRequest>,
) -> Result<Response<StopSandboxResponse>, Status> {
let req = request.into_inner();
if req.name.is_empty() {
return Err(Status::invalid_argument("name is required"));
if req.sandbox_id.is_empty() {
return Err(Status::invalid_argument("sandbox_id is required"));
}
self.backend.stop_sandbox(&req.name).await?;
self.backend.stop_sandbox(&req.sandbox_id).await?;
Ok(Response::new(StopSandboxResponse {}))
}
@@ -142,13 +137,7 @@ impl ComputeDriver for ComputeDriverService {
if req.sandbox_id.is_empty() {
return Err(Status::invalid_argument("sandbox_id is required"));
}
if req.name.is_empty() {
return Err(Status::invalid_argument("name is required"));
}
let deleted = self
.backend
.delete_sandbox(&req.sandbox_id, &req.name)
.await?;
let deleted = self.backend.delete_sandbox(&req.sandbox_id).await?;
Ok(Response::new(DeleteSandboxResponse { deleted }))
}
crates/openshell-driver-podman/src/grpc.rs
-2
View File
@@ -662,7 +662,6 @@ mod tests {
&service,
Request::new(DeleteSandboxRequest {
sandbox_id: String::new(),
name: "demo".to_string(),
}),
)
.await
@@ -693,7 +692,6 @@ mod tests {
&service,
Request::new(DeleteSandboxRequest {
sandbox_id: sandbox_id.to_string(),
name: "demo".to_string(),
}),
)
.await
crates/openshell-driver-vm/src/driver.rs
+45 -147
View File
@@ -585,35 +585,6 @@ struct SandboxRecord {
deleting: bool,
}
/// Resolve a lifecycle request to a registry key.
///
/// A non-empty `sandbox_id` is authoritative: resolution uses that id alone and
/// never falls back to the name, so a request for an already-removed sandbox
/// reports absence instead of matching a same-named sandbox in another
/// workspace. Only a caller that supplies no id resolves by name, and because
/// sandbox names are unique per workspace rather than globally, a name matching
/// more than one record is rejected instead of decided by iteration order.
fn resolve_record_id(
registry: &HashMap<String, SandboxRecord>,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<String>, Status> {
if !sandbox_id.is_empty() {
return Ok(registry.get_key_value(sandbox_id).map(|(id, _)| id.clone()));
}
let mut matches = registry
.iter()
.filter(|(_, record)| record.snapshot.name == sandbox_name);
let first = matches.next().map(|(id, _)| id.clone());
if matches.next().is_some() {
return Err(Status::failed_precondition(format!(
"sandbox_name {sandbox_name} matched more than one sandbox; supply sandbox_id"
)));
}
Ok(first)
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum OverlayPreparation {
Fresh,
@@ -1751,13 +1722,11 @@ impl VmDriver {
Ok(())
}
pub async fn stop_sandbox(&self, sandbox_id: &str, sandbox_name: &str) -> Result<(), Status> {
if !sandbox_id.is_empty() {
validate_sandbox_id(sandbox_id)?;
}
pub async fn stop_sandbox(&self, sandbox_id: &str) -> Result<(), Status> {
validate_sandbox_id(sandbox_id)?;
let record_id = {
let registry = self.registry.lock().await;
resolve_record_id(&registry, sandbox_id, sandbox_name)?
registry.get_key_value(sandbox_id).map(|(id, _)| id.clone())
}
.ok_or_else(|| Status::not_found("sandbox not found"))?;
@@ -1829,26 +1798,21 @@ impl VmDriver {
pub async fn start_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
generation_id: &str,
launch_authentication: Vec<u8>,
) -> Result<(), Status> {
if !sandbox_id.is_empty() {
validate_sandbox_id(sandbox_id)?;
}
validate_sandbox_id(sandbox_id)?;
let generation = openshell_core::sandbox_generation::SandboxGenerationId::parse(
generation_id.to_string(),
)
.map_err(|error| Status::invalid_argument(error.to_string()))?;
let (record_id, state_dir, already_running) = {
let registry = self.registry.lock().await;
let id = resolve_record_id(&registry, sandbox_id, sandbox_name)?
.ok_or_else(|| Status::not_found("sandbox not found"))?;
let record = registry
.get(&id)
let (id, record) = registry
.get_key_value(sandbox_id)
.ok_or_else(|| Status::not_found("sandbox not found"))?;
(
id,
id.clone(),
record.state_dir.clone(),
record.process.is_some() || record.provisioning_task.is_some(),
)
@@ -1883,7 +1847,7 @@ impl VmDriver {
// during startup recovery represents a new gateway session, so
// restart the VM before installing it rather than leaving the old
// supervisor connected with invalid credentials.
self.stop_sandbox(&record_id, sandbox_name).await?;
self.stop_sandbox(&record_id).await?;
}
remove_runtime_generation_material(&state_dir)
@@ -1949,22 +1913,15 @@ impl VmDriver {
otel.name = "vm.teardown",
otel.status_code = tracing::field::Empty,
sandbox.id = %sandbox_id,
sandbox.name = %sandbox_name,
)
)]
pub async fn delete_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<DeleteSandboxResponse, Status> {
pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result<DeleteSandboxResponse, Status> {
let span_status = openshell_otel::ErrorStatusGuard::current();
if !sandbox_id.is_empty() {
validate_sandbox_id(sandbox_id)?;
}
validate_sandbox_id(sandbox_id)?;
let record_id = {
let registry = self.registry.lock().await;
resolve_record_id(&registry, sandbox_id, sandbox_name)?
registry.get_key_value(sandbox_id).map(|(id, _)| id.clone())
};
let Some(record_id) = record_id else {
@@ -2024,26 +1981,13 @@ impl VmDriver {
span_status.finish(Ok(DeleteSandboxResponse { deleted: true }))
}
pub async fn get_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
) -> Result<Option<Sandbox>, Status> {
if !sandbox_id.is_empty() {
validate_sandbox_id(sandbox_id)?;
}
pub async fn get_sandbox(&self, sandbox_id: &str) -> Result<Option<Sandbox>, Status> {
validate_sandbox_id(sandbox_id)?;
let registry = self.registry.lock().await;
let sandbox = if sandbox_id.is_empty() {
registry
.values()
.find(|record| record.snapshot.name == sandbox_name)
.map(|record| record.snapshot.clone())
} else {
registry
.get(sandbox_id)
.map(|record| record.snapshot.clone())
};
let sandbox = registry
.get(sandbox_id)
.map(|record| record.snapshot.clone());
Ok(sandbox)
}
@@ -4355,23 +4299,11 @@ impl ComputeDriver for VmDriver {
request: Request<GetSandboxRequest>,
) -> Result<Response<GetSandboxResponse>, Status> {
let request = request.into_inner();
if request.sandbox_id.is_empty() && request.name.is_empty() {
return Err(Status::invalid_argument(
"sandbox_id or sandbox_name is required",
));
}
let sandbox = self
.get_sandbox(&request.sandbox_id, &request.name)
.get_sandbox(&request.sandbox_id)
.await?
.ok_or_else(|| Status::not_found("sandbox not found"))?;
if !request.sandbox_id.is_empty() && request.sandbox_id != sandbox.id {
return Err(Status::failed_precondition(
"sandbox_id did not match the fetched sandbox",
));
}
Ok(Response::new(GetSandboxResponse {
sandbox: Some(sandbox),
}))
@@ -4391,8 +4323,7 @@ impl ComputeDriver for VmDriver {
request: Request<StopSandboxRequest>,
) -> Result<Response<StopSandboxResponse>, Status> {
let request = request.into_inner();
self.stop_sandbox(&request.sandbox_id, &request.name)
.await?;
self.stop_sandbox(&request.sandbox_id).await?;
Ok(Response::new(StopSandboxResponse {}))
}
@@ -4403,7 +4334,6 @@ impl ComputeDriver for VmDriver {
let request = request.into_inner();
self.start_sandbox(
&request.sandbox_id,
&request.name,
&request.generation_id,
request.launch_authentication,
)
@@ -4416,9 +4346,7 @@ impl ComputeDriver for VmDriver {
request: Request<DeleteSandboxRequest>,
) -> Result<Response<DeleteSandboxResponse>, Status> {
let request = request.into_inner();
let response = self
.delete_sandbox(&request.sandbox_id, &request.name)
.await?;
let response = self.delete_sandbox(&request.sandbox_id).await?;
Ok(Response::new(response))
}
@@ -7282,7 +7210,6 @@ mod tests {
client
.get_sandbox(request_with_traceparent(GetSandboxRequest {
sandbox_id: String::new(),
name: String::new(),
}))
.await
.is_err()
@@ -7295,18 +7222,18 @@ mod tests {
client
.stop_sandbox(request_with_traceparent(StopSandboxRequest {
sandbox_id: String::new(),
name: String::new(),
}))
.await
.is_err()
);
client
.delete_sandbox(request_with_traceparent(DeleteSandboxRequest {
sandbox_id: String::new(),
name: String::new(),
}))
.await
.unwrap();
assert!(
client
.delete_sandbox(request_with_traceparent(DeleteSandboxRequest {
sandbox_id: String::new(),
}))
.await
.is_err()
);
let watch = client
.watch_sandboxes(request_with_traceparent(WatchSandboxesRequest {}))
.await
@@ -7666,7 +7593,7 @@ mod tests {
let _dispatch = tracing::dispatcher::set_default(&traced.dispatch);
let driver = test_driver_with_extensions(LifecycleExtensionRegistry::new());
assert!(driver.delete_sandbox("../invalid", "").await.is_err());
assert!(driver.delete_sandbox("../invalid").await.is_err());
let spans = traced.exporter.get_finished_spans().unwrap();
let deletion = spans
@@ -8640,12 +8567,7 @@ mod tests {
let (fresh_authentication, _) = test_launch_authentication("fresh");
let err = driver
.start_sandbox(
&sandbox.id,
&sandbox.name,
"g0000000000000001",
fresh_authentication,
)
.start_sandbox(&sandbox.id, "g0000000000000001", fresh_authentication)
.await
.expect_err("start without an image should fail");
@@ -8657,7 +8579,7 @@ mod tests {
"failed start must retain its durable stop marker"
);
let restored = driver
.get_sandbox(&sandbox.id, &sandbox.name)
.get_sandbox(&sandbox.id)
.await
.unwrap()
.expect("failed start must retain its stopped registry record");
@@ -8715,7 +8637,7 @@ mod tests {
);
driver
.start_sandbox(&sandbox.id, &sandbox.name, "g0000000000000001", Vec::new())
.start_sandbox(&sandbox.id, "g0000000000000001", Vec::new())
.await
.expect("matching already-running start must remain an idempotent no-op");
@@ -9404,7 +9326,7 @@ mod tests {
.await;
let err = driver
.delete_sandbox("sandbox-123", "sandbox-123")
.delete_sandbox("sandbox-123")
.await
.expect_err("state dir cleanup should fail for a file path");
assert!(err.message().contains("not a directory"));
@@ -9426,7 +9348,7 @@ mod tests {
}
let response = driver
.delete_sandbox("sandbox-123", "sandbox-123")
.delete_sandbox("sandbox-123")
.await
.expect("delete retry should succeed once cleanup works");
assert!(response.deleted);
@@ -9475,7 +9397,7 @@ mod tests {
}
let response = driver
.delete_sandbox("sandbox-123", "sandbox-123")
.delete_sandbox("sandbox-123")
.await
.expect("delete should handle accepted-but-not-started sandboxes");
assert!(response.deleted);
@@ -10851,7 +10773,7 @@ mod tests {
let beta = insert_named_record(&driver, "vm-beta", "demo", "beta").await;
driver
.stop_sandbox("vm-beta", "demo")
.stop_sandbox("vm-beta")
.await
.expect("stop by id should be accepted");
@@ -10873,7 +10795,7 @@ mod tests {
insert_named_record(&driver, "vm-beta", "demo", "beta").await;
let response = driver
.delete_sandbox("vm-beta", "demo")
.delete_sandbox("vm-beta")
.await
.expect("delete by id should be accepted");
@@ -10895,7 +10817,7 @@ mod tests {
insert_named_record(&driver, "vm-beta", "demo", "beta").await;
let first = driver
.delete_sandbox("vm-beta", "demo")
.delete_sandbox("vm-beta")
.await
.expect("the first delete should be accepted");
assert!(first.deleted);
@@ -10904,7 +10826,7 @@ mod tests {
// ordinary caller behavior. The id is gone from the registry by now, and
// resolving it by name instead would destroy the sandbox in `alpha`.
let second = driver
.delete_sandbox("vm-beta", "demo")
.delete_sandbox("vm-beta")
.await
.expect("a repeated delete should be accepted");
@@ -10926,19 +10848,14 @@ mod tests {
let alpha = insert_named_record(&driver, "vm-alpha", "demo", "alpha").await;
let stop_error = driver
.stop_sandbox("vm-absent", "demo")
.stop_sandbox("vm-absent")
.await
.expect_err("a supplied id that is not registered must not resolve by name");
assert_eq!(stop_error.code(), Code::NotFound);
let (start_authentication, _) = test_launch_authentication("absent");
let start_error = driver
.start_sandbox(
"vm-absent",
"demo",
"g0000000000000001",
start_authentication,
)
.start_sandbox("vm-absent", "g0000000000000001", start_authentication)
.await
.expect_err("a supplied id that is not registered must not resolve by name");
assert_eq!(start_error.code(), Code::NotFound);
@@ -10950,30 +10867,26 @@ mod tests {
}
#[tokio::test]
async fn a_name_only_request_matching_two_sandboxes_is_rejected() {
async fn lifecycle_requests_reject_an_empty_id() {
let temp = tempfile::tempdir().unwrap();
let driver = resolution_test_driver(temp.path());
let alpha = insert_named_record(&driver, "vm-alpha", "demo", "alpha").await;
let beta = insert_named_record(&driver, "vm-beta", "demo", "beta").await;
// Without an id the driver has nothing to disambiguate with: the request
// carries no workspace, and picking by iteration order would stop or
// delete an arbitrary one of the two.
for error in [
driver.stop_sandbox("", "demo").await.unwrap_err(),
driver.stop_sandbox("").await.unwrap_err(),
driver
.start_sandbox(
"",
"demo",
"g0000000000000001",
test_launch_authentication("ambiguous").0,
)
.await
.unwrap_err(),
driver.delete_sandbox("", "demo").await.unwrap_err(),
driver.delete_sandbox("").await.unwrap_err(),
] {
assert_eq!(error.code(), Code::FailedPrecondition);
assert!(error.message().contains("matched more than one sandbox"));
assert_eq!(error.code(), Code::InvalidArgument);
assert_eq!(error.message(), "sandbox id is required");
}
let registry = driver.registry.lock().await;
@@ -10982,19 +10895,4 @@ mod tests {
assert!(!alpha.join(SANDBOX_STOPPED_FILE).exists());
assert!(!beta.join(SANDBOX_STOPPED_FILE).exists());
}
#[tokio::test]
async fn a_name_only_request_still_resolves_a_unique_name() {
let temp = tempfile::tempdir().unwrap();
let driver = resolution_test_driver(temp.path());
let alpha = insert_named_record(&driver, "vm-alpha", "demo", "alpha").await;
insert_named_record(&driver, "vm-beta", "other", "beta").await;
driver
.stop_sandbox("", "demo")
.await
.expect("an unambiguous name-only stop should still resolve");
assert!(alpha.join(SANDBOX_STOPPED_FILE).exists());
}
}
crates/openshell-server/src/compute/mod.rs
+33 -107
View File
@@ -1289,7 +1289,7 @@ impl ComputeRuntime {
async fn delete_backend_after_failed_create(
&self,
sandbox_id: &str,
sandbox_name: &str,
_sandbox_name: &str,
) -> Result<bool, Status> {
self.driver
.call(
@@ -1297,13 +1297,9 @@ impl ComputeRuntime {
Some(sandbox_id),
|driver| {
let sandbox_id = sandbox_id.to_string();
let sandbox_name = sandbox_name.to_string();
async move {
driver
.delete_sandbox(Request::new(DeleteSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.delete_sandbox(Request::new(DeleteSandboxRequest { sandbox_id }))
.await
}
},
@@ -1404,7 +1400,7 @@ impl ComputeRuntime {
async fn complete_sandbox_stop(
&self,
sandbox_id: String,
sandbox_name: String,
_sandbox_name: String,
previous: Sandbox,
stopping: Sandbox,
lifecycle_guard: SandboxLifecycleGuard,
@@ -1416,13 +1412,9 @@ impl ComputeRuntime {
Some(&sandbox_id),
|driver| {
let sandbox_id = sandbox_id.clone();
let sandbox_name = sandbox_name.clone();
async move {
driver
.stop_sandbox(Request::new(StopSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.stop_sandbox(Request::new(StopSandboxRequest { sandbox_id }))
.await
}
},
@@ -1697,7 +1689,7 @@ impl ComputeRuntime {
async fn complete_sandbox_start(
&self,
sandbox_id: String,
sandbox_name: String,
_sandbox_name: String,
previous: Sandbox,
starting: Sandbox,
lifecycle_guard: SandboxLifecycleGuard,
@@ -1722,12 +1714,10 @@ impl ComputeRuntime {
Some(&sandbox_id),
|driver| {
let sandbox_id = sandbox_id.clone();
let sandbox_name = sandbox_name.clone();
async move {
driver
.start_sandbox(Request::new(StartSandboxRequest {
sandbox_id,
name: sandbox_name,
launch_authentication,
generation_id,
expected_runtime_identity,
@@ -1918,7 +1908,6 @@ impl ComputeRuntime {
original: Status,
) -> Status {
let sandbox_id = starting.object_id();
let sandbox_name = starting.object_name();
let stop_result = self
.driver
.call(
@@ -1926,13 +1915,9 @@ impl ComputeRuntime {
Some(sandbox_id),
|driver| {
let sandbox_id = sandbox_id.to_string();
let sandbox_name = sandbox_name.to_string();
async move {
driver
.stop_sandbox(Request::new(StopSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.stop_sandbox(Request::new(StopSandboxRequest { sandbox_id }))
.await
}
},
@@ -2256,13 +2241,9 @@ impl ComputeRuntime {
Some(transition.deleting.object_id()),
|driver| {
let sandbox_id = transition.deleting.object_id().to_string();
let sandbox_name = transition.deleting.object_name().to_string();
async move {
driver
.delete_sandbox(Request::new(DeleteSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.delete_sandbox(Request::new(DeleteSandboxRequest { sandbox_id }))
.await
}
},
@@ -2851,13 +2832,9 @@ impl ComputeRuntime {
Some(&sandbox_id),
|driver| {
let sandbox_id = sandbox_id.clone();
let sandbox_name = sandbox_name.clone();
async move {
driver
.stop_sandbox(Request::new(StopSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.stop_sandbox(Request::new(StopSandboxRequest { sandbox_id }))
.await
}
},
@@ -2997,7 +2974,6 @@ impl ComputeRuntime {
continue;
}
let sandbox_name = sandbox.object_name().to_string();
let generation_id = match sandbox_runtime_generation(&sandbox) {
Ok(generation) => generation.into_string(),
Err(error) => {
@@ -3039,14 +3015,12 @@ impl ComputeRuntime {
Some(&sandbox_id),
|driver| {
let sandbox_id = sandbox_id.clone();
let sandbox_name = sandbox_name.clone();
let launch_authentication = launch_authentication.clone();
let expected_runtime_identity = expected_runtime_identity.clone();
async move {
driver
.start_sandbox(Request::new(StartSandboxRequest {
sandbox_id,
name: sandbox_name,
launch_authentication,
generation_id,
expected_runtime_identity,
@@ -3215,7 +3189,6 @@ impl ComputeRuntime {
}
SandboxPhase::Stopping => {
let sandbox_id = sandbox.object_id().to_string();
let sandbox_name = sandbox.object_name().to_string();
let driver_sandbox_id = sandbox_id.clone();
match self
.driver
@@ -3226,7 +3199,6 @@ impl ComputeRuntime {
driver
.stop_sandbox(Request::new(StopSandboxRequest {
sandbox_id: driver_sandbox_id,
name: sandbox_name,
}))
.await
},
@@ -3276,7 +3248,6 @@ impl ComputeRuntime {
continue;
}
let sandbox_id = sandbox.object_id().to_string();
let sandbox_name = sandbox.object_name().to_string();
let driver_sandbox_id = sandbox_id.clone();
let generation_id = match sandbox_runtime_generation(&sandbox) {
Ok(generation) => generation.into_string(),
@@ -3295,7 +3266,6 @@ impl ComputeRuntime {
driver
.start_sandbox(Request::new(StartSandboxRequest {
sandbox_id: driver_sandbox_id,
name: sandbox_name,
launch_authentication: Vec::new(),
generation_id,
expected_runtime_identity,
@@ -4365,13 +4335,9 @@ impl ComputeRuntime {
Some(sandbox_id),
|driver| {
let sandbox_id = sandbox_id.to_string();
let sandbox_name = sandbox_name.to_string();
async move {
driver
.delete_sandbox(Request::new(DeleteSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.delete_sandbox(Request::new(DeleteSandboxRequest { sandbox_id }))
.await
}
},
@@ -4697,7 +4663,7 @@ impl ComputeRuntime {
async fn get_driver_sandbox(
&self,
sandbox_id: &str,
sandbox_name: &str,
_sandbox_name: &str,
) -> Result<Option<DriverSandbox>, String> {
match self
.driver
@@ -4706,13 +4672,9 @@ impl ComputeRuntime {
Some(sandbox_id),
|driver| {
let sandbox_id = sandbox_id.to_string();
let sandbox_name = sandbox_name.to_string();
async move {
driver
.get_sandbox(Request::new(GetSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.get_sandbox(Request::new(GetSandboxRequest { sandbox_id }))
.await
}
},
@@ -6641,19 +6603,10 @@ mod tests {
};
let sandbox = current
.iter()
.find(|sandbox| {
sandbox.name == request.name
&& (request.sandbox_id.is_empty() || sandbox.id == request.sandbox_id)
})
.find(|sandbox| sandbox.id == request.sandbox_id)
.cloned()
.ok_or_else(|| Status::not_found("sandbox not found"))?;
if !request.sandbox_id.is_empty() && request.sandbox_id != sandbox.id {
return Err(Status::failed_precondition(
"sandbox_id did not match the fetched sandbox",
));
}
Ok(tonic::Response::new(GetSandboxResponse {
sandbox: Some(sandbox),
}))
@@ -6771,7 +6724,7 @@ mod tests {
delete_release: Semaphore,
delete_blocked: AtomicBool,
delete_calls: AtomicUsize,
delete_requests: TestMutex<Vec<(String, String)>>,
delete_requests: TestMutex<Vec<String>>,
delete_outcome: TestMutex<ControlledDeleteOutcome>,
create_started: Notify,
create_release: Semaphore,
@@ -6781,14 +6734,14 @@ mod tests {
stop_release: Semaphore,
stop_blocked: AtomicBool,
stop_calls: AtomicUsize,
stop_requests: TestMutex<Vec<(String, String)>>,
stop_requests: TestMutex<Vec<String>>,
stop_outcome: TestMutex<ControlledLifecycleOutcome>,
start_started: Notify,
start_finished: Notify,
start_release: Semaphore,
start_blocked: AtomicBool,
start_calls: AtomicUsize,
start_requests: TestMutex<Vec<(String, String)>>,
start_requests: TestMutex<Vec<String>>,
start_authentications: TestMutex<Vec<Vec<u8>>>,
start_expected_runtime_identities: TestMutex<Vec<String>>,
start_outcome: TestMutex<ControlledLifecycleOutcome>,
@@ -6915,7 +6868,7 @@ mod tests {
self.delete_calls.load(Ordering::SeqCst)
}
fn delete_requests(&self) -> Vec<(String, String)> {
fn delete_requests(&self) -> Vec<String> {
self.delete_requests
.lock()
.expect("delete requests lock poisoned")
@@ -6926,7 +6879,7 @@ mod tests {
self.stop_calls.load(Ordering::SeqCst)
}
fn stop_requests(&self) -> Vec<(String, String)> {
fn stop_requests(&self) -> Vec<String> {
self.stop_requests
.lock()
.expect("stop requests lock poisoned")
@@ -6937,7 +6890,7 @@ mod tests {
self.start_calls.load(Ordering::SeqCst)
}
fn start_requests(&self) -> Vec<(String, String)> {
fn start_requests(&self) -> Vec<String> {
self.start_requests
.lock()
.expect("start requests lock poisoned")
@@ -7085,7 +7038,7 @@ mod tests {
self.stop_requests
.lock()
.expect("stop requests lock poisoned")
.push((request.sandbox_id, request.name));
.push(request.sandbox_id);
self.stop_calls.fetch_add(1, Ordering::SeqCst);
self.stop_started.notify_one();
if self.stop_blocked.load(Ordering::SeqCst) {
@@ -7116,7 +7069,7 @@ mod tests {
self.start_requests
.lock()
.expect("start requests lock poisoned")
.push((request.sandbox_id, request.name));
.push(request.sandbox_id);
self.start_authentications
.lock()
.expect("start authentications lock poisoned")
@@ -7161,7 +7114,7 @@ mod tests {
self.delete_requests
.lock()
.expect("delete requests lock poisoned")
.push((request.sandbox_id, request.name));
.push(request.sandbox_id);
self.delete_calls.fetch_add(1, Ordering::SeqCst);
self.delete_started.notify_one();
if self.delete_blocked.load(Ordering::SeqCst) {
@@ -7338,10 +7291,7 @@ mod tests {
assert_eq!(driver.delete_calls(), 1);
assert_eq!(
driver.delete_requests(),
vec![(
sandbox.object_id().to_string(),
sandbox.object_name().to_string()
)]
vec![sandbox.object_id().to_string()]
);
assert!(
runtime
@@ -7463,10 +7413,7 @@ mod tests {
assert_eq!(driver.delete_calls(), 1);
assert_eq!(
driver.delete_requests(),
vec![(
sandbox.object_id().to_string(),
sandbox.object_name().to_string()
)]
vec![sandbox.object_id().to_string()]
);
let retained = runtime
.store
@@ -10928,10 +10875,7 @@ mod tests {
tokio::time::timeout(Duration::from_secs(1), driver.delete_started.notified())
.await
.expect("background driver cleanup did not run");
assert_eq!(
driver.delete_requests(),
vec![("sb-1".to_string(), "sandbox-a".to_string())]
);
assert_eq!(driver.delete_requests(), vec!["sb-1".to_string()]);
assert_sandbox_owned_records(&runtime, &sandbox, &session, false).await;
assert!(
runtime
@@ -11007,10 +10951,7 @@ mod tests {
tokio::time::timeout(Duration::from_secs(1), driver.delete_started.notified())
.await
.expect("background driver cleanup did not run");
assert_eq!(
driver.delete_requests(),
vec![("sb-1".to_string(), "sandbox-a".to_string())]
);
assert_eq!(driver.delete_requests(), vec!["sb-1".to_string()]);
}
#[tokio::test]
@@ -12640,11 +12581,7 @@ mod tests {
.await
.unwrap();
let mut called_ids = driver
.stop_requests()
.into_iter()
.map(|(id, _)| id)
.collect::<Vec<_>>();
let mut called_ids = driver.stop_requests().into_iter().collect::<Vec<_>>();
called_ids.sort();
assert_eq!(
called_ids,
@@ -12872,11 +12809,7 @@ mod tests {
runtime.start_persisted_sandboxes().await.unwrap();
let mut called_ids = driver
.start_requests()
.into_iter()
.map(|(id, _)| id)
.collect::<Vec<_>>();
let mut called_ids = driver.start_requests().into_iter().collect::<Vec<_>>();
called_ids.sort();
assert_eq!(
called_ids,
@@ -13093,7 +13026,7 @@ mod tests {
assert_eq!(
driver.start_requests(),
vec![("sb-1".to_string(), "local".to_string())],
vec!["sb-1".to_string()],
"{driver_name} should reconcile persisted running intent"
);
}
@@ -13376,7 +13309,6 @@ mod tests {
remote
.get_sandbox(Request::new(GetSandboxRequest {
sandbox_id: sandbox.id.clone(),
name: String::new(),
}))
.await
.unwrap();
@@ -13387,7 +13319,6 @@ mod tests {
remote
.stop_sandbox(Request::new(StopSandboxRequest {
sandbox_id: sandbox.id.clone(),
name: String::new(),
}))
.await
.unwrap();
@@ -13398,7 +13329,6 @@ mod tests {
remote
.delete_sandbox(Request::new(DeleteSandboxRequest {
sandbox_id: sandbox.id,
name: String::new(),
}))
.await
.unwrap();
@@ -13564,16 +13494,16 @@ mod tests {
.unwrap();
assert!(matches!(
driver.calls().as_slice(),
[FakeComputeDriverCall::StopSandbox { sandbox_id, sandbox_name }]
if sandbox_id == "sb-uds" && sandbox_name == "uds-sandbox"
[FakeComputeDriverCall::StopSandbox { sandbox_id }]
if sandbox_id == "sb-uds"
));
driver.clear_calls();
runtime.start_persisted_sandboxes().await.unwrap();
assert!(matches!(
driver.calls().as_slice(),
[FakeComputeDriverCall::GetCapabilities, FakeComputeDriverCall::StartSandbox { sandbox_id, sandbox_name }]
if sandbox_id == "sb-uds" && sandbox_name == "uds-sandbox"
[FakeComputeDriverCall::GetCapabilities, FakeComputeDriverCall::StartSandbox { sandbox_id }]
if sandbox_id == "sb-uds"
));
driver.clear_calls();
assert!(
@@ -13587,12 +13517,8 @@ mod tests {
let calls = driver.calls();
assert_eq!(calls.len(), 1, "unexpected calls: {calls:?}");
match &calls[0] {
FakeComputeDriverCall::DeleteSandbox {
sandbox_id,
sandbox_name,
} => {
FakeComputeDriverCall::DeleteSandbox { sandbox_id } => {
assert_eq!(sandbox_id, "sb-uds");
assert_eq!(sandbox_name, "uds-sandbox");
}
other => panic!("expected DeleteSandbox call, got {other:?}"),
}
crates/openshell-server/src/compute/provisioning_deadline.rs
+2 -6
View File
@@ -501,9 +501,9 @@ impl super::ComputeRuntime {
expired: &openshell_core::proto::Sandbox,
lifecycle_guard: &super::SandboxLifecycleGuard,
) -> Result<(), String> {
use openshell_core::ObjectId;
use openshell_core::proto::Sandbox;
use openshell_core::proto::compute::v1::StopSandboxRequest;
use openshell_core::{ObjectId, ObjectName};
let current = {
let _global_guard = self.lock_global_for_lifecycle(lifecycle_guard).await;
self.store
@@ -573,7 +573,6 @@ impl super::ComputeRuntime {
.map_err(|error| error.to_string())?;
}
let sandbox_id = expired.object_id().to_string();
let sandbox_name = expired.object_name().to_string();
let result = tokio::time::timeout(
std::time::Duration::from_secs(30),
self.driver.call(
@@ -583,10 +582,7 @@ impl super::ComputeRuntime {
let sandbox_id = sandbox_id.clone();
async move {
driver
.stop_sandbox(tonic::Request::new(StopSandboxRequest {
sandbox_id,
name: sandbox_name,
}))
.stop_sandbox(tonic::Request::new(StopSandboxRequest { sandbox_id }))
.await
}
},
crates/openshell-server/src/test_support.rs
+8 -43
View File
@@ -94,29 +94,13 @@ pub fn authenticate_as_dev_user(mut request: Request<()>) -> Result<Request<()>,
#[derive(Debug, Clone, PartialEq)]
pub enum FakeComputeDriverCall {
GetCapabilities,
ValidateSandboxCreate {
sandbox: Option<DriverSandbox>,
},
GetSandbox {
sandbox_id: String,
sandbox_name: String,
},
ValidateSandboxCreate { sandbox: Option<DriverSandbox> },
GetSandbox { sandbox_id: String },
ListSandboxes,
CreateSandbox {
sandbox: Option<DriverSandbox>,
},
StopSandbox {
sandbox_id: String,
sandbox_name: String,
},
StartSandbox {
sandbox_id: String,
sandbox_name: String,
},
DeleteSandbox {
sandbox_id: String,
sandbox_name: String,
},
CreateSandbox { sandbox: Option<DriverSandbox> },
StopSandbox { sandbox_id: String },
StartSandbox { sandbox_id: String },
DeleteSandbox { sandbox_id: String },
WatchSandboxes,
}
@@ -331,15 +315,11 @@ impl ComputeDriver for FakeComputeDriver {
let sandbox = self.with_state(|state| {
state.calls.push(FakeComputeDriverCall::GetSandbox {
sandbox_id: request.sandbox_id.clone(),
sandbox_name: request.name.clone(),
});
state
.sandboxes
.values()
.find(|sandbox| {
(!request.sandbox_id.is_empty() && sandbox.id == request.sandbox_id)
|| (!request.name.is_empty() && sandbox.name == request.name)
})
.find(|sandbox| sandbox.id == request.sandbox_id)
.cloned()
});
let sandbox = sandbox.ok_or_else(|| Status::not_found("sandbox not found"))?;
@@ -387,7 +367,6 @@ impl ComputeDriver for FakeComputeDriver {
self.with_state(|state| {
state.calls.push(FakeComputeDriverCall::StopSandbox {
sandbox_id: request.sandbox_id,
sandbox_name: request.name,
});
});
Ok(Response::new(StopSandboxResponse {}))
@@ -402,7 +381,6 @@ impl ComputeDriver for FakeComputeDriver {
self.with_state(|state| {
state.calls.push(FakeComputeDriverCall::StartSandbox {
sandbox_id: request.sandbox_id,
sandbox_name: request.name,
});
});
Ok(Response::new(StartSandboxResponse::default()))
@@ -417,21 +395,8 @@ impl ComputeDriver for FakeComputeDriver {
let deleted = self.with_state(|state| {
state.calls.push(FakeComputeDriverCall::DeleteSandbox {
sandbox_id: request.sandbox_id.clone(),
sandbox_name: request.name.clone(),
});
if request.sandbox_id.is_empty() {
let Some(id) = state
.sandboxes
.iter()
.find(|(_, sandbox)| sandbox.name == request.name)
.map(|(id, _)| id.clone())
else {
return false;
};
state.sandboxes.remove(&id).is_some()
} else {
state.sandboxes.remove(&request.sandbox_id).is_some()
}
state.sandboxes.remove(&request.sandbox_id).is_some()
});
Ok(Response::new(DeleteSandboxResponse { deleted }))
}
docs/reference/sandbox-compute-drivers.mdx
+8
View File
@@ -103,6 +103,14 @@ The gateway connects to the operator-provided endpoint; it does not provision
or supervise the remote driver. The operator must protect the socket so only
the gateway uid can access it.
The compute-driver lifecycle contract addresses sandboxes only by the stable
`sandbox_id` assigned by the gateway. `GetSandbox`, `StopSandbox`,
`StartSandbox`, and `DeleteSandbox` require that ID; drivers must not use a
sandbox name as a fallback lookup key. `ListSandboxes` is unscoped and returns
the complete inventory for that driver instance. Every returned sandbox must
have a non-empty, globally unique ID. Names and workspaces in returned
snapshots are descriptive metadata.
Sandbox create supports `--cpu` and `--memory` for per-sandbox compute sizing.
Docker and Podman apply them as runtime limits. Kubernetes applies them as both
container requests and limits. The VM driver accepts the fields but currently
proto/README.md
+4
View File
@@ -21,6 +21,10 @@ changing existing APIs; generated SDK naming follows from these definitions.
`driver_name`, `runtime_class_name`, `rule_name`, and `middleware_name`.
- Public callers reference entities by canonical name. Keep immutable IDs at
authentication, persistence, compute-driver, and other internal boundaries.
- Compute-driver sandbox lifecycle requests use the gateway-assigned
`sandbox_id` as their sole resource reference. `DriverSandbox.name` and
`DriverSandbox.workspace` are observation metadata and must not be used as
lookup keys by a driver.
For example:
proto/compute_driver.proto
+13 -13
View File
@@ -370,10 +370,9 @@ message ValidateSandboxCreateRequest {
message ValidateSandboxCreateResponse {}
message GetSandboxRequest {
// Stable sandbox ID stored by the gateway.
// Required stable sandbox ID assigned by the gateway. This immutable ID is
// the sole sandbox reference at the compute-driver boundary.
string sandbox_id = 1;
// Compute-runtime name used by the driver.
string name = 2;
}
message GetSandboxResponse {
@@ -381,10 +380,14 @@ message GetSandboxResponse {
DriverSandbox sandbox = 1;
}
// Requests the complete sandbox inventory managed by this configured driver
// instance. This collection is intentionally unscoped by workspace.
message ListSandboxesRequest {}
message ListSandboxesResponse {
// Platform-observed sandbox snapshots returned by the driver.
// Platform-observed sandbox snapshots returned by the driver. Every entry
// must carry a non-empty, globally unique DriverSandbox.id. Workspace and
// name are observation metadata, not collection keys.
repeated DriverSandbox sandboxes = 1;
}
@@ -400,19 +403,17 @@ message CreateSandboxResponse {
}
message StopSandboxRequest {
// Stable sandbox ID stored by the gateway.
// Required stable sandbox ID assigned by the gateway. This immutable ID is
// the sole sandbox reference at the compute-driver boundary.
string sandbox_id = 1;
// Compute-runtime name used by the driver.
string name = 2;
}
message StopSandboxResponse {}
message StartSandboxRequest {
// Stable sandbox ID stored by the gateway.
// Required stable sandbox ID assigned by the gateway. This immutable ID is
// the sole sandbox reference at the compute-driver boundary.
string sandbox_id = 1;
// Compute-runtime name used by the driver.
string name = 2;
// Fresh launch credentials for start-from-stopped. Empty only for drivers
// that do not implement the OpenShell Sandbox Protocol.
bytes launch_authentication = 3 [(openshell.options.v1.secret) = true];
@@ -433,10 +434,9 @@ message StartSandboxResponse {
}
message DeleteSandboxRequest {
// Stable sandbox ID stored by the gateway.
// Required stable sandbox ID assigned by the gateway. This immutable ID is
// the sole sandbox reference at the compute-driver boundary.
string sandbox_id = 1;
// Compute-runtime name used by the driver.
string name = 2;
}
message DeleteSandboxResponse {