fix: do not delete custom bin on add cmd

When the `--path` argument is specified on the node manager's `add` command, a custom binary is
supplied, and now, the source of `--path` is not deleted.

By default, the `add` command downloads the latest version of a binary to a temporary location, and
that binary is then copied to the service location. Having finished with it, the node manager then
cleans up the temporary binary. When the `--path` argument is used, it goes through the same code
path, and hence the source of `--path` was being deleted as a side effect. Now, a flag is used to
indicate whether the binary should be deleted, and it is set to `false` when the `--path` argument
is supplied.

The `--path` argument is being used when users build their own `safenode`, so they are expecting
that the built binary will still exist.

sn_node_manager/src/add_services/config.rs

@@ -108,6 +108,7 @@ impl InstallNodeServiceCtxBuilder {
 pub struct AddNodeServiceOptions {
     pub bootstrap_peers: Vec<Multiaddr>,
     pub count: Option<u16>,
+    pub delete_safenode_src: bool,
     pub env_variables: Option<Vec<(String, String)>>,
     pub genesis: bool,
     pub local: bool,

sn_node_manager/src/add_services/mod.rs

@@ -198,7 +198,9 @@ pub async fn add_node(
         rpc_port = increment_port_option(rpc_port);
     }
 
-    std::fs::remove_file(options.safenode_src_path)?;
+    if options.delete_safenode_src {
+        std::fs::remove_file(options.safenode_src_path)?;
+    }
 
     if !added_service_data.is_empty() {
         println!("Services Added:");

sn_node_manager/src/add_services/tests.rs

@@ -133,6 +133,7 @@ async fn add_genesis_node_should_use_latest_version_and_add_one_service() -> Res
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: None,
+            delete_safenode_src: true,
             env_variables: None,
             genesis: true,
             local: true,
@@ -229,6 +230,7 @@ async fn add_genesis_node_should_return_an_error_if_there_is_already_a_genesis_n
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: None,
+            delete_safenode_src: true,
             env_variables: None,
             genesis: true,
             local: true,
@@ -286,6 +288,7 @@ async fn add_genesis_node_should_return_an_error_if_count_is_greater_than_1() ->
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(3),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: true,
             local: true,
@@ -442,6 +445,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(3),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -582,6 +586,7 @@ async fn add_node_should_update_the_bootstrap_peers_inside_node_registry() -> Re
         AddNodeServiceOptions {
             bootstrap_peers: new_peers.clone(),
             count: None,
+            delete_safenode_src: true,
             env_variables: None,
             local: false,
             genesis: false,
@@ -696,6 +701,7 @@ async fn add_node_should_update_the_environment_variables_inside_node_registry()
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: None,
+            delete_safenode_src: true,
             env_variables: env_variables.clone(),
             genesis: false,
             local: false,
@@ -820,6 +826,7 @@ async fn add_new_node_should_add_another_service() -> Result<()> {
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: None,
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -925,6 +932,7 @@ async fn add_node_should_use_custom_ports_for_one_service() -> Result<()> {
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: None,
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1136,6 +1144,7 @@ async fn add_node_should_use_a_custom_port_range() -> Result<()> {
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(3),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1190,6 +1199,7 @@ async fn add_node_should_return_an_error_if_port_and_node_count_do_not_match() -
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(2),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1250,6 +1260,7 @@ async fn add_node_should_return_an_error_if_multiple_services_are_specified_with
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(2),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1448,6 +1459,7 @@ async fn add_node_should_use_a_custom_port_range_for_metrics_server() -> Result<
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(3),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1620,6 +1632,7 @@ async fn add_node_should_use_a_custom_port_range_for_the_rpc_server() -> Result<
         AddNodeServiceOptions {
             bootstrap_peers: vec![],
             count: Some(3),
+            delete_safenode_src: true,
             env_variables: None,
             genesis: false,
             local: false,
@@ -1936,3 +1949,93 @@ async fn add_daemon_should_return_an_error_if_a_daemon_service_was_already_creat
 
     Ok(())
 }
+
+#[tokio::test]
+async fn add_node_should_not_delete_the_source_binary_if_path_arg_is_used() -> Result<()> {
+    let tmp_data_dir = assert_fs::TempDir::new()?;
+    let node_reg_path = tmp_data_dir.child("node_reg.json");
+
+    let mut mock_service_control = MockServiceControl::new();
+
+    let mut node_registry = NodeRegistry {
+        faucet: None,
+        save_path: node_reg_path.to_path_buf(),
+        nodes: vec![],
+        bootstrap_peers: vec![],
+        environment_variables: None,
+        daemon: None,
+    };
+
+    let latest_version = "0.96.4";
+    let temp_dir = assert_fs::TempDir::new()?;
+    let node_data_dir = temp_dir.child("data");
+    node_data_dir.create_dir_all()?;
+    let node_logs_dir = temp_dir.child("logs");
+    node_logs_dir.create_dir_all()?;
+    let safenode_download_path = temp_dir.child(SAFENODE_FILE_NAME);
+    safenode_download_path.write_binary(b"fake safenode bin")?;
+
+    let mut seq = Sequence::new();
+
+    // Expected calls for first installation
+    mock_service_control
+        .expect_get_available_port()
+        .times(1)
+        .returning(|| Ok(8081))
+        .in_sequence(&mut seq);
+
+    let install_ctx = InstallNodeServiceCtxBuilder {
+        bootstrap_peers: vec![],
+        data_dir_path: node_data_dir.to_path_buf().join("safenode1"),
+        env_variables: None,
+        genesis: false,
+        local: false,
+        log_dir_path: node_logs_dir.to_path_buf().join("safenode1"),
+        metrics_port: None,
+        name: "safenode1".to_string(),
+        node_port: None,
+        rpc_socket_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8081),
+        safenode_path: node_data_dir
+            .to_path_buf()
+            .join("safenode1")
+            .join(SAFENODE_FILE_NAME),
+        service_user: get_username(),
+    }
+    .build()?;
+
+    mock_service_control
+        .expect_install()
+        .times(1)
+        .with(eq(install_ctx))
+        .returning(|_| Ok(()))
+        .in_sequence(&mut seq);
+
+    add_node(
+        AddNodeServiceOptions {
+            bootstrap_peers: vec![],
+            count: Some(1),
+            delete_safenode_src: false,
+            env_variables: None,
+            genesis: false,
+            local: false,
+            metrics_port: None,
+            node_port: None,
+            rpc_address: None,
+            rpc_port: None,
+            safenode_dir_path: temp_dir.to_path_buf(),
+            safenode_src_path: safenode_download_path.to_path_buf(),
+            service_data_dir_path: node_data_dir.to_path_buf(),
+            service_log_dir_path: node_logs_dir.to_path_buf(),
+            user: get_username(),
+            version: latest_version.to_string(),
+        },
+        &mut node_registry,
+        &mock_service_control,
+        VerbosityLevel::Normal,
+    )
+    .await?;
+
+    safenode_download_path.assert(predicate::path::is_file());
+
+    Ok(())
+}

sn_node_manager/src/cmd/node.rs

@@ -73,7 +73,7 @@ pub async fn add(
     let mut node_registry = NodeRegistry::load(&config::get_node_registry_path()?)?;
     let release_repo = <dyn SafeReleaseRepoActions>::default_config();
 
-    let (safenode_src_path, version) = if let Some(path) = src_path {
+    let (safenode_src_path, version) = if let Some(path) = src_path.clone() {
         let version = get_bin_version(&path)?;
         (path, version)
     } else {
@@ -102,6 +102,7 @@ pub async fn add(
 
     let options = AddNodeServiceOptions {
         count,
+        delete_safenode_src: src_path.is_none(),
         env_variables,
         genesis: is_first,
         local,