fix(iroh): do not remove the rpc lockfile if an iroh node is already …

…running (#2013)

## Description

Previously, we were running `RpcStatus::clear`, every time we ran the
`iroh start` command (`start::run_with_command`).

This meant that even when we correctly exited the `iroh start` command
in with the error "iroh is already running", we would then remove the
rpc lock file, even though iroh was still running in a different
process.

Any subsequent calls to `iroh console` or any other iroh commands that
expected an iroh node to be running, would assume that a node was *no
longer running* because the rpc lock file was gone.

This PR adds a new concrete `AlreadyRunningError` that we return from
`start_node`. Before we run `RpcStatus::clear`, we check for the
`AlreadyRunningError`, and do not clear the lockfile if that error is
present.

closes #2002 

## Notes & open questions

**One big change in this PR**

Using the `iroh_data_root` function makes it impossible to test anything
at the `iroh::start` level, because the `IROH_DATA_DIR` env var makes
sure that nothing can be tested in isolation. Setting the `env var` in
one test, clobbers setting it in another.

I have a commit that adjusts the code to:
- run `iroh_data_dir()` once in `main.rs`
- cascades the `iroh_data_dir` down through the code from there
- remove any other references to calling `iroh_data_dir` anywhere else

This mostly effects `commands::console` and `commands::start`

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

iroh/src/commands.rs

@@ -1,10 +1,10 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::{bail, ensure, Context, Result};
 use clap::Parser;
 use tokio_util::task::LocalPoolHandle;
 
-use crate::config::{iroh_data_root, ConsoleEnv, NodeConfig};
+use crate::config::{ConsoleEnv, NodeConfig};
 
 use self::blob::{BlobAddOptions, BlobSource};
 use self::rpc::{RpcCommands, RpcStatus};
@@ -84,37 +84,39 @@ pub enum Commands {
 }
 
 impl Cli {
-    pub async fn run(self, rt: LocalPoolHandle) -> Result<()> {
+    pub async fn run(self, rt: LocalPoolHandle, data_dir: &Path) -> Result<()> {
         match self.command {
             Commands::Console => {
-                let env = ConsoleEnv::for_console()?;
+                let env = ConsoleEnv::for_console(data_dir)?;
                 if self.start {
                     let config = NodeConfig::from_env(self.config.as_deref())?;
                     start::run_with_command(
                         &rt,
                         &config,
+                        data_dir,
                         RunType::SingleCommandNoAbort,
                         |iroh| async move { console::run(&iroh, &env).await },
                     )
                     .await
                 } else {
-                    let iroh = iroh_quic_connect().await.context("rpc connect")?;
+                    let iroh = iroh_quic_connect(data_dir).await.context("rpc connect")?;
                     console::run(&iroh, &env).await
                 }
             }
             Commands::Rpc(command) => {
-                let env = ConsoleEnv::for_cli()?;
+                let env = ConsoleEnv::for_cli(data_dir)?;
                 if self.start {
                     let config = NodeConfig::from_env(self.config.as_deref())?;
                     start::run_with_command(
                         &rt,
                         &config,
+                        data_dir,
                         RunType::SingleCommandAbortable,
                         |iroh| async move { command.run(&iroh, &env).await },
                     )
                     .await
                 } else {
-                    let iroh = iroh_quic_connect().await.context("rpc connect")?;
+                    let iroh = iroh_quic_connect(data_dir).await.context("rpc connect")?;
                     command.run(&iroh, &env).await
                 }
             }
@@ -134,12 +136,18 @@ impl Cli {
                     options: add_options,
                 });
 
-                start::run_with_command(&rt, &config, RunType::UntilStopped, |client| async move {
-                    match add_command {
-                        None => Ok(()),
-                        Some(command) => command.run(&client).await,
-                    }
-                })
+                start::run_with_command(
+                    &rt,
+                    &config,
+                    data_dir,
+                    RunType::UntilStopped,
+                    |client| async move {
+                        match add_command {
+                            None => Ok(()),
+                            Some(command) => command.run(&client).await,
+                        }
+                    },
+                )
                 .await
             }
             Commands::Doctor { command } => {
@@ -150,8 +158,7 @@ impl Cli {
     }
 }
 
-async fn iroh_quic_connect() -> Result<iroh::client::quic::Iroh> {
-    let root = iroh_data_root()?;
+async fn iroh_quic_connect(root: &Path) -> Result<iroh::client::quic::Iroh> {
     let rpc_status = RpcStatus::load(root).await?;
     match rpc_status {
         RpcStatus::Stopped => {

iroh/src/commands/console.rs

@@ -53,7 +53,7 @@ impl Repl {
     pub fn run(self) -> anyhow::Result<()> {
         let mut rl =
             DefaultEditor::with_config(Config::builder().check_cursor_position(true).build())?;
-        let history_path = ConsolePaths::History.with_env()?;
+        let history_path = ConsolePaths::History.with_root(self.env.iroh_data_dir());
         rl.load_history(&history_path).ok();
         loop {
             // prepare a channel to receive a signal from the main thread when a command completed

iroh/src/commands/doc.rs

@@ -947,17 +947,15 @@ mod tests {
 
         let data_dir = tempfile::tempdir()?;
 
-        // run iroh node in the background, as if running `iroh start`
-        std::env::set_var("IROH_DATA_DIR", data_dir.path().as_os_str());
         let lp = tokio_util::task::LocalPoolHandle::new(1);
-        let node = crate::commands::start::start_node(&lp, None).await?;
+        let node = crate::commands::start::start_node(&lp, data_dir.path(), None).await?;
         let client = node.client();
         let doc = client.docs.create().await.context("doc create")?;
         let author = client.authors.create().await.context("author create")?;
 
         // set up command, getting iroh node
-        let cli = ConsoleEnv::for_console().context("ConsoleEnv")?;
-        let iroh = crate::commands::iroh_quic_connect()
+        let cli = ConsoleEnv::for_console(data_dir.path()).context("ConsoleEnv")?;
+        let iroh = crate::commands::iroh_quic_connect(data_dir.path())
             .await
             .context("rpc connect")?;
 

iroh/src/commands/doctor.rs

@@ -9,7 +9,7 @@ use std::{
     time::{Duration, Instant},
 };
 
-use crate::config::{path_with_env, NodeConfig};
+use crate::config::{iroh_data_root, NodeConfig};
 
 use anyhow::Context;
 use clap::Subcommand;
@@ -859,7 +859,7 @@ fn create_secret_key(secret_key: SecretKeyOption) -> anyhow::Result<SecretKey> {
             SecretKey::try_from(&bytes[..])?
         }
         SecretKeyOption::Local => {
-            let path = path_with_env(IrohPaths::SecretKey)?;
+            let path = IrohPaths::SecretKey.with_root(iroh_data_root()?);
             if path.exists() {
                 let bytes = std::fs::read(&path)?;
                 SecretKey::try_from_openssh(bytes)?