diff --git a/sdks/c/Cargo.toml b/sdks/c/Cargo.toml index 458aa8884..01625b09f 100644 --- a/sdks/c/Cargo.toml +++ b/sdks/c/Cargo.toml @@ -15,12 +15,12 @@ crate-type = ["cdylib", "staticlib"] default = [] [dependencies] -boxlite = { workspace = true, features = ["rest", "gvproxy"] } +boxlite = { workspace = true, features = ["rest"] } futures = "0.3" tokio = { version = "1.37", features = ["io-util", "net", "rt", "rt-multi-thread"] } [dev-dependencies] -boxlite = { workspace = true, features = ["rest", "gvproxy"] } +boxlite = { workspace = true, features = ["rest"] } [build-dependencies] cbindgen = "0.29" diff --git a/src/boxlite/src/portal/interfaces/container.rs b/src/boxlite/src/portal/interfaces/container.rs index ce812baca..042bf4bdf 100644 --- a/src/boxlite/src/portal/interfaces/container.rs +++ b/src/boxlite/src/portal/interfaces/container.rs @@ -179,13 +179,31 @@ impl ContainerInterface { pub async fn start(&mut self, container_id: &str) -> BoxliteResult<()> { use boxlite_shared::{ContainerStartRequest, container_start_response}; - let response = self + let response = match self .client .start(ContainerStartRequest { container_id: container_id.to_string(), }) - .await? - .into_inner(); + .await + { + Ok(response) => response.into_inner(), + // A guest agent that predates #988 has no `Container.Start`; tonic + // answers the unknown method with `Unimplemented`. On that agent the + // fused `Container.Init` — still called on every boot — already + // started the container, so `Start` is a redundant no-op here. + // Treat it as started rather than surfacing a cryptic gRPC error; + // the code must be read before `?` flattens it into `Rpc(String)`. + // (Recreate the box to regain attach-before-start semantics.) + Err(status) if status.code() == tonic::Code::Unimplemented => { + tracing::warn!( + container_id = %container_id, + "guest agent predates Container.Start (pre-#988); its Init already \ + started the container — treating as started" + ); + return Ok(()); + } + Err(status) => return Err(status.into()), + }; match response.result { Some(container_start_response::Result::Success(_)) => { @@ -205,3 +223,130 @@ impl ContainerInterface { } } } + +#[cfg(test)] +mod tests { + use super::*; + use boxlite_shared::{ + Container as ContainerService, ContainerInitRequest, ContainerInitResponse, + ContainerInitSuccess, ContainerServer, ContainerStartRequest, ContainerStartResponse, + ContainerStartSuccess, container_init_response, container_start_response, + }; + use tonic::transport::{Endpoint, Server}; + use tonic::{Request, Response, Status}; + + /// How the stub guest answers `Container.Start`. + #[derive(Clone, Copy)] + enum StartReply { + /// Emulates a pre-#988 agent that lacks the method: tonic answers an + /// unknown method with `Unimplemented` (empty message). + Unimplemented, + /// A genuine server-side failure that must NOT be swallowed. + RealError, + /// A current agent that starts the container. + Success, + } + + struct StubGuest { + start_reply: StartReply, + } + + #[tonic::async_trait] + impl ContainerService for StubGuest { + async fn init( + &self, + request: Request, + ) -> Result, Status> { + let container_id = request.into_inner().container_id; + Ok(Response::new(ContainerInitResponse { + result: Some(container_init_response::Result::Success( + ContainerInitSuccess { container_id }, + )), + })) + } + + async fn start( + &self, + request: Request, + ) -> Result, Status> { + let container_id = request.into_inner().container_id; + match self.start_reply { + StartReply::Unimplemented => Err(Status::unimplemented("")), + StartReply::RealError => Err(Status::internal("guest blew up")), + StartReply::Success => Ok(Response::new(ContainerStartResponse { + result: Some(container_start_response::Result::Success( + ContainerStartSuccess { container_id }, + )), + })), + } + } + } + + /// Serve `StubGuest` on an ephemeral loopback port and return a + /// `ContainerInterface` wired to it. + async fn interface_for(start_reply: StartReply) -> ContainerInterface { + // Bind with std to learn a free port, then hand the address to tonic. + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + drop(listener); + + tokio::spawn(async move { + Server::builder() + .add_service(ContainerServer::new(StubGuest { start_reply })) + .serve(addr) + .await + .unwrap(); + }); + + let endpoint = Endpoint::from_shared(format!("http://{addr}")).unwrap(); + let mut attempts = 0; + let channel = loop { + match endpoint.connect().await { + Ok(channel) => break channel, + Err(e) => { + attempts += 1; + assert!( + attempts < 100, + "stub guest never accepted a connection: {e}" + ); + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + } + } + }; + ContainerInterface::new(channel) + } + + /// #988 regression: a guest agent that predates `Container.Start` answers + /// with gRPC `Unimplemented`. Its fused `Container.Init` already started the + /// container, so the host must treat the missing method as "already started" + /// rather than surfacing `code=16`. Without the degrade this returns + /// `BoxliteError::Rpc` and the restarted box fails to come up. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn container_start_tolerates_unimplemented_from_legacy_guest() { + let mut iface = interface_for(StartReply::Unimplemented).await; + let result = iface.start("box-legacy").await; + assert!( + result.is_ok(), + "legacy guest (Start Unimplemented) must be tolerated, got {result:?}" + ); + } + + /// The degrade must be narrow: a real server-side error on `Start` still + /// surfaces as an error, never swallowed as "started". + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn container_start_propagates_real_error() { + let mut iface = interface_for(StartReply::RealError).await; + let result = iface.start("box-broken").await; + assert!( + result.is_err(), + "a real Start error must not be swallowed: {result:?}" + ); + } + + /// Normal path against a current guest is unaffected. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn container_start_ok_on_success() { + let mut iface = interface_for(StartReply::Success).await; + assert!(iface.start("box-ok").await.is_ok()); + } +}