Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 1 addition & 14 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion firepilot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ hyperlocal = "0.8"
serde_derive = "1.0.160"
url = "^2.2"
tokio = { version = "1.27.0", features = ["process", "rt", "macros"], default-features = false }
firepilot_models = "1.3.0"
firepilot_models = { path = "../firepilot_models" }
tracing = "0.1"

[dev-dependencies]
Expand Down
12 changes: 10 additions & 2 deletions firepilot/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,18 @@
//! ```
use crate::executor::Executor;

use firepilot_models::models::{BootSource, Drive, NetworkInterface};
use firepilot_models::models::{BootSource, Drive, NetworkInterface, Vsock};

pub mod drive;
pub mod executor;
pub mod kernel;
pub mod network_interface;
pub mod vsock;

fn assert_not_none<T>(key: &str, value: &Option<T>) -> Result<(), BuilderError> {
match value {
Some(_) => Ok(()),
None => return Err(BuilderError::MissingRequiredField(key.to_string())),
None => Err(BuilderError::MissingRequiredField(key.to_string())),
}
}

Expand Down Expand Up @@ -102,6 +103,7 @@ pub struct Configuration {
pub kernel: Option<BootSource>,
pub storage: Vec<Drive>,
pub interfaces: Vec<NetworkInterface>,
pub vsock: Option<Vsock>,

pub vm_id: String,
}
Expand All @@ -113,6 +115,7 @@ impl Configuration {
executor: None,
storage: Vec::new(),
interfaces: Vec::new(),
vsock: None,
vm_id,
}
}
Expand All @@ -137,6 +140,11 @@ impl Configuration {
self.interfaces.push(iface);
self
}

pub fn with_vsock(mut self, vsock: Vsock) -> Configuration {
self.vsock = Some(vsock);
self
}
}

#[cfg(test)]
Expand Down
68 changes: 68 additions & 0 deletions firepilot/src/builder/vsock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
use crate::builder::{Builder, BuilderError};
use firepilot_models::models::Vsock;

use super::assert_not_none;

#[derive(Debug)]
pub struct VsockBuilder {
pub guest_cid: Option<i32>,
pub uds_path: Option<String>,
}

impl Default for VsockBuilder {
fn default() -> Self {
Self::new()
}
}

impl VsockBuilder {
pub fn new() -> VsockBuilder {
VsockBuilder {
guest_cid: None,
uds_path: None,
}
}

pub fn with_guest_cid(mut self, guest_cid: i32) -> VsockBuilder {
self.guest_cid = Some(guest_cid);
self
}

pub fn with_uds_path(mut self, uds_path: String) -> VsockBuilder {
self.uds_path = Some(uds_path);
self
}
}

impl Builder<Vsock> for VsockBuilder {
fn try_build(self) -> Result<Vsock, BuilderError> {
assert_not_none(stringify!(self.guest_cid), &self.guest_cid)?;
assert_not_none(stringify!(self.uds_path), &self.uds_path)?;
Ok(Vsock {
guest_cid: self.guest_cid.unwrap(),
uds_path: self.uds_path.unwrap(),
vsock_id: None,
})
}
}

#[cfg(test)]
mod tests {
use crate::builder::vsock::VsockBuilder;
use crate::builder::Builder;

#[test]
fn full_kernel() {
VsockBuilder::new()
.with_guest_cid(3)
.with_uds_path("/tmp/fc.sock".to_string())
.try_build()
.unwrap();
Comment on lines +56 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having a panic with no context (if it fails), please add the expected behaviour in a expect

Suggested change
VsockBuilder::new()
.with_guest_cid(3)
.with_uds_path("/tmp/fc.sock".to_string())
.try_build()
.unwrap();
VsockBuilder::new()
.with_guest_cid(3)
.with_uds_path("/tmp/fc.sock".to_string())
.try_build()
.expect("Minimal build should be valid")

}

#[test]
#[should_panic]
fn partial_kernel() {
VsockBuilder::new().with_guest_cid(3).try_build().unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

}
}
28 changes: 23 additions & 5 deletions firepilot/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use tracing::{debug, error, info, instrument, trace};

use crate::machine::FirepilotError;
use firepilot_models::models::vm::Vm;
use firepilot_models::models::{BootSource, Drive, NetworkInterface};
use firepilot_models::models::{BootSource, Drive, NetworkInterface, Vsock};

/// Interface to determine how to execute commands on the socket and where to do it
pub trait Execute {
Expand Down Expand Up @@ -61,7 +61,7 @@ impl From<ExecuteError> for FirepilotError {
fn from(e: ExecuteError) -> FirepilotError {
match e {
ExecuteError::CommandExecution(e) => FirepilotError::Setup(e),
ExecuteError::Request(url, e) => FirepilotError::Configure(format!("{}: {}", url, e)),
ExecuteError::Request(url, e) => FirepilotError::Configure(format!("{url}: {e}")),
ExecuteError::Serialize(e) => FirepilotError::Configure(e.to_string()),
ExecuteError::Socket(e) => FirepilotError::Configure(e),
ExecuteError::WorkspaceCreation(e) => FirepilotError::Setup(e),
Expand Down Expand Up @@ -102,6 +102,12 @@ pub struct Executor {
id: String,
}

impl Default for Executor {
fn default() -> Self {
Self::new()
}
}

impl Executor {
Comment on lines +105 to 111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
impl Default for Executor {
fn default() -> Self {
Self::new()
}
}
impl Executor {
#[derive(Default)]
impl Executor {

/// Create a new Executor with no implementation, and with id "default"
pub fn new() -> Executor {
Expand Down Expand Up @@ -135,7 +141,7 @@ impl Executor {
/// Return the configured executor, or panic if none is configured
fn executor(&self) -> &dyn Execute {
match &self.firecracker {
Some(firecracker) => return firecracker,
Some(firecracker) => firecracker,
None => panic!("No executor found"),
}
}
Expand Down Expand Up @@ -195,8 +201,7 @@ impl Executor {
String::from_utf8(body.to_vec()).unwrap()
);
return Err(ExecuteError::CommandExecution(format!(
"Failed to send request to {}, status: {}",
url, status
"Failed to send request to {url}, status: {status}"
)));
}

Expand Down Expand Up @@ -318,6 +323,19 @@ impl Executor {
Ok(())
}

/// Apply vsock configuration on the VM
#[instrument(skip_all, fields(id = %self.id))]
pub async fn configure_vsock(&self, vsock: Vsock) -> Result<(), ExecuteError> {
debug!("Configure vsock");
let json = serde_json::to_string(&vsock).map_err(ExecuteError::Serialize)?;

let path = "/vsock";
let url: hyper::Uri = Uri::new(self.chroot().join("firecracker.socket"), path).into();
self.send_request(url, Method::PUT, json).await?;

Ok(())
}

/// Create needed folders where the VM will be configured
#[instrument(skip(self), fields(id = %self.id))]
pub fn create_workspace(&self) -> Result<(), ExecuteError> {
Expand Down
4 changes: 4 additions & 0 deletions firepilot/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ impl Machine {
self.executor.configure_drives(config.storage).await?;
self.executor.configure_boot_source(kernel).await?;
self.executor.configure_network(config.interfaces).await?;
if let Some(vsock) = config.vsock {
self.executor.configure_vsock(vsock).await?;
}

Ok(())
}

Expand Down