sindri: Replace hand-rolled scp with rsync (#421)

Co-authored-by: Mark Honkoop <markhonkoop@hotmail.com>
Co-authored-by: Gijs de Jong <14833076+oxkitsune@users.noreply.github.com>

Author: 3 people

Cargo.lock

@@ -1,7 +1,7 @@
 [package]
 description = "tool that simplifies robot interaction by automating the process of building and deploying to the robots"
 name = "sindri"
-version = "0.13.0"
+version = "0.14.0"
 
 authors.workspace = true
 categories.workspace = true

tools/sindri/Cargo.toml

@@ -38,6 +38,25 @@ pub async fn has_rerun() -> bool {
     get_rerun_version().await.is_ok_and(|success| success)
 }
 
+/// Check if the `rsync` binary is installed.
+///
+/// We check if the `rsync` binary is installed by running `rsync --version` and checking if the
+/// command was successful.
+pub async fn has_rsync() -> bool {
+    async fn get_rsync_version() -> Result<bool> {
+        Ok(Command::new("rsync")
+            .arg("--version")
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .status()
+            .await
+            .into_diagnostic()?
+            .success())
+    }
+
+    get_rsync_version().await.is_ok_and(|success| success)
+}
+
 /// Compiles the `re_control` binary
 async fn build_re_control() -> Result<()> {
     let features = vec![];

tools/sindri/src/cli/re_control.rs

@@ -2,19 +2,11 @@ use clap::{builder::ArgPredicate, Parser};
 use colored::Colorize;
 use indicatif::{HumanDuration, ProgressBar, ProgressDrawTarget, ProgressStyle};
 use miette::{miette, Context, IntoDiagnostic};
-use ssh2::{ErrorCode, OpenFlags, OpenType, Session, Sftp};
 use std::{
-    borrow::Cow,
-    collections::HashMap,
-    fmt, fs,
-    io::BufWriter,
-    net::Ipv4Addr,
-    path::{Component, Path, PathBuf},
-    str::FromStr,
+    borrow::Cow, collections::HashMap, fmt, fs, net::Ipv4Addr, process::Stdio, str::FromStr,
     time::Duration,
 };
-use tokio::{self, net::TcpStream};
-use walkdir::{DirEntry, WalkDir};
+use tokio::{self, process::Command};
 use yggdrasil::core::config::showtime::ShowtimeConfig;
 use yggdrasil::prelude::*;
 
@@ -33,14 +25,8 @@ const ROBOT_TARGET: &str = "x86_64-unknown-linux-gnu";
 const RELEASE_PATH_REMOTE: &str = "./target/x86_64-unknown-linux-gnu/release/yggdrasil";
 const RELEASE_PATH_LOCAL: &str = "./target/release/yggdrasil";
 const DEPLOY_PATH: &str = "./deploy/yggdrasil";
-const CONNECTION_TIMEOUT: u64 = 5;
-const LOCAL_ROBOT_ID_STR: &str = "0";
 
-/// The size of the `BufWriter`'s buffer.
-///
-/// This is currently set to 1 MiB, as the [`Write`] implementation for [`ssh2::sftp::File`]
-/// is rather slow due to the locking mechanism.
-const UPLOAD_BUFFER_SIZE: usize = 1024 * 1024;
+const LOCAL_ROBOT_ID_STR: &str = "local";
 
 // enum for either the name or the number of a robot thats given
 #[derive(Clone, Debug)]
@@ -601,157 +587,28 @@ pub(crate) async fn stop_single_yggdrasil_service(robot: &Robot, output: Output)
 }
 
 /// Copy the contents of the 'deploy' folder to the robot.
-pub(crate) async fn upload_to_robot(addr: &Ipv4Addr, output: Output) -> Result<()> {
-    output.connecting_phase(addr);
-    let sftp = create_sftp_connection(addr).await?;
-    match output.clone() {
-        Output::Silent => {}
-        Output::Multi(pb) => {
-            pb.set_message(format!("{}", "Connected".bright_blue().bold()));
-        }
-        Output::Single(pb) => {
-            pb.set_message(format!("{}", "  Connected".bright_blue().bold()));
-        }
-    }
-
-    let entries: Vec<DirEntry> = WalkDir::new("./deploy")
-        .into_iter()
-        .filter_map(std::result::Result::ok)
-        .filter(|e| !e.file_name().to_string_lossy().starts_with('.'))
-        .collect();
-    let num_files = entries
-        .iter()
-        .filter(|e| e.metadata().unwrap().is_file())
-        .count();
-
-    output.upload_phase(num_files as u64);
-
-    for entry in &entries {
-        let remote_path = get_remote_path(entry.path());
-
-        if entry.path().is_dir() {
-            // Ensure all directories exist on remote
-            ensure_directory_exists(&sftp, remote_path)?;
-            continue;
-        }
-
-        let file_remote = sftp
-            .open_mode(
-                &remote_path,
-                OpenFlags::WRITE | OpenFlags::TRUNCATE,
-                0o777,
-                OpenType::File,
-            )
-            .map_err(|e| Error::Sftp {
-                source: e,
-                msg: format!("Failed to open remote file {:?}!", entry.path()),
-            })?;
-
-        let mut file_local = std::fs::File::open(entry.path())?;
-
-        match output.clone() {
-            Output::Silent => {}
-            Output::Multi(pb) => {
-                pb.set_message(format!("{}", entry.path().to_string_lossy().dimmed()));
-            }
-            Output::Single(pb) => {
-                pb.set_length(file_local.metadata()?.len());
-                pb.set_message(format!("{}", entry.path().to_string_lossy()));
-            }
-        }
-
-        // Since `file_remote` impl's Write, we can just copy directly using a BufWriter!
-        // The Write impl is rather slow, so we set a large buffer size of 1 mb.
-        let mut buf_writer = BufWriter::with_capacity(UPLOAD_BUFFER_SIZE, file_remote);
-
-        match output.clone() {
-            Output::Silent => {
-                std::io::copy(&mut file_local, &mut buf_writer)?;
-            }
-            Output::Multi(pb) => {
-                std::io::copy(&mut file_local, &mut buf_writer)?;
-                pb.inc(1);
-            }
-            Output::Single(pb) => {
-                std::io::copy(&mut file_local, &mut pb.wrap_write(buf_writer))
-                    .map_err(Error::Io)?;
+pub(crate) async fn upload_to_robot(addr: &Ipv4Addr) -> Result<()> {
+    let output = Command::new("rsync")
+        .args([
+            "-az",
+            "deploy/",
+            &format!("nao@{addr}:/home/nao"),
+            "--out-format=\"%f\"",
+        ])
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .output()
+        .await?;
 
-                pb.println(format!(
-                    "{} {}",
-                    "    Uploaded".bright_blue().bold(),
-                    entry.path().to_string_lossy().dimmed()
-                ));
-            }
+    if !output.status.success() {
+        if let Some(code) = output.status.code() {
+            return Err(Error::Rsync {
+                exit_code: code,
+                reason: String::from_utf8_lossy(&output.stderr).into_owned(),
+            });
         }
     }
 
-    output.spinner();
-
-    if let Output::Multi(pb) = &output {
-        pb.set_message(format!(
-            "    {} {}",
-            "Uploaded".green().bold(),
-            addr.to_string().red()
-        ));
-    }
-
+    println!("{}", String::from_utf8_lossy(&output.stdout));
     Ok(())
 }
-
-async fn create_sftp_connection(ip: &Ipv4Addr) -> Result<Sftp> {
-    let tcp = tokio::time::timeout(
-        Duration::from_secs(CONNECTION_TIMEOUT),
-        TcpStream::connect(format!("{ip}:22")),
-    )
-    .await
-    .map_err(Error::Elapsed)??;
-    let mut session = Session::new().map_err(|e| Error::Sftp {
-        source: e,
-        msg: "Failed to create ssh session!".to_owned(),
-    })?;
-
-    session.set_tcp_stream(tcp);
-    session.handshake().map_err(|e| Error::Sftp {
-        source: e,
-        msg: "Failed to perform ssh handshake!".to_owned(),
-    })?;
-    session
-        .userauth_password("nao", "")
-        .map_err(|e| Error::Sftp {
-            source: e,
-            msg: "Failed to authenticate using ssh!".to_owned(),
-        })?;
-
-    session.sftp().map_err(|e| Error::Sftp {
-        source: e,
-        msg: "Failed to create sftp session!".to_owned(),
-    })
-}
-
-fn ensure_directory_exists(sftp: &Sftp, remote_path: impl AsRef<Path>) -> Result<()> {
-    match sftp.mkdir(remote_path.as_ref(), 0o777) {
-        Ok(()) => Ok(()),
-        // Error code 4, means the directory already exists, so we can ignore it
-        Err(error) if error.code() == ErrorCode::SFTP(4) => Ok(()),
-        Err(error) => Err(Error::Sftp {
-            source: error,
-            msg: "Failed to ensure directory exists".to_owned(),
-        }),
-    }
-}
-
-fn get_remote_path(local_path: &Path) -> PathBuf {
-    let mut remote_path = PathBuf::from("/home/nao");
-
-    for component in local_path.components() {
-        // Would be nice to replace this with an if let chain once https://github.com/rust-lang/rust/issues/53667#issuecomment-1374336460 is stable.
-        match component {
-            // Prevent "deploy" from being added to the remote path, as we'll deploy directly to home directory.
-            Component::Normal(c) if c != "deploy" => remote_path.push(c),
-            // Any other component kind should ignored, such as ".".
-            _ => continue,
-        }
-    }
-
-    remote_path
-}

tools/sindri/src/cli/robot_ops.rs

@@ -12,7 +12,7 @@ use crate::{
     config::SindriConfig,
 };
 
-use super::re_control::{has_rerun, run_re_control};
+use super::re_control::{has_rerun, has_rsync, run_re_control};
 
 const DEFAULT_TRACY_PORT: u16 = 8086;
 
@@ -40,8 +40,13 @@ impl Run {
 
         let local = self.robot_ops.local;
         let rerun = self.robot_ops.rerun_args.rerun.is_some();
-        let has_rerun = has_rerun().await;
 
+        let has_rsync = has_rsync().await;
+        if !has_rsync {
+            bail!("rsync is not installed, install it using your package manager!")
+        }
+
+        let has_rerun = has_rerun().await;
         if rerun && !has_rerun {
             println!(
                 "{}: {}",
@@ -70,7 +75,7 @@ impl Run {
         if !self.robot_ops.local {
             output.spinner();
             robot_ops::stop_single_yggdrasil_service(&robot, output.clone()).await?;
-            robot_ops::upload_to_robot(&robot.ip(), output.clone()).await?;
+            robot_ops::upload_to_robot(&robot.ip()).await?;
 
             if let Some(network) = self.robot_ops.network {
                 output.spinner();

tools/sindri/src/cli/run.rs

@@ -43,7 +43,7 @@ impl Showtime {
 
             output.spinner();
             robot_ops::stop_single_yggdrasil_service(&robot, output.clone()).await?;
-            robot_ops::upload_to_robot(&robot.ip(), output.clone()).await?;
+            robot_ops::upload_to_robot(&robot.ip()).await?;
             output.spinner();
             robot_ops::start_single_yggdrasil_service(&robot, output.clone()).await?;
 
@@ -101,7 +101,7 @@ impl Showtime {
                     .block_on(async move {
                         output.spinner();
                         robot_ops::stop_single_yggdrasil_service(&robot, output.clone()).await?;
-                        robot_ops::upload_to_robot(&robot.ip(), output.clone()).await?;
+                        robot_ops::upload_to_robot(&robot.ip()).await?;
                         output.spinner();
                         robot_ops::start_single_yggdrasil_service(&robot, output.clone()).await?;
 

tools/sindri/src/cli/showtime.rs

@@ -14,12 +14,8 @@ pub enum Error {
     #[error(transparent)]
     Cargo(crate::cargo::CargoError),
 
-    #[error("Sftp error: {msg}")]
-    Sftp {
-        #[source]
-        source: ssh2::Error,
-        msg: String,
-    },
+    #[error("Rsync error: {reason}, look up rsync error code: {exit_code:?}")]
+    Rsync { exit_code: i32, reason: String },
     #[error("Ssh error: {command}")]
     Ssh {
         #[source]