diff --git a/src/sandbox/firecracker/sandbox.rs b/src/sandbox/firecracker/sandbox.rs index 5adcaef0..f28718fe 100644 --- a/src/sandbox/firecracker/sandbox.rs +++ b/src/sandbox/firecracker/sandbox.rs @@ -454,8 +454,23 @@ impl FirecrackerSandbox { #[tracing::instrument(skip(self))] pub async fn start(&mut self) -> Result<()> { debug!("starting firecracker sandbox"); - self.start_nowait().await?; - self.wait_for_ready().await + let start_result = async { + self.start_nowait().await?; + self.wait_for_ready().await + } + .await; + + if let Err(err) = start_result { + if let Err(stop_err) = self.stop().await { + warn!( + error = %stop_err, + "failed to stop sandbox after start failure" + ); + } + return Err(err); + } + + Ok(()) } /// Start the sandbox WITHOUT waiting for envd's readiness. @@ -879,6 +894,19 @@ impl FirecrackerSandbox { ) } + fn configure_network_slot( + &mut self, + slot: Slot, + policy: Option<&SandboxNetworkPolicy>, + ) -> Result<()> { + self.network_slot = Some(slot); + self.network_slot + .as_ref() + .expect("network slot was just assigned") + .set_egress_policy(policy) + .context("Failed to configure sandbox egress policy") + } + fn mmds_metadata(&self, common: &FirecrackerCommonConfig) -> MmdsMetadata { common .mmds_metadata @@ -1201,9 +1229,7 @@ impl FirecrackerSandbox { // valid DNS server IP in the 8th field. See that method for format details. let ip_config = slot.build_ip_boot_arg(); let netns = slot.namespace_path(); - slot.set_egress_policy(config.common.network_policy.as_ref()) - .context("Failed to configure sandbox egress policy")?; - self.network_slot = Some(slot); + self.configure_network_slot(slot, config.common.network_policy.as_ref())?; boot_args = Some(match boot_args.take() { Some(existing) => format!("{existing} {ip_config}"), None => ip_config, @@ -1909,6 +1935,27 @@ mod tests { Ok(()) } + #[tokio::test] + async fn start_failure_rolls_back_initialized_runtime_state() -> Result<()> { + let mut sandbox = FirecrackerSandbox::new(fresh_config())?; + sandbox.envd_instance = Some(EnvdInstance::new(format!( + "http://127.0.0.1:{}", + ToolsConfig::default().control_plane_port + ))); + + let err = sandbox + .start() + .await + .expect_err("missing launch artifacts should fail validation"); + + assert!(err.to_string().contains("firecracker binary not found")); + assert!( + sandbox.envd_instance.is_none(), + "failed start must roll back initialized runtime state" + ); + Ok(()) + } + #[test] fn host_interaction_ip_reflects_network_slot_state() -> Result<()> { let mut sandbox = FirecrackerSandbox::new(fresh_config())?; @@ -1928,6 +1975,24 @@ mod tests { Ok(()) } + #[test] + fn egress_policy_failure_keeps_network_slot_owned_for_cleanup() -> Result<()> { + let mut sandbox = FirecrackerSandbox::new(fresh_config())?; + let manager = NetworkManager::new(false, 0, 0); + let slot = manager.allocate_test_slot()?; + + sandbox + .configure_network_slot(slot, None) + .expect_err("test slot has no network namespace"); + + let slot = sandbox + .network_slot + .take() + .expect("failed policy setup must leave the slot owned by the sandbox"); + manager.cleanup_allocated_slot(slot, true)?; + Ok(()) + } + #[test] fn work_rootfs_path_uses_overlaybd_symlink_path() -> Result<()> { let sandbox = FirecrackerSandbox::new(overlaybd_config())?;