merge: #4020

4020: chore: firecracker scripts as bytes instead of from repo r=sprutton1 a=sprutton1

This moves the firecracker scripts into the firecracker library so we no longer go up to github to get them. This means you can make changes to them in a branch without having to muck with having to change which branch they are pulled from in the repo. Also some refactoring as I learn more about how to handle errors correctly.

Co-authored-by: Scott Prutton <[email protected]>

bin/veritech/BUCK

@@ -45,6 +45,8 @@ rust_binary(
         "dev.decryption.key": "//lib/veritech-server:dev.decryption.key",
         "lang-js": "//bin/lang-js:bin",
         "firecracker-setup.sh": "//lib/si-firecracker:firecracker-setup.sh",
+        "prepare_jailer.sh": "//lib/si-firecracker:prepare_jailer.sh",
+        "stop.sh": "//lib/si-firecracker:stop.sh",
     },
     visibility = ["PUBLIC"],
 )

lib/si-firecracker/BUCK

@@ -8,11 +8,23 @@ rust_library(
         "//third-party/rust:thiserror",
         "//third-party/rust:tokio",
     ],
-    srcs = glob(["src/**/*.rs","src/firecracker-setup.sh"]),
+    srcs = glob(["src/**/*.rs","src/scripts/*"]),
 )
 
 export_file(
     name = "firecracker-setup.sh",
-    src = "src/firecracker-setup.sh",
+    src = "src/scripts/firecracker-setup.sh",
+    visibility = ["PUBLIC"],
+)
+
+export_file(
+    name = "prepare_jailer.sh",
+    src = "src/scripts/prepare_jailer.sh",
+    visibility = ["PUBLIC"],
+)
+
+export_file(
+    name = "stop.sh",
+    src = "src/scripts/stop.sh",
     visibility = ["PUBLIC"],
 )

lib/si-firecracker/src/errors.rs

@@ -6,16 +6,19 @@ use thiserror::Error;
 pub enum FirecrackerJailError {
     // Failed to clean a jail
     #[error("Failed to clean a jail: {0}")]
-    Clean(String),
+    Clean(#[source] tokio::io::Error),
+    // Failed running a script to output
+    #[error("Failed to run a script: {0}")]
+    Output(String),
     // Failed to prepare a jail
     #[error("Failed to prepare a jail: {0}")]
-    Prepare(String),
+    Prepare(#[source] tokio::io::Error),
     // Failed to setup firecracker
     #[error("Failed to setup firecracker: {0}")]
-    Setup(String),
+    Setup(#[from] tokio::io::Error),
     // Failed to spawn firecracker
     #[error("Failed to spawn firecracker: {0}")]
-    Spawn(String),
+    Spawn(#[source] tokio::io::Error),
     // Failed to terminate firecracker
     #[error("Failed to terminate firecracker: {0}")]
     Terminate(#[from] cyclone_core::process::ShutdownError),

lib/si-firecracker/src/firecracker.rs

@@ -1,14 +1,29 @@
 use crate::errors::FirecrackerJailError;
 use cyclone_core::process;
+use std::fs::Permissions;
 use std::os::unix::fs::PermissionsExt;
 use std::path::Path;
 use std::path::PathBuf;
 use std::result;
+use tokio::fs;
 use tokio::process::Child;
 use tokio::process::Command;
 
 type Result<T> = result::Result<T, FirecrackerJailError>;
 
+const FIRECRACKER_PREPARE_PATH: &str = "/firecracker-data/prepare_jailer.sh";
+const FIRECRACKER_SETUP_PATH: &str = "/firecracker-data/firecracker-setup.sh";
+const FIRECRACKER_STOP_PATH: &str = "/firecracker-data/stop.sh";
+const FIRECRACKER_PREPARE_BYTES: &[u8] = include_bytes!("scripts/prepare_jailer.sh");
+const FIRECRACKER_SETUP_BYTES: &[u8] = include_bytes!("scripts/firecracker-setup.sh");
+const FIRECRACKER_STOP_BYTES: &[u8] = include_bytes!("scripts/stop.sh");
+
+const FIRECRACKER_SCRIPTS: &[(&str, &[u8])] = &[
+    (FIRECRACKER_PREPARE_PATH, FIRECRACKER_PREPARE_BYTES),
+    (FIRECRACKER_SETUP_PATH, FIRECRACKER_SETUP_BYTES),
+    (FIRECRACKER_STOP_PATH, FIRECRACKER_STOP_BYTES),
+];
+
 #[derive(Debug)]
 pub struct FirecrackerJail {
     jailer: Command,
@@ -48,15 +63,14 @@ impl FirecrackerJail {
     }
 
     pub async fn clean(id: u32) -> Result<()> {
-        let command = String::from("/firecracker-data/stop.sh");
-        let output = Command::new(command)
+        let output = Command::new(FIRECRACKER_STOP_PATH)
             .arg(id.to_string())
             .output()
             .await
-            .map_err(|e| FirecrackerJailError::Clean(e.to_string()))?;
+            .map_err(FirecrackerJailError::Clean)?;
 
         if !output.status.success() {
-            return Err(FirecrackerJailError::Clean(
+            return Err(FirecrackerJailError::Output(
                 String::from_utf8(output.stderr)
                     .unwrap_or_else(|_| "Failed to decode stderr".to_string()),
             ));
@@ -65,15 +79,14 @@ impl FirecrackerJail {
     }
 
     pub async fn prepare(id: u32) -> Result<()> {
-        let command = String::from("/firecracker-data/prepare_jailer.sh");
-        let output = Command::new(command)
+        let output = Command::new(FIRECRACKER_PREPARE_PATH)
             .arg(id.to_string())
             .output()
             .await
-            .map_err(|e| FirecrackerJailError::Prepare(e.to_string()))?;
+            .map_err(FirecrackerJailError::Prepare)?;
 
         if !output.status.success() {
-            return Err(FirecrackerJailError::Prepare(
+            return Err(FirecrackerJailError::Output(
                 String::from_utf8(output.stderr)
                     .unwrap_or_else(|_| "Failed to decode stderr".to_string()),
             ));
@@ -82,37 +95,19 @@ impl FirecrackerJail {
     }
 
     pub async fn setup(pool_size: u16) -> Result<()> {
-        let script_bytes = include_bytes!("firecracker-setup.sh");
-        let command = Path::new("/firecracker-data/firecracker-setup.sh");
-
-        // we need to ensure the file is in the correct location with the correct permissions
-        std::fs::create_dir_all(
-            command
-                .parent()
-                .expect("This should never happen. Did you remove the path from the string above?"),
-        )
-        .map_err(|e| FirecrackerJailError::Setup(e.to_string()))?;
-
-        std::fs::write(command, script_bytes)
-            .map_err(|e| FirecrackerJailError::Setup(e.to_string()))?;
+        Self::create_scripts().await?;
 
-        std::fs::set_permissions(command, std::fs::Permissions::from_mode(0o755))
-            .map_err(|e| FirecrackerJailError::Setup(e.to_string()))?;
-
-        // Spawn the shell process
         let output = Command::new("sudo")
-            .arg(command)
+            .arg(FIRECRACKER_SETUP_PATH)
             .arg("-j")
             .arg(pool_size.to_string())
             .arg("-rk")
-            .spawn()
-            .map_err(|e| FirecrackerJailError::Setup(e.to_string()))?
+            .spawn()?
             .wait_with_output()
-            .await
-            .map_err(|e| FirecrackerJailError::Setup(e.to_string()))?;
+            .await?;
 
         if !output.status.success() {
-            return Err(FirecrackerJailError::Setup(
+            return Err(FirecrackerJailError::Output(
                 String::from_utf8(output.stderr)
                     .unwrap_or_else(|_| "Failed to decode stderr".to_string()),
             ));
@@ -122,11 +117,7 @@ impl FirecrackerJail {
     }
 
     pub async fn spawn(&mut self) -> Result<()> {
-        self.child = Some(
-            self.jailer
-                .spawn()
-                .map_err(|e| FirecrackerJailError::Spawn(e.to_string()))?,
-        );
+        self.child = Some(self.jailer.spawn().map_err(FirecrackerJailError::Spawn)?);
         Ok(())
     }
 
@@ -139,4 +130,20 @@ impl FirecrackerJail {
             None => Ok(()),
         }
     }
+
+    async fn create_scripts() -> Result<()> {
+        for (path, bytes) in FIRECRACKER_SCRIPTS {
+            Self::create_script(Path::new(*path), bytes).await?;
+        }
+        Ok(())
+    }
+
+    async fn create_script(path: &Path, bytes: &[u8]) -> Result<()> {
+        if let Some(parent_dir) = path.parent() {
+            fs::create_dir_all(parent_dir).await?
+        }
+        fs::write(&path, bytes).await?;
+        fs::set_permissions(&path, Permissions::from_mode(0o755)).await?;
+        Ok(())
+    }
 }

lib/si-firecracker/src/firecracker-setup.sh → lib/si-firecracker/src/scripts/firecracker-setup.sh

@@ -124,11 +124,6 @@ execute_configuration_management() {
         # Mount secondary EBS volume at /data for
         mkdir -p /firecracker-data/output/ && cd /firecracker-data/
 
-        # Helper Scripts
-        curl https://raw.githubusercontent.com/systeminit/si/${CONFIGURATION_MANAGEMENT_BRANCH:-main}/bin/veritech/scripts/start.sh > ./start.sh
-        curl https://raw.githubusercontent.com/systeminit/si/${CONFIGURATION_MANAGEMENT_BRANCH:-main}/bin/veritech/scripts/stop.sh > ./stop.sh
-        curl https://raw.githubusercontent.com/systeminit/si/${CONFIGURATION_MANAGEMENT_BRANCH:-main}/bin/veritech/scripts/prepare_jailer.sh > ./prepare_jailer.sh
-
         arch=$(uname -m)
         # Remainder of the binaries
         # TODO(scott): perform some kind of check to decide if we should
@@ -198,7 +193,7 @@ execute_configuration_management() {
 
         # Set up correct permissions for the /firecracker-data/ folder
         chown -R jailer-shared:jailer-shared /firecracker-data/
-        chmod a+x /firecracker-data/*{.sh,firecracker,jailer}
+        chmod a+x /firecracker-data/{firecracker,jailer}
         # chmod 400 /firecracker-data/micro-vm-key
 
         # Copy bins to /usr/bin/