From 45fac1faeb966047b8d4c58d227147b679d7db63 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Wed, 15 Oct 2025 13:48:08 +0200 Subject: [PATCH 01/18] wip --- crates/icp-cli/src/commands/network/run.rs | 137 +++++++++++++++++++-- 1 file changed, 129 insertions(+), 8 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 8bc3d4e8..85527f99 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -1,18 +1,34 @@ +use std::{process::Command, time::Duration}; + use clap::Parser; +use ic_agent::{Agent, AgentError}; use icp::{ identity::manifest::{LoadIdentityManifestError, load_identity_list}, manifest, - network::{Configuration, NetworkDirectory, RunNetworkError, run_network}, + network::{ + Configuration, NetworkDirectory, RunNetworkError, config::NetworkDescriptorModel, + run_network, + }, }; use crate::commands::Context; +const BACKGROUND_ENV_VAR: &str = "ICP_CLI_RUN_NETWORK_BACKGROUND"; + /// Run a given network #[derive(Parser, Debug)] pub struct Cmd { /// Name of the network to run #[arg(default_value = "local")] name: String, + + /// Starts the network in a background process. This command will exit once the network is running. + #[arg(long)] + background: bool, + + /// Set if this is the process that runs the network in the background + #[arg(long, env = BACKGROUND_ENV_VAR, hide = true)] + run_in_background: bool, } #[derive(Debug, thiserror::Error)] @@ -23,12 +39,18 @@ pub enum CommandError { #[error(transparent)] Locate(#[from] manifest::LocateError), + #[error(transparent)] + Agent(#[from] AgentError), + #[error("project does not contain a network named '{name}'")] Network { name: String }, #[error("network '{name}' must be a managed network")] Unmanaged { name: String }, + #[error("timed out waiting for network to start: {err}")] + Timeout { err: String }, + #[error(transparent)] Identities(#[from] LoadIdentityManifestError), @@ -79,13 +101,112 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { eprintln!("Project root: {pdir}"); eprintln!("Network root: {ndir}"); - run_network( - cfg, // config - nd, // nd - &pdir, // project_root - seed_accounts, // seed_accounts - ) - .await?; + if cmd.background { + run_in_background()?; + wait_for_healthy_network(&nd).await?; + } else { + run_network( + cfg, // config + nd, // nd + &pdir, // project_root + seed_accounts, // seed_accounts + ) + .await?; + } + Ok(()) +} + +fn run_in_background() -> Result<(), CommandError> { + // Background strategy is different; we spawn `dfx` with the same arguments + // (minus --background), ping and exit. + let exe = std::env::current_exe().expect("Failed to get current executable."); + let mut cmd = Command::new(exe); + // Skip 1 because arg0 is this executable's path. + cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) + .env(BACKGROUND_ENV_VAR, "true"); // Set the environment variable which will be used by the second start. + cmd.spawn().expect("Failed to spawn child process."); Ok(()) } + +async fn retry_with_timeout(mut f: F, max_retries: usize, delay_ms: u64) -> Option +where + F: FnMut() -> Fut, + Fut: std::future::Future> + Send, +{ + let mut retries = 0; + loop { + if let Some(result) = f().await { + return Some(result); + } + if retries > max_retries { + return None; + } + retries += 1; + tokio::time::sleep(Duration::from_millis(delay_ms)).await; + } +} + +async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandError> { + let network = wait_for_network_descriptor(nd).await?; + let port = network.gateway.port; + let agent = Agent::builder() + .with_url(format!("127.0.0.1:{port}")) + .build()?; + + let max_retries = 30; + let delay_ms = 1000; + let result: Option<()> = retry_with_timeout( + || { + let agent = agent.clone(); + async move { + let status = agent.status().await; + if let Ok(status) = status { + if matches!(&status.replica_health_status, Some(status) if status == "healthy") + { + return Some(()); + } + } + None + } + }, + max_retries, + delay_ms, + ) + .await; + + match result { + Some(()) => Ok(()), + None => Err(CommandError::Timeout { + err: "timed out waiting for network to start".to_string(), + }), + } +} + +async fn wait_for_network_descriptor( + nd: &NetworkDirectory, +) -> Result { + let max_retries = 30; + let delay_ms = 1000; + let result: Option = retry_with_timeout( + || { + let nd = nd; + async move { + if let Ok(Some(descriptor)) = nd.load_network_descriptor() { + return Some(descriptor); + } + None + } + }, + max_retries, + delay_ms, + ) + .await; + + match result { + Some(descriptor) => Ok(descriptor), + None => Err(CommandError::Timeout { + err: "timed out waiting for network descriptor".to_string(), + }), + } +} From f583a6b88573a9aaea2522b77d3e92a00d0af809 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Wed, 15 Oct 2025 13:56:24 +0200 Subject: [PATCH 02/18] start seems to work --- crates/icp-cli/src/commands/network/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 85527f99..a6aa89d2 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -151,7 +151,7 @@ async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandEr let network = wait_for_network_descriptor(nd).await?; let port = network.gateway.port; let agent = Agent::builder() - .with_url(format!("127.0.0.1:{port}")) + .with_url(format!("http://127.0.0.1:{port}")) .build()?; let max_retries = 30; From bb38a607afd3fd2c736775a341f1972c6e006541 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Wed, 15 Oct 2025 14:54:48 +0200 Subject: [PATCH 03/18] save controller PID --- crates/icp-cli/src/commands/network/run.rs | 18 ++++-- crates/icp/src/network/directory.rs | 74 +++++++++++++++++++++- crates/icp/src/network/mod.rs | 2 +- crates/icp/src/network/structure.rs | 6 ++ 4 files changed, 91 insertions(+), 9 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index a6aa89d2..3ae63ee8 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -1,4 +1,7 @@ -use std::{process::Command, time::Duration}; +use std::{ + process::{Child, Command}, + time::Duration, +}; use clap::Parser; use ic_agent::{Agent, AgentError}; @@ -56,6 +59,9 @@ pub enum CommandError { #[error(transparent)] RunNetwork(#[from] RunNetworkError), + + #[error(transparent)] + SavePid(#[from] icp::network::SavePidError), } pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { @@ -102,7 +108,8 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { eprintln!("Network root: {ndir}"); if cmd.background { - run_in_background()?; + let child = run_in_background()?; + nd.save_background_network_runner_pid(child.id())?; wait_for_healthy_network(&nd).await?; } else { run_network( @@ -116,7 +123,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { Ok(()) } -fn run_in_background() -> Result<(), CommandError> { +fn run_in_background() -> Result { // Background strategy is different; we spawn `dfx` with the same arguments // (minus --background), ping and exit. let exe = std::env::current_exe().expect("Failed to get current executable."); @@ -124,9 +131,8 @@ fn run_in_background() -> Result<(), CommandError> { // Skip 1 because arg0 is this executable's path. cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) .env(BACKGROUND_ENV_VAR, "true"); // Set the environment variable which will be used by the second start. - - cmd.spawn().expect("Failed to spawn child process."); - Ok(()) + let child = cmd.spawn().expect("Failed to spawn child process."); + Ok(child) } async fn retry_with_timeout(mut f: F, max_retries: usize, delay_ms: u64) -> Option diff --git a/crates/icp/src/network/directory.rs b/crates/icp/src/network/directory.rs index 0b39bbea..3fc38f93 100644 --- a/crates/icp/src/network/directory.rs +++ b/crates/icp/src/network/directory.rs @@ -1,9 +1,9 @@ -use std::io::ErrorKind; +use std::io::{ErrorKind, Seek, Write}; use snafu::prelude::*; use crate::{ - fs::{create_dir_all, json}, + fs::{create_dir_all, json, read_to_string}, network::{ config::NetworkDescriptorModel, lock::{AcquireWriteLockError, OpenFileForWriteLockError, RwFileLock}, @@ -143,6 +143,46 @@ impl NetworkDirectory { } Ok(()) } + + fn open_background_runner_pid_file_for_writelock( + &self, + ) -> Result { + RwFileLock::open_for_write(self.structure.background_network_runner_pid_file()) + } + + pub fn save_background_network_runner_pid(&self, pid: u32) -> Result<(), SavePidError> { + let mut file_lock = self.open_background_runner_pid_file_for_writelock()?; + let mut write_guard = file_lock.acquire_write_lock()?; + + // Truncate the file first + write_guard.set_len(0).context(TruncatePidFileSnafu { + path: self.structure.background_network_runner_pid_file(), + })?; + + (*write_guard) + .seek(std::io::SeekFrom::Start(0)) + .context(TruncatePidFileSnafu { + path: self.structure.background_network_runner_pid_file(), + })?; + + // Write the PID + write!(*write_guard, "{}", pid).context(WritePidSnafu { + path: self.structure.background_network_runner_pid_file(), + })?; + + Ok(()) + } + + pub fn load_background_network_runner_pid(&self) -> Result, LoadPidError> { + let path = self.structure.background_network_runner_pid_file(); + + read_to_string(&path) + .map(|content| content.trim().parse::().ok()) + .or_else(|err| match err.kind() { + ErrorKind::NotFound => Ok(None), + _ => Err(err).context(ReadPidSnafu { path: path.clone() }), + }) + } } #[derive(Debug, Snafu)] @@ -171,3 +211,33 @@ pub enum CleanupNetworkDescriptorError { #[snafu(transparent)] AcquireWriteLock { source: AcquireWriteLockError }, } + +#[derive(Debug, Snafu)] +pub enum SavePidError { + #[snafu(transparent)] + OpenFileForWriteLock { source: OpenFileForWriteLockError }, + + #[snafu(transparent)] + AcquireWriteLock { source: AcquireWriteLockError }, + + #[snafu(display("failed to truncate PID file at {path}"))] + TruncatePidFile { + source: std::io::Error, + path: PathBuf, + }, + + #[snafu(display("failed to write PID to {path}"))] + WritePid { + source: std::io::Error, + path: PathBuf, + }, +} + +#[derive(Debug, Snafu)] +pub enum LoadPidError { + #[snafu(display("failed to read PID from {path}"))] + ReadPid { + source: crate::fs::Error, + path: PathBuf, + }, +} diff --git a/crates/icp/src/network/mod.rs b/crates/icp/src/network/mod.rs index 1004118b..2c25c9ac 100644 --- a/crates/icp/src/network/mod.rs +++ b/crates/icp/src/network/mod.rs @@ -5,7 +5,7 @@ use async_trait::async_trait; use schemars::JsonSchema; use serde::{Deserialize, Deserializer}; -pub use directory::NetworkDirectory; +pub use directory::{LoadPidError, NetworkDirectory, SavePidError}; pub use managed::run::{RunNetworkError, run_network}; use crate::{ diff --git a/crates/icp/src/network/structure.rs b/crates/icp/src/network/structure.rs index 93e30071..923c3df5 100644 --- a/crates/icp/src/network/structure.rs +++ b/crates/icp/src/network/structure.rs @@ -46,4 +46,10 @@ impl NetworkDirectoryStructure { pub fn pocketic_port_file(&self) -> PathBuf { self.pocketic_dir().join("port") } + + /// When running a network in the background, we store the PID of the background controlling process here. + /// Once that process receives a SIGINT, it will shut down the network. + pub fn background_network_runner_pid_file(&self) -> PathBuf { + self.network_root.join("background_network_runner.pid") + } } From 58cc32e4c0da5d428eeaf2f366a4b37b0078c9ba Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 10:49:20 +0200 Subject: [PATCH 04/18] network can now start in tests --- crates/icp-cli/src/commands/network/run.rs | 9 +++++++-- crates/icp-cli/tests/network_tests.rs | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 3ae63ee8..e74280e8 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -1,5 +1,5 @@ use std::{ - process::{Child, Command}, + process::{Child, Command, Stdio}, time::Duration, }; @@ -97,6 +97,8 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { &ndir, // network_root &ctx.dirs.port_descriptor(), // port_descriptor_dir ); + nd.ensure_exists() + .map_err(|e| RunNetworkError::CreateDirFailed { source: e })?; // Identities let ids = load_identity_list(&ctx.dirs.identity())?; @@ -130,7 +132,10 @@ fn run_in_background() -> Result { let mut cmd = Command::new(exe); // Skip 1 because arg0 is this executable's path. cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) - .env(BACKGROUND_ENV_VAR, "true"); // Set the environment variable which will be used by the second start. + .env(BACKGROUND_ENV_VAR, "true") // Set the environment variable which will be used by the second start. + .stdin(Stdio::null()) // Redirect stdin from /dev/null + .stdout(Stdio::null()) // Redirect stdout to /dev/null + .stderr(Stdio::null()); // Redirect stderr to /dev/null let child = cmd.spawn().expect("Failed to spawn child process."); Ok(child) } diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 4de27631..ed68fd28 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -286,3 +286,21 @@ fn network_seeds_preexisting_identities_icp_and_cycles_balances() { .stdout(contains("Balance: 0 TCYCLES")) .success(); } + +#[test] +fn network_run_background() { + let ctx = TestContext::new(); + + let project_dir = ctx.create_project_dir("icp"); + + // Project manifest + write_string(&project_dir.join("icp.yaml"), NETWORK_RANDOM_PORT) + .expect("failed to write project manifest"); + + // start network in background + ctx.icp() + .current_dir(&project_dir) + .args(["network", "run", "my-network", "--background"]) + .assert() + .success(); +} From 9fa728149e0b307e76ef8f2cf136e3ed5258f0d5 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 11:04:13 +0200 Subject: [PATCH 05/18] confirm network can be pinged --- crates/icp-cli/tests/network_tests.rs | 29 +++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index ed68fd28..9767eef5 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -287,8 +287,8 @@ fn network_seeds_preexisting_identities_icp_and_cycles_balances() { .success(); } -#[test] -fn network_run_background() { +#[tokio::test] +async fn network_run_background() { let ctx = TestContext::new(); let project_dir = ctx.create_project_dir("icp"); @@ -303,4 +303,29 @@ fn network_run_background() { .args(["network", "run", "my-network", "--background"]) .assert() .success(); + let network = ctx.wait_for_network_descriptor(&project_dir, "my-network"); + + // Verify PID file was written + let pid_file_path = project_dir + .join(".icp") + .join("networks") + .join("my-network") + .join("background_network_runner.pid"); + assert!( + pid_file_path.exists(), + "PID file should exist at {:?}", + pid_file_path + ); + + // Verify network can be pinged with agent.status() + let agent = ic_agent::Agent::builder() + .with_url(format!("http://127.0.0.1:{}", network.gateway_port)) + .build() + .expect("Failed to build agent"); + + let status = agent.status().await.expect("Failed to get network status"); + assert!( + matches!(&status.replica_health_status, Some(health) if health == "healthy"), + "Network should be healthy" + ); } From 69fb3b12f3fa9509bebf36d17165c1b69f22e665 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 11:29:46 +0200 Subject: [PATCH 06/18] show output during network start --- crates/icp-cli/src/commands/network/run.rs | 66 ++++++++++++++++++++-- crates/icp-cli/tests/network_tests.rs | 7 ++- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index e74280e8..f14c1847 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -1,5 +1,7 @@ use std::{ + io::{BufRead, BufReader}, process::{Child, Command, Stdio}, + thread, time::Duration, }; @@ -110,9 +112,14 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { eprintln!("Network root: {ndir}"); if cmd.background { - let child = run_in_background()?; + let mut child = run_in_background()?; nd.save_background_network_runner_pid(child.id())?; - wait_for_healthy_network(&nd).await?; + + // Relay stdout/stderr from child to parent until network is healthy + relay_child_output_until_healthy(&mut child, &nd).await?; + + // Explicitly forget the child handle so parent doesn't wait for it + std::mem::forget(child); } else { run_network( cfg, // config @@ -126,20 +133,67 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { } fn run_in_background() -> Result { - // Background strategy is different; we spawn `dfx` with the same arguments - // (minus --background), ping and exit. + // Background strategy: spawn a child process with piped stdout/stderr + // so the parent can relay output until the network is healthy, then detach. let exe = std::env::current_exe().expect("Failed to get current executable."); let mut cmd = Command::new(exe); // Skip 1 because arg0 is this executable's path. cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) .env(BACKGROUND_ENV_VAR, "true") // Set the environment variable which will be used by the second start. .stdin(Stdio::null()) // Redirect stdin from /dev/null - .stdout(Stdio::null()) // Redirect stdout to /dev/null - .stderr(Stdio::null()); // Redirect stderr to /dev/null + .stdout(Stdio::piped()) // Capture stdout so parent can relay it + .stderr(Stdio::piped()); // Capture stderr so parent can relay it + + // On Unix, create a new process group so the child can continue running + // independently after the parent exits + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + cmd.process_group(0); + } + let child = cmd.spawn().expect("Failed to spawn child process."); Ok(child) } +async fn relay_child_output_until_healthy( + child: &mut Child, + nd: &NetworkDirectory, +) -> Result<(), CommandError> { + // Take stdout and stderr from the child + let stdout = child.stdout.take().expect("Failed to take child stdout"); + let stderr = child.stderr.take().expect("Failed to take child stderr"); + + // Spawn threads to relay output + let stdout_thread = thread::spawn(move || { + let reader = BufReader::new(stdout); + for line in reader.lines() { + if let Ok(line) = line { + println!("{}", line); // stdout -> stdout + } + } + }); + + let stderr_thread = thread::spawn(move || { + let reader = BufReader::new(stderr); + for line in reader.lines() { + if let Ok(line) = line { + eprintln!("{}", line); // stderr -> stderr + } + } + }); + + // Wait for network to become healthy + wait_for_healthy_network(nd).await?; + + // Note: We don't join the threads - they'll continue running until the pipes close + // or we can just let them be orphaned since the parent is about to exit + drop(stdout_thread); + drop(stderr_thread); + + Ok(()) +} + async fn retry_with_timeout(mut f: F, max_retries: usize, delay_ms: u64) -> Option where F: FnMut() -> Fut, diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 9767eef5..bf54ca8a 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -297,12 +297,15 @@ async fn network_run_background() { write_string(&project_dir.join("icp.yaml"), NETWORK_RANDOM_PORT) .expect("failed to write project manifest"); - // start network in background + // start network in background and verify we can see child process output ctx.icp() .current_dir(&project_dir) .args(["network", "run", "my-network", "--background"]) .assert() - .success(); + .success() + .stderr(contains("Project root:")) + .stderr(contains("Network root:")); + let network = ctx.wait_for_network_descriptor(&project_dir, "my-network"); // Verify PID file was written From 3defc81a6047df14f6dc5255b4fb98903ecfd142 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 14:37:30 +0200 Subject: [PATCH 07/18] add stop command --- Cargo.lock | 161 +++++++++++++++++++- Cargo.toml | 2 + crates/icp-cli/Cargo.toml | 2 + crates/icp-cli/src/commands/network/mod.rs | 6 + crates/icp-cli/src/commands/network/stop.rs | 121 +++++++++++++++ crates/icp-cli/tests/network_tests.rs | 42 ++++- crates/icp/Cargo.toml | 1 + crates/icp/src/network/directory.rs | 5 +- 8 files changed, 332 insertions(+), 8 deletions(-) create mode 100644 crates/icp-cli/src/commands/network/stop.rs diff --git a/Cargo.lock b/Cargo.lock index 21d6cf23..d2637bb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -301,7 +301,7 @@ dependencies = [ "miniz_oxide", "object", "rustc-demangle", - "windows-link", + "windows-link 0.2.0", ] [[package]] @@ -2471,6 +2471,7 @@ dependencies = [ "slog", "snafu", "strum 0.27.2", + "sysinfo", "thiserror 2.0.17", "time", "tiny-bip39 2.0.0", @@ -2535,6 +2536,7 @@ dependencies = [ "serde_yaml", "serial_test", "snafu", + "sysinfo", "thiserror 2.0.17", "tiny-bip39 2.0.0", "tokio", @@ -3159,6 +3161,15 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" +[[package]] +name = "ntapi" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8a3895c6391c39d7fe7ebc444a87eb2991b2a0bc718fdabd071eec617fc68e4" +dependencies = [ + "winapi", +] + [[package]] name = "nu-ansi-term" version = "0.50.1" @@ -3230,6 +3241,25 @@ dependencies = [ "libm", ] +[[package]] +name = "objc2-core-foundation" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a180dd8642fa45cdb7dd721cd4c11b1cadd4929ce112ebd8b9f5803cc79d536" +dependencies = [ + "bitflags 2.9.4", +] + +[[package]] +name = "objc2-io-kit" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33fafba39597d6dc1fb709123dfa8289d39406734be322956a69f0931c73bb15" +dependencies = [ + "libc", + "objc2-core-foundation", +] + [[package]] name = "object" version = "0.37.3" @@ -4968,6 +4998,20 @@ dependencies = [ "syn 2.0.106", ] +[[package]] +name = "sysinfo" +version = "0.37.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16607d5caffd1c07ce073528f9ed972d88db15dd44023fa57142963be3feb11f" +dependencies = [ + "libc", + "memchr", + "ntapi", + "objc2-core-foundation", + "objc2-io-kit", + "windows", +] + [[package]] name = "tap" version = "1.0.1" @@ -5744,12 +5788,114 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows" +version = "0.61.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893" +dependencies = [ + "windows-collections", + "windows-core", + "windows-future", + "windows-link 0.1.3", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" +dependencies = [ + "windows-core", +] + +[[package]] +name = "windows-core" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link 0.1.3", + "windows-result", + "windows-strings", +] + +[[package]] +name = "windows-future" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" +dependencies = [ + "windows-core", + "windows-link 0.1.3", + "windows-threading", +] + +[[package]] +name = "windows-implement" +version = "0.60.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] + +[[package]] +name = "windows-interface" +version = "0.59.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] + +[[package]] +name = "windows-link" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" + [[package]] name = "windows-link" version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "45e46c0661abb7180e7b9c281db115305d49ca1709ab8242adf09666d2173c65" +[[package]] +name = "windows-numerics" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" +dependencies = [ + "windows-core", + "windows-link 0.1.3", +] + +[[package]] +name = "windows-result" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" +dependencies = [ + "windows-link 0.1.3", +] + +[[package]] +name = "windows-strings" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows-sys" version = "0.52.0" @@ -5783,7 +5929,7 @@ version = "0.61.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6f109e41dd4a3c848907eb83d5a42ea98b3769495597450cf6d153507b166f0f" dependencies = [ - "windows-link", + "windows-link 0.2.0", ] [[package]] @@ -5808,7 +5954,7 @@ version = "0.53.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d42b7b7f66d2a06854650af09cfdf8713e427a439c97ad65a6375318033ac4b" dependencies = [ - "windows-link", + "windows-link 0.2.0", "windows_aarch64_gnullvm 0.53.0", "windows_aarch64_msvc 0.53.0", "windows_i686_gnu 0.53.0", @@ -5819,6 +5965,15 @@ dependencies = [ "windows_x86_64_msvc 0.53.0", ] +[[package]] +name = "windows-threading" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66463ad2e0ea3bbf808b7f1d371311c80e115c0b71d60efc142cafbcfb057a6" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" diff --git a/Cargo.toml b/Cargo.toml index 6aeb57e0..075c5858 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ itertools = "0.14.0" k256 = { version = "0.13.4", features = ["pem", "pkcs8", "std"] } lazy_static = "1.5.0" mockall = "0.13.1" +nix = { version = "0.30.1", features = ["process", "signal"] } p256 = { version = "0.13.2", features = ["pem", "pkcs8", "std"] } pathdiff = { version = "0.2.3", features = ["camino"] } pem = "3.0.5" @@ -67,6 +68,7 @@ shellwords = "1.1.0" slog = "2.7.0" snafu = "0.8.5" strum = { version = "0.27", features = ["derive"] } +sysinfo = "0.37.2" tempfile = "3" thiserror = "2.0.16" time = "0.3.9" diff --git a/crates/icp-cli/Cargo.toml b/crates/icp-cli/Cargo.toml index 147dccff..5afdc4e5 100644 --- a/crates/icp-cli/Cargo.toml +++ b/crates/icp-cli/Cargo.toml @@ -36,6 +36,7 @@ indicatif.workspace = true itertools.workspace = true k256.workspace = true lazy_static.workspace = true +nix.workspace = true pem.workspace = true phf.workspace = true pkcs8.workspace = true @@ -45,6 +46,7 @@ sec1.workspace = true serde_json.workspace = true serde.workspace = true snafu.workspace = true +sysinfo.workspace = true thiserror.workspace = true tiny-bip39.workspace = true tokio.workspace = true diff --git a/crates/icp-cli/src/commands/network/mod.rs b/crates/icp-cli/src/commands/network/mod.rs index 0372949a..6ab464b0 100644 --- a/crates/icp-cli/src/commands/network/mod.rs +++ b/crates/icp-cli/src/commands/network/mod.rs @@ -6,6 +6,7 @@ use crate::commands::Context; mod list; mod ping; mod run; +mod stop; #[derive(Parser, Debug)] pub struct NetworkCmd { @@ -18,6 +19,7 @@ pub enum NetworkSubcmd { List(list::Cmd), Ping(ping::Cmd), Run(run::Cmd), + Stop(stop::Cmd), } pub async fn dispatch(ctx: &Context, cmd: NetworkCmd) -> Result<(), NetworkCommandError> { @@ -25,6 +27,7 @@ pub async fn dispatch(ctx: &Context, cmd: NetworkCmd) -> Result<(), NetworkComma NetworkSubcmd::List(cmd) => list::exec(ctx, cmd).await?, NetworkSubcmd::Ping(cmd) => ping::exec(ctx, cmd).await?, NetworkSubcmd::Run(cmd) => run::exec(ctx, cmd).await?, + NetworkSubcmd::Stop(cmd) => stop::exec(ctx, cmd).await?, } Ok(()) @@ -40,4 +43,7 @@ pub enum NetworkCommandError { #[snafu(transparent)] Run { source: run::CommandError }, + + #[snafu(transparent)] + Stop { source: stop::CommandError }, } diff --git a/crates/icp-cli/src/commands/network/stop.rs b/crates/icp-cli/src/commands/network/stop.rs new file mode 100644 index 00000000..31608c14 --- /dev/null +++ b/crates/icp-cli/src/commands/network/stop.rs @@ -0,0 +1,121 @@ +use std::time::Duration; + +use clap::Parser; +use icp::{manifest, network::NetworkDirectory}; +use sysinfo::{Pid, ProcessesToUpdate, Signal, System}; + +use crate::commands::Context; + +/// Stop a background network +#[derive(Parser, Debug)] +pub struct Cmd { + /// Name of the network to stop + #[arg(default_value = "local")] + name: String, + + /// Maximum time to wait for the process to exit (in seconds) + #[arg(long, default_value = "30")] + timeout: u64, +} + +#[derive(Debug, thiserror::Error)] +pub enum CommandError { + #[error(transparent)] + Project(#[from] icp::LoadError), + + #[error(transparent)] + Locate(#[from] manifest::LocateError), + + #[error("project does not contain a network named '{name}'")] + Network { name: String }, + + #[error("network '{name}' is not running in the background")] + NotRunning { name: String }, + + #[error(transparent)] + LoadPid(#[from] icp::network::LoadPidError), + + #[error("process {pid} did not exit within {timeout} seconds")] + Timeout { pid: Pid, timeout: u64 }, + + #[error("failed to remove PID file: {source}")] + RemovePidFile { source: std::io::Error }, +} + +pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { + // Load project + let p = ctx.project.load().await?; + + // Check network exists + p.networks.get(&cmd.name).ok_or(CommandError::Network { + name: cmd.name.clone(), + })?; + + // Project root + let pdir = ctx.workspace.locate()?; + + // Network root + let ndir = pdir.join(".icp").join("networks").join(&cmd.name); + + // Network directory + let nd = NetworkDirectory::new( + &cmd.name, // name + &ndir, // network_root + &ctx.dirs.port_descriptor(), // port_descriptor_dir + ); + + // Load PID from file + let pid = nd + .load_background_network_runner_pid()? + .ok_or(CommandError::NotRunning { + name: cmd.name.clone(), + })?; + + eprintln!("Stopping background network (PID: {})...", pid); + + // Send SIGINT to the process + send_sigint(pid); + + // Wait for process to exit + wait_for_process_exit(pid, cmd.timeout)?; + + // Remove PID file + let pid_file = nd.structure.background_network_runner_pid_file(); + std::fs::remove_file(&pid_file).map_err(|source| CommandError::RemovePidFile { source })?; + + eprintln!("Network stopped successfully"); + + Ok(()) +} + +fn send_sigint(pid: Pid) { + let mut system = System::new(); + system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); + if let Some(process) = system.process(pid) { + process.kill_with(Signal::Interrupt); + } +} + +fn wait_for_process_exit(pid: Pid, timeout_secs: u64) -> Result<(), CommandError> { + let start = std::time::Instant::now(); + let timeout = Duration::from_secs(timeout_secs); + let mut system = System::new(); + + loop { + // Check if process is still running + system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); + if system.process(pid).is_none() { + return Ok(()); + } + + if start.elapsed() > timeout { + return Err(CommandError::Timeout { + pid, + timeout: timeout_secs, + }); + } + + // Sleep briefly before checking again + std::thread::sleep(Duration::from_millis(100)); + } +} diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index bf54ca8a..10fd66cc 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -288,7 +288,7 @@ fn network_seeds_preexisting_identities_icp_and_cycles_balances() { } #[tokio::test] -async fn network_run_background() { +async fn network_run_and_stop_background() { let ctx = TestContext::new(); let project_dir = ctx.create_project_dir("icp"); @@ -297,7 +297,7 @@ async fn network_run_background() { write_string(&project_dir.join("icp.yaml"), NETWORK_RANDOM_PORT) .expect("failed to write project manifest"); - // start network in background and verify we can see child process output + // Start network in background and verify we can see child process output ctx.icp() .current_dir(&project_dir) .args(["network", "run", "my-network", "--background"]) @@ -320,7 +320,13 @@ async fn network_run_background() { pid_file_path ); - // Verify network can be pinged with agent.status() + let pid_contents = std::fs::read_to_string(&pid_file_path).expect("Failed to read PID file"); + let pid: u32 = pid_contents + .trim() + .parse() + .expect("PID file should contain a valid process ID"); + + // Verify network is healthy with agent.status() let agent = ic_agent::Agent::builder() .with_url(format!("http://127.0.0.1:{}", network.gateway_port)) .build() @@ -331,4 +337,34 @@ async fn network_run_background() { matches!(&status.replica_health_status, Some(health) if health == "healthy"), "Network should be healthy" ); + + // Stop the network + ctx.icp() + .current_dir(&project_dir) + .args(["network", "stop", "my-network"]) + .assert() + .success() + .stderr(contains(&format!( + "Stopping background network (PID: {})", + pid + ))) + .stderr(contains("Network stopped successfully")); + + // Verify PID file is removed + assert!( + !pid_file_path.exists(), + "PID file should be removed after stopping" + ); + + // Verify process is no longer running + #[cfg(unix)] + { + use nix::sys::signal::kill; + use nix::unistd::Pid; + let nix_pid = Pid::from_raw(pid as i32); + assert!( + kill(nix_pid, None).is_err(), + "Process should no longer be running" + ); + } } diff --git a/crates/icp/Cargo.toml b/crates/icp/Cargo.toml index c82b5f6d..6332dca1 100644 --- a/crates/icp/Cargo.toml +++ b/crates/icp/Cargo.toml @@ -46,6 +46,7 @@ shellwords = { workspace = true } slog = { workspace = true } snafu = { workspace = true } strum = { workspace = true } +sysinfo = { workspace = true } thiserror = { workspace = true } time = { workspace = true } tiny-bip39 = { workspace = true } diff --git a/crates/icp/src/network/directory.rs b/crates/icp/src/network/directory.rs index 3fc38f93..f143be6e 100644 --- a/crates/icp/src/network/directory.rs +++ b/crates/icp/src/network/directory.rs @@ -1,6 +1,7 @@ use std::io::{ErrorKind, Seek, Write}; use snafu::prelude::*; +use sysinfo::Pid; use crate::{ fs::{create_dir_all, json, read_to_string}, @@ -173,11 +174,11 @@ impl NetworkDirectory { Ok(()) } - pub fn load_background_network_runner_pid(&self) -> Result, LoadPidError> { + pub fn load_background_network_runner_pid(&self) -> Result, LoadPidError> { let path = self.structure.background_network_runner_pid_file(); read_to_string(&path) - .map(|content| content.trim().parse::().ok()) + .map(|content| content.trim().parse::().ok()) .or_else(|err| match err.kind() { ErrorKind::NotFound => Ok(None), _ => Err(err).context(ReadPidSnafu { path: path.clone() }), From 9b2c830c41ba2d0d9007ce5611d412124187f4c8 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 15:10:26 +0200 Subject: [PATCH 08/18] wip --- crates/icp-cli/src/commands/network/run.rs | 12 ++++++++---- crates/icp-cli/src/commands/network/stop.rs | 13 ++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index f14c1847..b79560b7 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -15,6 +15,7 @@ use icp::{ run_network, }, }; +use tracing::debug; use crate::commands::Context; @@ -108,8 +109,8 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { // Determine ICP accounts to seed let seed_accounts = ids.identities.values().map(|id| id.principal()); - eprintln!("Project root: {pdir}"); - eprintln!("Network root: {ndir}"); + debug!("Project root: {pdir}"); + debug!("Network root: {ndir}"); if cmd.background { let mut child = run_in_background()?; @@ -157,28 +158,31 @@ fn run_in_background() -> Result { } async fn relay_child_output_until_healthy( + ctx: &Context, child: &mut Child, nd: &NetworkDirectory, ) -> Result<(), CommandError> { // Take stdout and stderr from the child let stdout = child.stdout.take().expect("Failed to take child stdout"); let stderr = child.stderr.take().expect("Failed to take child stderr"); + let term = ctx.term.clone(); // Spawn threads to relay output let stdout_thread = thread::spawn(move || { let reader = BufReader::new(stdout); for line in reader.lines() { if let Ok(line) = line { - println!("{}", line); // stdout -> stdout + let _ = term.write_line(&line); // stdout -> stdout } } }); + let term = ctx.term.clone(); let stderr_thread = thread::spawn(move || { let reader = BufReader::new(stderr); for line in reader.lines() { if let Ok(line) = line { - eprintln!("{}", line); // stderr -> stderr + let _ = term.write_line(&line); // stderr -> stderr } } }); diff --git a/crates/icp-cli/src/commands/network/stop.rs b/crates/icp-cli/src/commands/network/stop.rs index 31608c14..df75eee7 100644 --- a/crates/icp-cli/src/commands/network/stop.rs +++ b/crates/icp-cli/src/commands/network/stop.rs @@ -1,7 +1,7 @@ use std::time::Duration; use clap::Parser; -use icp::{manifest, network::NetworkDirectory}; +use icp::{fs::remove_file, manifest, network::NetworkDirectory}; use sysinfo::{Pid, ProcessesToUpdate, Signal, System}; use crate::commands::Context; @@ -37,9 +37,6 @@ pub enum CommandError { #[error("process {pid} did not exit within {timeout} seconds")] Timeout { pid: Pid, timeout: u64 }, - - #[error("failed to remove PID file: {source}")] - RemovePidFile { source: std::io::Error }, } pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { @@ -71,7 +68,9 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { name: cmd.name.clone(), })?; - eprintln!("Stopping background network (PID: {})...", pid); + let _ = ctx + .term + .write_line(&format!("Stopping background network (PID: {})...", pid)); // Send SIGINT to the process send_sigint(pid); @@ -81,9 +80,9 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { // Remove PID file let pid_file = nd.structure.background_network_runner_pid_file(); - std::fs::remove_file(&pid_file).map_err(|source| CommandError::RemovePidFile { source })?; + let _ = remove_file(&pid_file); // Cleanup is nice, but optional - eprintln!("Network stopped successfully"); + let _ = ctx.term.write_line("Network stopped successfully"); Ok(()) } From ad3afe2ada00c56d072edaeed804db8282778019 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 15:32:21 +0200 Subject: [PATCH 09/18] improve output printing --- crates/icp-cli/src/commands/network/run.rs | 32 ++++++++++++++++------ crates/icp-cli/tests/network_tests.rs | 7 ++--- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index b79560b7..36126d03 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -1,6 +1,10 @@ use std::{ io::{BufRead, BufReader}, process::{Child, Command, Stdio}, + sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + }, thread, time::Duration, }; @@ -117,7 +121,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { nd.save_background_network_runner_pid(child.id())?; // Relay stdout/stderr from child to parent until network is healthy - relay_child_output_until_healthy(&mut child, &nd).await?; + relay_child_output_until_healthy(ctx, &mut child, &nd).await?; // Explicitly forget the child handle so parent doesn't wait for it std::mem::forget(child); @@ -162,36 +166,48 @@ async fn relay_child_output_until_healthy( child: &mut Child, nd: &NetworkDirectory, ) -> Result<(), CommandError> { - // Take stdout and stderr from the child let stdout = child.stdout.take().expect("Failed to take child stdout"); let stderr = child.stderr.take().expect("Failed to take child stderr"); - let term = ctx.term.clone(); + + // Use atomic bool to signal threads to stop + let should_stop = Arc::new(AtomicBool::new(false)); // Spawn threads to relay output + let term = ctx.term.clone(); + let should_stop_clone = Arc::clone(&should_stop); let stdout_thread = thread::spawn(move || { let reader = BufReader::new(stdout); for line in reader.lines() { + if should_stop_clone.load(Ordering::Relaxed) { + break; + } if let Ok(line) = line { - let _ = term.write_line(&line); // stdout -> stdout + let _ = term.write_line(&line); } } }); let term = ctx.term.clone(); + let should_stop_clone = Arc::clone(&should_stop); let stderr_thread = thread::spawn(move || { let reader = BufReader::new(stderr); for line in reader.lines() { + if should_stop_clone.load(Ordering::Relaxed) { + break; + } if let Ok(line) = line { - let _ = term.write_line(&line); // stderr -> stderr + let _ = term.write_line(&line); } } }); - // Wait for network to become healthy wait_for_healthy_network(nd).await?; - // Note: We don't join the threads - they'll continue running until the pipes close - // or we can just let them be orphaned since the parent is about to exit + // Signal threads to stop + should_stop.store(true, Ordering::Relaxed); + + // Don't join the threads - they're likely blocked on I/O waiting for the next line. + // They'll terminate naturally when the parent process exits and the pipes close, or when the next line arrives. drop(stdout_thread); drop(stderr_thread); diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 10fd66cc..3be4bbfd 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -303,8 +303,7 @@ async fn network_run_and_stop_background() { .args(["network", "run", "my-network", "--background"]) .assert() .success() - .stderr(contains("Project root:")) - .stderr(contains("Network root:")); + .stdout(contains("Created instance with id")); // part of network start output let network = ctx.wait_for_network_descriptor(&project_dir, "my-network"); @@ -344,11 +343,11 @@ async fn network_run_and_stop_background() { .args(["network", "stop", "my-network"]) .assert() .success() - .stderr(contains(&format!( + .stdout(contains(&format!( "Stopping background network (PID: {})", pid ))) - .stderr(contains("Network stopped successfully")); + .stdout(contains("Network stopped successfully")); // Verify PID file is removed assert!( From 46e83f19792bff19645684a7a312dbf9785e0f7a Mon Sep 17 00:00:00 2001 From: Vivienne Date: Thu, 16 Oct 2025 15:34:18 +0200 Subject: [PATCH 10/18] no need to explicitly forget a child - drop takes care of it --- crates/icp-cli/src/commands/network/run.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 36126d03..bea5fc61 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -122,9 +122,6 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { // Relay stdout/stderr from child to parent until network is healthy relay_child_output_until_healthy(ctx, &mut child, &nd).await?; - - // Explicitly forget the child handle so parent doesn't wait for it - std::mem::forget(child); } else { run_network( cfg, // config From 2005606d3ab403696bc714b78b6dfdf6d49b3cd2 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Fri, 17 Oct 2025 09:58:38 +0200 Subject: [PATCH 11/18] cleanup --- crates/icp-cli/src/commands/network/run.rs | 8 -------- crates/icp-cli/src/commands/network/stop.rs | 14 ++++++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index bea5fc61..504e3691 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -23,8 +23,6 @@ use tracing::debug; use crate::commands::Context; -const BACKGROUND_ENV_VAR: &str = "ICP_CLI_RUN_NETWORK_BACKGROUND"; - /// Run a given network #[derive(Parser, Debug)] pub struct Cmd { @@ -35,10 +33,6 @@ pub struct Cmd { /// Starts the network in a background process. This command will exit once the network is running. #[arg(long)] background: bool, - - /// Set if this is the process that runs the network in the background - #[arg(long, env = BACKGROUND_ENV_VAR, hide = true)] - run_in_background: bool, } #[derive(Debug, thiserror::Error)] @@ -120,7 +114,6 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { let mut child = run_in_background()?; nd.save_background_network_runner_pid(child.id())?; - // Relay stdout/stderr from child to parent until network is healthy relay_child_output_until_healthy(ctx, &mut child, &nd).await?; } else { run_network( @@ -141,7 +134,6 @@ fn run_in_background() -> Result { let mut cmd = Command::new(exe); // Skip 1 because arg0 is this executable's path. cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) - .env(BACKGROUND_ENV_VAR, "true") // Set the environment variable which will be used by the second start. .stdin(Stdio::null()) // Redirect stdin from /dev/null .stdout(Stdio::piped()) // Capture stdout so parent can relay it .stderr(Stdio::piped()); // Capture stderr so parent can relay it diff --git a/crates/icp-cli/src/commands/network/stop.rs b/crates/icp-cli/src/commands/network/stop.rs index df75eee7..b4394179 100644 --- a/crates/icp-cli/src/commands/network/stop.rs +++ b/crates/icp-cli/src/commands/network/stop.rs @@ -6,16 +6,14 @@ use sysinfo::{Pid, ProcessesToUpdate, Signal, System}; use crate::commands::Context; +const TIMEOUT_SECS: u64 = 30; + /// Stop a background network #[derive(Parser, Debug)] pub struct Cmd { /// Name of the network to stop #[arg(default_value = "local")] name: String, - - /// Maximum time to wait for the process to exit (in seconds) - #[arg(long, default_value = "30")] - timeout: u64, } #[derive(Debug, thiserror::Error)] @@ -76,7 +74,7 @@ pub async fn exec(ctx: &Context, cmd: Cmd) -> Result<(), CommandError> { send_sigint(pid); // Wait for process to exit - wait_for_process_exit(pid, cmd.timeout)?; + wait_for_process_exit(pid)?; // Remove PID file let pid_file = nd.structure.background_network_runner_pid_file(); @@ -95,9 +93,9 @@ fn send_sigint(pid: Pid) { } } -fn wait_for_process_exit(pid: Pid, timeout_secs: u64) -> Result<(), CommandError> { +fn wait_for_process_exit(pid: Pid) -> Result<(), CommandError> { let start = std::time::Instant::now(); - let timeout = Duration::from_secs(timeout_secs); + let timeout = Duration::from_secs(TIMEOUT_SECS); let mut system = System::new(); loop { @@ -110,7 +108,7 @@ fn wait_for_process_exit(pid: Pid, timeout_secs: u64) -> Result<(), CommandError if start.elapsed() > timeout { return Err(CommandError::Timeout { pid, - timeout: timeout_secs, + timeout: TIMEOUT_SECS, }); } From 35e675c9897129d03246a4998c63a870c654dbdf Mon Sep 17 00:00:00 2001 From: Vivienne Date: Fri, 17 Oct 2025 10:45:44 +0200 Subject: [PATCH 12/18] clippy --- crates/icp-cli/src/commands/network/run.rs | 24 ++++++++++------------ crates/icp-cli/tests/network_tests.rs | 2 +- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index b1452247..9bc7fe68 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -122,7 +122,7 @@ pub async fn exec(ctx: &Context, args: &RunArgs) -> Result<(), CommandError> { run_network( cfg, // config nd, // nd - &pdir, // project_root + pdir, // project_root seed_accounts, // seed_accounts ) .await?; @@ -185,6 +185,7 @@ async fn relay_child_output_until_healthy( Ok(()) } +#[allow(clippy::result_large_err)] fn run_in_background() -> Result { // Background strategy: spawn a child process with piped stdout/stderr // so the parent can relay output until the network is healthy, then detach. @@ -240,12 +241,12 @@ async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandEr let agent = agent.clone(); async move { let status = agent.status().await; - if let Ok(status) = status { - if matches!(&status.replica_health_status, Some(status) if status == "healthy") - { - return Some(()); - } + if let Ok(status) = status + && matches!(&status.replica_health_status, Some(status) if status == "healthy") + { + return Some(()); } + None } }, @@ -268,14 +269,11 @@ async fn wait_for_network_descriptor( let max_retries = 30; let delay_ms = 1000; let result: Option = retry_with_timeout( - || { - let nd = nd; - async move { - if let Ok(Some(descriptor)) = nd.load_network_descriptor() { - return Some(descriptor); - } - None + || async move { + if let Ok(Some(descriptor)) = nd.load_network_descriptor() { + return Some(descriptor); } + None }, max_retries, delay_ms, diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 3be4bbfd..045631fe 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -343,7 +343,7 @@ async fn network_run_and_stop_background() { .args(["network", "stop", "my-network"]) .assert() .success() - .stdout(contains(&format!( + .stdout(contains(format!( "Stopping background network (PID: {})", pid ))) From e4f7a90a95f946ebd17dc359ed6f018e4c4a8d5b Mon Sep 17 00:00:00 2001 From: Vivienne Date: Fri, 17 Oct 2025 10:53:05 +0200 Subject: [PATCH 13/18] docs --- docs/cli-reference.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 7608adc5..c92b1a8c 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -37,6 +37,7 @@ This document contains the help content for the `icp-cli` command-line program. * [`icp-cli network list`↴](#icp-cli-network-list) * [`icp-cli network ping`↴](#icp-cli-network-ping) * [`icp-cli network run`↴](#icp-cli-network-run) +* [`icp-cli network stop`↴](#icp-cli-network-stop) * [`icp-cli sync`↴](#icp-cli-sync) * [`icp-cli token`↴](#icp-cli-token) * [`icp-cli token balance`↴](#icp-cli-token-balance) @@ -526,6 +527,7 @@ Launch and manage local test networks * `list` — List networks in the project * `ping` — Try to connect to a network, and print out its status * `run` — Run a given network +* `stop` — Stop a background network @@ -559,7 +561,7 @@ Try to connect to a network, and print out its status Run a given network -**Usage:** `icp-cli network run [NAME]` +**Usage:** `icp-cli network run [OPTIONS] [NAME]` ###### **Arguments:** @@ -567,6 +569,24 @@ Run a given network Default value: `local` +###### **Options:** + +* `--background` — Starts the network in a background process. This command will exit once the network is running + + + +## `icp-cli network stop` + +Stop a background network + +**Usage:** `icp-cli network stop [NAME]` + +###### **Arguments:** + +* `` — Name of the network to stop + + Default value: `local` + ## `icp-cli sync` From 31ec91f40ea487e988654929a213cef8e4baada2 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Fri, 17 Oct 2025 11:35:44 +0200 Subject: [PATCH 14/18] cleanup --- Cargo.toml | 1 - crates/icp-cli/Cargo.toml | 1 - crates/icp-cli/src/commands/network/mod.rs | 8 +- crates/icp-cli/src/commands/network/run.rs | 85 ++++++++------------- crates/icp-cli/src/commands/network/stop.rs | 10 +-- crates/icp-cli/tests/network_tests.rs | 22 +++--- crates/icp/src/network/directory.rs | 3 +- 7 files changed, 47 insertions(+), 83 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 075c5858..ae1edc0d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,6 @@ itertools = "0.14.0" k256 = { version = "0.13.4", features = ["pem", "pkcs8", "std"] } lazy_static = "1.5.0" mockall = "0.13.1" -nix = { version = "0.30.1", features = ["process", "signal"] } p256 = { version = "0.13.2", features = ["pem", "pkcs8", "std"] } pathdiff = { version = "0.2.3", features = ["camino"] } pem = "3.0.5" diff --git a/crates/icp-cli/Cargo.toml b/crates/icp-cli/Cargo.toml index bae3fe61..d70d162d 100644 --- a/crates/icp-cli/Cargo.toml +++ b/crates/icp-cli/Cargo.toml @@ -36,7 +36,6 @@ indicatif.workspace = true itertools.workspace = true k256.workspace = true lazy_static.workspace = true -nix.workspace = true pem.workspace = true phf.workspace = true pkcs8.workspace = true diff --git a/crates/icp-cli/src/commands/network/mod.rs b/crates/icp-cli/src/commands/network/mod.rs index ed1242ee..29350cac 100644 --- a/crates/icp-cli/src/commands/network/mod.rs +++ b/crates/icp-cli/src/commands/network/mod.rs @@ -1,16 +1,10 @@ -use clap::{Parser, Subcommand}; +use clap::Subcommand; pub(crate) mod list; pub(crate) mod ping; pub(crate) mod run; pub(crate) mod stop; -#[derive(Parser, Debug)] -pub struct NetworkCmd { - #[command(subcommand)] - subcmd: NetworkSubcmd, -} - #[derive(Subcommand, Debug)] pub enum NetworkSubcmd { List(list::ListArgs), diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 9bc7fe68..92921938 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -14,11 +14,9 @@ use ic_agent::{Agent, AgentError}; use icp::{ identity::manifest::{LoadIdentityManifestError, load_identity_list}, manifest, - network::{ - Configuration, NetworkDirectory, RunNetworkError, config::NetworkDescriptorModel, - run_network, - }, + network::{Configuration, NetworkDirectory, RunNetworkError, run_network}, }; +use sysinfo::Pid; use tracing::debug; use crate::commands::{Context, Mode}; @@ -115,8 +113,7 @@ pub async fn exec(ctx: &Context, args: &RunArgs) -> Result<(), CommandError> { if args.background { let mut child = run_in_background()?; - nd.save_background_network_runner_pid(child.id())?; - + nd.save_background_network_runner_pid(Pid::from(child.id() as usize))?; relay_child_output_until_healthy(ctx, &mut child, &nd).await?; } else { run_network( @@ -140,12 +137,11 @@ async fn relay_child_output_until_healthy( let stdout = child.stdout.take().expect("Failed to take child stdout"); let stderr = child.stderr.take().expect("Failed to take child stderr"); - // Use atomic bool to signal threads to stop - let should_stop = Arc::new(AtomicBool::new(false)); + let stop_printing_child_output = Arc::new(AtomicBool::new(false)); // Spawn threads to relay output let term = ctx.term.clone(); - let should_stop_clone = Arc::clone(&should_stop); + let should_stop_clone = Arc::clone(&stop_printing_child_output); let stdout_thread = thread::spawn(move || { let reader = BufReader::new(stdout); for line in reader.lines() { @@ -159,7 +155,7 @@ async fn relay_child_output_until_healthy( }); let term = ctx.term.clone(); - let should_stop_clone = Arc::clone(&should_stop); + let should_stop_clone = Arc::clone(&stop_printing_child_output); let stderr_thread = thread::spawn(move || { let reader = BufReader::new(stderr); for line in reader.lines() { @@ -175,10 +171,10 @@ async fn relay_child_output_until_healthy( wait_for_healthy_network(nd).await?; // Signal threads to stop - should_stop.store(true, Ordering::Relaxed); + stop_printing_child_output.store(true, Ordering::Relaxed); // Don't join the threads - they're likely blocked on I/O waiting for the next line. - // They'll terminate naturally when the parent process exits and the pipes close, or when the next line arrives. + // They'll terminate naturally when the pipes close, or when the next line arrives. drop(stdout_thread); drop(stderr_thread); @@ -187,8 +183,6 @@ async fn relay_child_output_until_healthy( #[allow(clippy::result_large_err)] fn run_in_background() -> Result { - // Background strategy: spawn a child process with piped stdout/stderr - // so the parent can relay output until the network is healthy, then detach. let exe = std::env::current_exe().expect("Failed to get current executable."); let mut cmd = Command::new(exe); // Skip 1 because arg0 is this executable's path. @@ -228,15 +222,31 @@ where } async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandError> { - let network = wait_for_network_descriptor(nd).await?; + let max_retries = 30; + let delay_ms = 1000; + + // Wait for network descriptor to be written + let network = retry_with_timeout( + || async move { + if let Ok(Some(descriptor)) = nd.load_network_descriptor() { + return Some(descriptor); + } + None + }, + max_retries, + delay_ms, + ) + .await + .ok_or(CommandError::Timeout { + err: "timed out waiting for network descriptor".to_string(), + })?; + + // Wait for network to report itself healthy let port = network.gateway.port; let agent = Agent::builder() .with_url(format!("http://127.0.0.1:{port}")) .build()?; - - let max_retries = 30; - let delay_ms = 1000; - let result: Option<()> = retry_with_timeout( + retry_with_timeout( || { let agent = agent.clone(); async move { @@ -253,37 +263,8 @@ async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandEr max_retries, delay_ms, ) - .await; - - match result { - Some(()) => Ok(()), - None => Err(CommandError::Timeout { - err: "timed out waiting for network to start".to_string(), - }), - } -} - -async fn wait_for_network_descriptor( - nd: &NetworkDirectory, -) -> Result { - let max_retries = 30; - let delay_ms = 1000; - let result: Option = retry_with_timeout( - || async move { - if let Ok(Some(descriptor)) = nd.load_network_descriptor() { - return Some(descriptor); - } - None - }, - max_retries, - delay_ms, - ) - .await; - - match result { - Some(descriptor) => Ok(descriptor), - None => Err(CommandError::Timeout { - err: "timed out waiting for network descriptor".to_string(), - }), - } + .await + .ok_or(CommandError::Timeout { + err: "timed out waiting for network to start".to_string(), + }) } diff --git a/crates/icp-cli/src/commands/network/stop.rs b/crates/icp-cli/src/commands/network/stop.rs index aafabc0a..bbfbb42f 100644 --- a/crates/icp-cli/src/commands/network/stop.rs +++ b/crates/icp-cli/src/commands/network/stop.rs @@ -53,12 +53,12 @@ pub async fn exec(ctx: &Context, cmd: &Cmd) -> Result<(), CommandError> { })?; // Network root - let ndir = pdir.join(".icp").join("networks").join(&cmd.name); + let nroot = pdir.join(".icp").join("networks").join(&cmd.name); // Network directory let nd = NetworkDirectory::new( &cmd.name, // name - &ndir, // network_root + &nroot, // network_root &ctx.dirs.port_descriptor(), // port_descriptor_dir ); @@ -73,13 +73,9 @@ pub async fn exec(ctx: &Context, cmd: &Cmd) -> Result<(), CommandError> { .term .write_line(&format!("Stopping background network (PID: {})...", pid)); - // Send SIGINT to the process send_sigint(pid); - - // Wait for process to exit wait_for_process_exit(pid)?; - // Remove PID file let pid_file = nd.structure.background_network_runner_pid_file(); let _ = remove_file(&pid_file); // Cleanup is nice, but optional @@ -104,7 +100,6 @@ fn wait_for_process_exit(pid: Pid) -> Result<(), CommandError> { let mut system = System::new(); loop { - // Check if process is still running system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); if system.process(pid).is_none() { return Ok(()); @@ -117,7 +112,6 @@ fn wait_for_process_exit(pid: Pid) -> Result<(), CommandError> { }); } - // Sleep briefly before checking again std::thread::sleep(Duration::from_millis(100)); } } diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 045631fe..be7908c3 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -4,6 +4,7 @@ use predicates::{ str::{PredicateStrExt, contains}, }; use serial_test::file_serial; +use sysinfo::{Pid, ProcessesToUpdate, System}; use crate::common::{ ENVIRONMENT_RANDOM_PORT, NETWORK_RANDOM_PORT, TestContext, TestNetwork, clients, @@ -320,7 +321,7 @@ async fn network_run_and_stop_background() { ); let pid_contents = std::fs::read_to_string(&pid_file_path).expect("Failed to read PID file"); - let pid: u32 = pid_contents + let pid: Pid = pid_contents .trim() .parse() .expect("PID file should contain a valid process ID"); @@ -355,15 +356,12 @@ async fn network_run_and_stop_background() { "PID file should be removed after stopping" ); - // Verify process is no longer running - #[cfg(unix)] - { - use nix::sys::signal::kill; - use nix::unistd::Pid; - let nix_pid = Pid::from_raw(pid as i32); - assert!( - kill(nix_pid, None).is_err(), - "Process should no longer be running" - ); - } + // Verify contrller process is no longer running + // We do not check that the PocketIC process is no longer running because it will take a while to shut down on its own. + let mut system = System::new(); + system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); + assert!( + system.process(pid).is_none(), + "Process should no longer be running" + ); } diff --git a/crates/icp/src/network/directory.rs b/crates/icp/src/network/directory.rs index f143be6e..a9c04ce1 100644 --- a/crates/icp/src/network/directory.rs +++ b/crates/icp/src/network/directory.rs @@ -151,7 +151,7 @@ impl NetworkDirectory { RwFileLock::open_for_write(self.structure.background_network_runner_pid_file()) } - pub fn save_background_network_runner_pid(&self, pid: u32) -> Result<(), SavePidError> { + pub fn save_background_network_runner_pid(&self, pid: Pid) -> Result<(), SavePidError> { let mut file_lock = self.open_background_runner_pid_file_for_writelock()?; let mut write_guard = file_lock.acquire_write_lock()?; @@ -159,7 +159,6 @@ impl NetworkDirectory { write_guard.set_len(0).context(TruncatePidFileSnafu { path: self.structure.background_network_runner_pid_file(), })?; - (*write_guard) .seek(std::io::SeekFrom::Start(0)) .context(TruncatePidFileSnafu { From 226ce126b86c81c72adbfc37f670c1859a9ed756 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Fri, 17 Oct 2025 11:45:28 +0200 Subject: [PATCH 15/18] cleanup --- crates/icp-cli/src/commands/mod.rs | 2 +- crates/icp-cli/src/commands/network/mod.rs | 2 +- crates/icp-cli/src/commands/network/run.rs | 8 ++++---- crates/icp-cli/src/main.rs | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/icp-cli/src/commands/mod.rs b/crates/icp-cli/src/commands/mod.rs index 7729140f..dafbb4c7 100644 --- a/crates/icp-cli/src/commands/mod.rs +++ b/crates/icp-cli/src/commands/mod.rs @@ -54,7 +54,7 @@ pub enum Command { /// Launch and manage local test networks #[command(subcommand)] - Network(network::NetworkSubcmd), + Network(network::Command), /// Display information about the current project #[clap(hide = true)] // TODO: figure out how to structure the commands later diff --git a/crates/icp-cli/src/commands/network/mod.rs b/crates/icp-cli/src/commands/network/mod.rs index 29350cac..a9cde15b 100644 --- a/crates/icp-cli/src/commands/network/mod.rs +++ b/crates/icp-cli/src/commands/network/mod.rs @@ -6,7 +6,7 @@ pub(crate) mod run; pub(crate) mod stop; #[derive(Subcommand, Debug)] -pub enum NetworkSubcmd { +pub enum Command { List(list::ListArgs), Ping(ping::PingArgs), Run(run::RunArgs), diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 92921938..720159f5 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -187,12 +187,12 @@ fn run_in_background() -> Result { let mut cmd = Command::new(exe); // Skip 1 because arg0 is this executable's path. cmd.args(std::env::args().skip(1).filter(|a| !a.eq("--background"))) - .stdin(Stdio::null()) // Redirect stdin from /dev/null - .stdout(Stdio::piped()) // Capture stdout so parent can relay it - .stderr(Stdio::piped()); // Capture stderr so parent can relay it + .stdin(Stdio::null()) + .stdout(Stdio::piped()) // Capture stdout so we can relay it + .stderr(Stdio::piped()); // Capture stderr so we can relay it // On Unix, create a new process group so the child can continue running - // independently after the parent exits + // independently after the run command exits #[cfg(unix)] { use std::os::unix::process::CommandExt; diff --git a/crates/icp-cli/src/main.rs b/crates/icp-cli/src/main.rs index 6c0d5239..295026bf 100644 --- a/crates/icp-cli/src/main.rs +++ b/crates/icp-cli/src/main.rs @@ -403,25 +403,25 @@ async fn main() -> Result<(), Error> { // Network Command::Network(cmd) => match cmd { - commands::network::NetworkSubcmd::List(args) => { + commands::network::Command::List(args) => { commands::network::list::exec(&ctx, &args) .instrument(trace_span) .await? } - commands::network::NetworkSubcmd::Ping(args) => { + commands::network::Command::Ping(args) => { commands::network::ping::exec(&ctx, &args) .instrument(trace_span) .await? } - commands::network::NetworkSubcmd::Run(args) => { + commands::network::Command::Run(args) => { commands::network::run::exec(&ctx, &args) .instrument(trace_span) .await? } - commands::network::NetworkSubcmd::Stop(args) => { + commands::network::Command::Stop(args) => { commands::network::stop::exec(&ctx, &args) .instrument(trace_span) .await? From 7a3849312da5c7530b9327407c8ef2d26e09ae12 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Mon, 20 Oct 2025 13:27:42 +0200 Subject: [PATCH 16/18] do not panic if writing to closed pipe --- crates/icp-cli/tests/network_tests.rs | 10 ++++++++-- crates/icp/src/network/managed/run.rs | 14 +++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index be7908c3..861f2309 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -356,12 +356,18 @@ async fn network_run_and_stop_background() { "PID file should be removed after stopping" ); - // Verify contrller process is no longer running - // We do not check that the PocketIC process is no longer running because it will take a while to shut down on its own. + // Verify controller process is no longer running let mut system = System::new(); system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); assert!( system.process(pid).is_none(), "Process should no longer be running" ); + + // Verify network is no longer reachable + let status_result = agent.status().await; + assert!( + status_result.is_err(), + "Network should not be reachable after stopping" + ); } diff --git a/crates/icp/src/network/managed/run.rs b/crates/icp/src/network/managed/run.rs index e9a25576..02f455e1 100644 --- a/crates/icp/src/network/managed/run.rs +++ b/crates/icp/src/network/managed/run.rs @@ -17,7 +17,7 @@ use pocket_ic::{ }; use reqwest::Url; use snafu::prelude::*; -use std::{env::var, fs::read_to_string, process::ExitStatus, time::Duration}; +use std::{env::var, fs::read_to_string, io::Write, process::ExitStatus, time::Duration}; use tokio::{process::Child, select, signal::ctrl_c, time::sleep}; use uuid::Uuid; @@ -177,19 +177,27 @@ pub enum RunPocketIcError { WaitForPort { source: WaitForPortError }, } +#[derive(Debug)] pub enum ShutdownReason { CtrlC, ChildExited, } +/// Write to stderr, ignoring any errors. This is safe to use even when stderr is closed +/// (e.g., in a background process after the parent exits), unlike eprintln! which panics. +fn safe_eprintln(msg: &str) { + let _ = std::io::stderr().write_all(msg.as_bytes()); + let _ = std::io::stderr().write_all(b"\n"); +} + async fn wait_for_shutdown(child: &mut Child) -> ShutdownReason { select!( _ = ctrl_c() => { - eprintln!("Received Ctrl-C, shutting down PocketIC..."); + safe_eprintln("Received Ctrl-C, shutting down PocketIC..."); ShutdownReason::CtrlC } res = notice_child_exit(child) => { - eprintln!("PocketIC exited with status: {:?}", res.status); + safe_eprintln(&format!("PocketIC exited with status: {:?}", res.status)); ShutdownReason::ChildExited } ) From 30f5613c63c38dcf83e15518db89ca6d68c4701d Mon Sep 17 00:00:00 2001 From: Vivienne Date: Mon, 20 Oct 2025 14:47:07 +0200 Subject: [PATCH 17/18] cleanup --- crates/icp-cli/src/commands/network/run.rs | 8 ++------ crates/icp-cli/tests/network_tests.rs | 15 +++++++++------ crates/icp/src/network/structure.rs | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/crates/icp-cli/src/commands/network/run.rs b/crates/icp-cli/src/commands/network/run.rs index 720159f5..556cb14a 100644 --- a/crates/icp-cli/src/commands/network/run.rs +++ b/crates/icp-cli/src/commands/network/run.rs @@ -29,6 +29,7 @@ pub struct RunArgs { name: String, /// Starts the network in a background process. This command will exit once the network is running. + /// To stop the network, use 'icp network stop'. #[arg(long)] background: bool, } @@ -227,12 +228,7 @@ async fn wait_for_healthy_network(nd: &NetworkDirectory) -> Result<(), CommandEr // Wait for network descriptor to be written let network = retry_with_timeout( - || async move { - if let Ok(Some(descriptor)) = nd.load_network_descriptor() { - return Some(descriptor); - } - None - }, + || async move { nd.load_network_descriptor().unwrap_or(None) }, max_retries, delay_ms, ) diff --git a/crates/icp-cli/tests/network_tests.rs b/crates/icp-cli/tests/network_tests.rs index 22cbf98d..5ef34877 100644 --- a/crates/icp-cli/tests/network_tests.rs +++ b/crates/icp-cli/tests/network_tests.rs @@ -15,7 +15,10 @@ use sysinfo::{Pid, ProcessesToUpdate, System}; use crate::common::{ ENVIRONMENT_RANDOM_PORT, NETWORK_RANDOM_PORT, TestContext, TestNetwork, clients, }; -use icp::{fs::write_string, prelude::*}; +use icp::{ + fs::{read_to_string, write_string}, + prelude::*, +}; mod common; @@ -326,8 +329,8 @@ async fn network_run_and_stop_background() { pid_file_path ); - let pid_contents = std::fs::read_to_string(&pid_file_path).expect("Failed to read PID file"); - let pid: Pid = pid_contents + let pid_contents = read_to_string(&pid_file_path).expect("Failed to read PID file"); + let background_controller_pid: Pid = pid_contents .trim() .parse() .expect("PID file should contain a valid process ID"); @@ -352,7 +355,7 @@ async fn network_run_and_stop_background() { .success() .stdout(contains(format!( "Stopping background network (PID: {})", - pid + background_controller_pid ))) .stdout(contains("Network stopped successfully")); @@ -364,9 +367,9 @@ async fn network_run_and_stop_background() { // Verify controller process is no longer running let mut system = System::new(); - system.refresh_processes(ProcessesToUpdate::Some(&[pid]), true); + system.refresh_processes(ProcessesToUpdate::Some(&[background_controller_pid]), true); assert!( - system.process(pid).is_none(), + system.process(background_controller_pid).is_none(), "Process should no longer be running" ); diff --git a/crates/icp/src/network/structure.rs b/crates/icp/src/network/structure.rs index 923c3df5..bd78122b 100644 --- a/crates/icp/src/network/structure.rs +++ b/crates/icp/src/network/structure.rs @@ -47,8 +47,8 @@ impl NetworkDirectoryStructure { self.pocketic_dir().join("port") } - /// When running a network in the background, we store the PID of the background controlling process here. - /// Once that process receives a SIGINT, it will shut down the network. + /// When running a network in the background, we store the PID of the background controlling `icp` process here. + /// This does _not_ contain pocket-ic processess. pub fn background_network_runner_pid_file(&self) -> PathBuf { self.network_root.join("background_network_runner.pid") } From fac139d558e1a5a5baa6cf61b946760c53a315a3 Mon Sep 17 00:00:00 2001 From: Vivienne Date: Mon, 20 Oct 2025 14:58:13 +0200 Subject: [PATCH 18/18] docs --- docs/cli-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index c92b1a8c..6f1cf348 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -571,7 +571,7 @@ Run a given network ###### **Options:** -* `--background` — Starts the network in a background process. This command will exit once the network is running +* `--background` — Starts the network in a background process. This command will exit once the network is running. To stop the network, use 'icp network stop'