diff options
| -rw-r--r-- | .github/workflows/git-commit-message-style.yml | 3 | ||||
| -rw-r--r-- | test/Cargo.toml | 2 | ||||
| -rw-r--r-- | test/test-manager/Cargo.toml | 1 | ||||
| -rw-r--r-- | test/test-manager/src/vm/network/macos.rs | 85 |
4 files changed, 59 insertions, 32 deletions
diff --git a/.github/workflows/git-commit-message-style.yml b/.github/workflows/git-commit-message-style.yml index 6f93ea9d40..3903db9ca4 100644 --- a/.github/workflows/git-commit-message-style.yml +++ b/.github/workflows/git-commit-message-style.yml @@ -34,4 +34,5 @@ jobs: # This action defaults to 50 char subjects, but 72 is fine. max-subject-line-length: '72' # The action's wordlist is a bit short. Add more accepted verbs - additional-verbs: 'tidy, wrap, obfuscate, bias, prohibit, forbid, revert, slim, impl, disregard, reproduce' + additional-verbs: 'tidy, wrap, obfuscate, bias, prohibit, forbid, revert, slim, impl, disregard, reproduce, + signal' diff --git a/test/Cargo.toml b/test/Cargo.toml index b8b132bab3..7b7b822d3d 100644 --- a/test/Cargo.toml +++ b/test/Cargo.toml @@ -84,4 +84,4 @@ bytes = "1.3.0" async-trait = "0.1.58" socket2 = "0.5.7" surge-ping = "0.8" -nix = { version = "0.30.1", features = ["ioctl", "socket", "net"] } +nix = { version = "0.30.1", features = ["ioctl", "socket", "net", "signal"] } diff --git a/test/test-manager/Cargo.toml b/test/test-manager/Cargo.toml index 4aa8d12018..2225f46ad9 100644 --- a/test/test-manager/Cargo.toml +++ b/test/test-manager/Cargo.toml @@ -68,6 +68,7 @@ socket2 = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] tun = "0.5.1" +nix.workspace = true [dependencies.tokio-util] version = "0.7" diff --git a/test/test-manager/src/vm/network/macos.rs b/test/test-manager/src/vm/network/macos.rs index 4803d2aaa5..39227c7a55 100644 --- a/test/test-manager/src/vm/network/macos.rs +++ b/test/test-manager/src/vm/network/macos.rs @@ -1,8 +1,18 @@ -use anyhow::{Context, Result, anyhow}; -use futures::future::{self, Either}; -use nix::sys::socket::SockaddrStorage; -use std::net::{Ipv4Addr, SocketAddrV4}; -use tokio::{io::AsyncWriteExt, process::Command}; +use anyhow::{Context, Result, anyhow, bail}; +use futures::{FutureExt, TryFutureExt, select}; +use nix::{ + sys::{ + signal::{Signal, kill}, + socket::SockaddrStorage, + }, + unistd::Pid, +}; +use std::{ + convert::Infallible, + net::{Ipv4Addr, SocketAddrV4}, +}; +use talpid_types::drop_guard::on_drop; +use tokio::{io::AsyncWriteExt, process::Command, time::sleep}; // Private key of the wireguard remote peer on host. const CUSTOM_TUN_REMOTE_PRIVKEY: &str = "gLvQuyqazziyf+pUCAFUgTnWIwn6fPE5MOReOqPEGHU="; @@ -86,24 +96,38 @@ async fn enable_forwarding() -> Result<()> { async fn create_wireguard_interface() -> Result<()> { log::debug!("Creating custom WireGuard tunnel"); - let mut go_proc = tokio::spawn(async move { + // Create a future that spawns wireguard-go, and SIGTERMs it when dropped. + let wireguard_go = async move { let mut cmd = Command::new("/usr/bin/sudo"); - cmd.kill_on_drop(true); cmd.args(["wireguard-go", "-f", CUSTOM_TUN_INTERFACE_NAME]); - let output = cmd.output().await.context("Run wireguard-go")?; + + // We don't want to SIGKILL sudo, as that would leave wireguard-go orphaned and running + cmd.kill_on_drop(false); + let child = cmd.spawn().context("Failed to spawn wireguard-go")?; + + let pid = child.id().context("wireguard-go exited prematurely")?; + let pid = Pid::from_raw(pid as libc::pid_t); + + let _term_on_drop = on_drop(|| { + if let Err(e) = kill(pid, Signal::SIGTERM) { + log::warn!("Failed to kill wireguard-go ({pid}): {e}"); + } + }); + + let output = child.wait_with_output().await.context("Run wireguard-go")?; if output.status.success() { - Ok(()) + bail!("wireguard-go exited prematurely") } else { - Err(anyhow!( - "wireguard-go failed with status {:?}", - output.status.code() - )) + bail!("wireguard-go failed with status {:?}", output.status.code()) } - }); + }; - let mut tunnel_check: tokio::task::JoinHandle<Result<()>> = tokio::spawn(async move { + // Spawn wireguard-go using a tokio task. The task will hang around until this process exits. + let wireguard_go = tokio::spawn(wireguard_go.inspect_err(|e| log::error!("{e}"))); + + // Create a future that waits until the tunnel interface appears. + let tunnel_check = async move { loop { - // Check if the tunnel already exists let mut cmd = Command::new("/sbin/ifconfig"); cmd.arg(CUSTOM_TUN_INTERFACE_NAME); let output = cmd @@ -114,24 +138,25 @@ async fn create_wireguard_interface() -> Result<()> { log::debug!("Created custom WireGuard tunnel interface"); return Ok(()); } - tokio::time::sleep(Duration::from_secs(1)).await; + sleep(Duration::from_secs(1)).await; } - }); - - let result = tokio::time::timeout( - INTERFACE_SETUP_TIMEOUT, - future::select(&mut go_proc, &mut tunnel_check), - ) - .await - .context("WireGuard interface setup timed out")?; - - let result = match result { - Either::Left((result, _)) | Either::Right((result, _)) => result, }; - tunnel_check.abort(); + // Wait until... + select! { + // ...the utun-interface is created + result = tunnel_check.fuse() => result, - result? + // ...or wireguard-go exits with an error + result = wireguard_go.fuse() => result + .context("wireguard-go task panicked")? + .map(|never: Infallible| match never {}), // this task never exits with Ok + + // ...or we hit the timeout + _timeout = sleep(INTERFACE_SETUP_TIMEOUT).fuse() => { + bail!("WireGuard interface setup timed out"); + } + } } pub async fn configure_tunnel() -> Result<()> { |