From 7c4d6249d9e69ea182a70be99d0bb28e412b51b1 Mon Sep 17 00:00:00 2001 From: Chris O'Neil Date: Fri, 1 Dec 2023 15:16:53 +0000 Subject: [PATCH] feat: provide `remove` command Removes an added node, which will delete its data and log directories. The node is marked as removed rather than actually being removed from the node registry just because the service number is incremented by one each time a new node is added, so it might be a little odd if numbers were missing in the sequence. --- Cargo.toml | 4 +- README.md | 26 ++++- src/add_service.rs | 32 +++--- src/control.rs | 269 +++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 75 +++++++++++-- src/node.rs | 8 +- src/service.rs | 11 +- tests/e2e.rs | 15 +++ 8 files changed, 384 insertions(+), 56 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 33670fa..4067b08 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,13 +16,13 @@ clap = { version = "4.4.6", features = ["derive", "env"]} colored = "2.0.4" color-eyre = "~0.6" indicatif = { version = "0.17.5", features = ["tokio"] } -libp2p = { version="0.53" , features = [] } +libp2p = { version = "0.53", features = [] } libp2p-identity = { version="0.2.7", features = ["rand"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" service-manager = "0.5.1" sn_node_rpc_client = "0.1.43" -sn_peers_acquisition = "0.1.10" +sn_peers_acquisition = { version = "0.1.10", features = ["network-contacts"] } sn-releases = "0.1.1" sysinfo = "0.29.10" tokio = { version = "1.26", features = ["full"] } diff --git a/README.md b/README.md index adfaa8e..7b4e1cf 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ The command can run as many times as you like to repeatedly add more nodes. ### Start - Command: `start` -- Description: Starts an installed `safenode` service. +- Description: Starts a `safenode` service. - Options: - `--peer-id`: Peer ID of the service to start. Optional. - `--service-name`: Name of the service to start. Optional. @@ -42,14 +42,14 @@ The command can run as many times as you like to repeatedly add more nodes. This command must run as the root user on Linux/macOS and the Administrator user on Windows. -Running the command with no arguments will start every installed node that is not already running. The peer ID or service name can be used to start a specific service. +Running the command with no arguments will start every node that is not already running. The peer ID or service name can be used to start a specific service. A peer ID will be assigned to a node after it is started for the first time. ### Status - Command: `status` -- Description: Displays the status of installed services. +- Description: Displays the status of `safenode` services. - Options: - `--details`: Displays more detailed information. Boolean flag. - Usage: `safenode-manager status [OPTIONS]` @@ -57,10 +57,10 @@ A peer ID will be assigned to a node after it is started for the first time. ### Stop - Command: `stop` -- Description: Stops an installed `safenode` service. +- Description: Stops a `safenode` service. - Options: - - `--peer_id`: Peer ID of the service to stop. Optional. - - `--service_name`: Name of the service to stop. Optional. + - `--peer-id`: Peer ID of the service to stop. Optional. + - `--service-name`: Name of the service to stop. Optional. - Usage: `safenode-manager stop [OPTIONS]` This command must run as the root user on Linux/macOS and the Administrator user on Windows. @@ -69,6 +69,20 @@ Running the command with no arguments will stop every installed node that is not If started again, the node's data and peer ID will be retained. +### Remove + +- Command: `remove` +- Description: Removes a `safenode` service. +- Options: + - `--peer-id`: Peer ID of the service to remove. Optional. + - `--service-name`: Name of the service to remove. Optional. + - `--keep-directories`: Set this flag to keep the node's data and log directories. Optional. +- Usage: `safenode-manager remove [OPTIONS]` + +This command must run as the root user on Linux/macOS and the Administrator user on Windows. + +Removes the node and its data/log directories. The node must be stopped before running this command. + ## License This Safe Network repository is licensed under the General Public License (GPL), version 3 ([LICENSE](LICENSE) http://www.gnu.org/licenses/gpl-3.0.en.html). diff --git a/src/add_service.rs b/src/add_service.rs index 6adeeb5..6f5b9b6 100644 --- a/src/add_service.rs +++ b/src/add_service.rs @@ -121,8 +121,8 @@ pub async fn add( status: NodeStatus::Added, pid: None, peer_id: None, - log_dir_path: service_log_dir_path.clone(), - data_dir_path: service_data_dir_path.clone(), + log_dir_path: Some(service_log_dir_path.clone()), + data_dir_path: Some(service_data_dir_path.clone()), }); node_number += 1; @@ -295,11 +295,11 @@ mod tests { assert_eq!(node_registry.nodes[0].rpc_port, 8081); assert_eq!( node_registry.nodes[0].log_dir_path, - node_logs_dir.to_path_buf().join("safenode1") + Some(node_logs_dir.to_path_buf().join("safenode1")) ); assert_eq!( node_registry.nodes[0].data_dir_path, - node_data_dir.to_path_buf().join("safenode1") + Some(node_data_dir.to_path_buf().join("safenode1")) ); assert_matches!(node_registry.nodes[0].status, NodeStatus::Added); @@ -471,11 +471,11 @@ mod tests { assert_eq!(node_registry.nodes[0].rpc_port, 8081); assert_eq!( node_registry.nodes[0].log_dir_path, - node_logs_dir.to_path_buf().join("safenode1") + Some(node_logs_dir.to_path_buf().join("safenode1")) ); assert_eq!( node_registry.nodes[0].data_dir_path, - node_data_dir.to_path_buf().join("safenode1") + Some(node_data_dir.to_path_buf().join("safenode1")) ); assert_matches!(node_registry.nodes[0].status, NodeStatus::Added); assert_eq!(node_registry.nodes[1].version, latest_version); @@ -486,11 +486,11 @@ mod tests { assert_eq!(node_registry.nodes[1].rpc_port, 8083); assert_eq!( node_registry.nodes[1].log_dir_path, - node_logs_dir.to_path_buf().join("safenode2") + Some(node_logs_dir.to_path_buf().join("safenode2")) ); assert_eq!( node_registry.nodes[1].data_dir_path, - node_data_dir.to_path_buf().join("safenode2") + Some(node_data_dir.to_path_buf().join("safenode2")) ); assert_matches!(node_registry.nodes[1].status, NodeStatus::Added); assert_eq!(node_registry.nodes[2].version, latest_version); @@ -501,11 +501,11 @@ mod tests { assert_eq!(node_registry.nodes[2].rpc_port, 8085); assert_eq!( node_registry.nodes[2].log_dir_path, - node_logs_dir.to_path_buf().join("safenode3") + Some(node_logs_dir.to_path_buf().join("safenode3")) ); assert_eq!( node_registry.nodes[2].data_dir_path, - node_data_dir.to_path_buf().join("safenode3") + Some(node_data_dir.to_path_buf().join("safenode3")) ); assert_matches!(node_registry.nodes[2].status, NodeStatus::Added); @@ -614,11 +614,11 @@ mod tests { assert_eq!(node_registry.nodes[0].rpc_port, 8081); assert_eq!( node_registry.nodes[0].log_dir_path, - node_logs_dir.to_path_buf().join("safenode1") + Some(node_logs_dir.to_path_buf().join("safenode1")) ); assert_eq!( node_registry.nodes[0].data_dir_path, - node_data_dir.to_path_buf().join("safenode1") + Some(node_data_dir.to_path_buf().join("safenode1")) ); assert_matches!(node_registry.nodes[0].status, NodeStatus::Added); @@ -642,8 +642,8 @@ mod tests { status: NodeStatus::Added, pid: None, peer_id: None, - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }], }; let temp_dir = assert_fs::TempDir::new()?; @@ -746,11 +746,11 @@ mod tests { assert_eq!(node_registry.nodes[1].rpc_port, 8083); assert_eq!( node_registry.nodes[1].log_dir_path, - node_logs_dir.to_path_buf().join("safenode2") + Some(node_logs_dir.to_path_buf().join("safenode2")) ); assert_eq!( node_registry.nodes[1].data_dir_path, - node_data_dir.to_path_buf().join("safenode2") + Some(node_data_dir.to_path_buf().join("safenode2")) ); assert_matches!(node_registry.nodes[0].status, NodeStatus::Added); diff --git a/src/control.rs b/src/control.rs index 364be70..10bf926 100644 --- a/src/control.rs +++ b/src/control.rs @@ -8,7 +8,7 @@ use crate::node::{Node, NodeRegistry, NodeStatus}; use crate::service::ServiceControl; -use color_eyre::{eyre::eyre, Result}; +use color_eyre::{eyre::eyre, Help, Result}; use colored::Colorize; use sn_node_rpc_client::RpcActions; @@ -53,6 +53,7 @@ pub async fn stop(node: &mut Node, service_control: &dyn ServiceControl) -> Resu "Service {} has not been started since it was installed", node.service_name )), + NodeStatus::Removed => Err(eyre!("Service {} has been removed", node.service_name)), NodeStatus::Running => { let pid = node.pid.unwrap(); if service_control.is_service_process_running(pid) { @@ -118,15 +119,28 @@ pub async fn status( "\tPID: {}", node.pid.map_or("-".to_string(), |p| p.to_string()) ); - println!("\tData path: {}", node.data_dir_path.to_string_lossy()); - println!("\tLog path: {}", node.log_dir_path.to_string_lossy()); + println!( + "\tData path: {}", + node.data_dir_path + .as_ref() + .map_or("-".to_string(), |p| p.to_string_lossy().to_string()) + ); + println!( + "\tLog path: {}", + node.log_dir_path + .as_ref() + .map_or("-".to_string(), |p| p.to_string_lossy().to_string()) + ); } } else { println!("{:<20} {:<52} Status", "Service Name", "Peer ID"); - for node in &node_registry.nodes { - let peer_id = node - .peer_id - .map_or_else(|| "-".to_string(), |p| p.to_string()); + let nodes = node_registry + .nodes + .iter() + .filter(|n| n.status != NodeStatus::Removed) + .collect::>(); + for node in nodes { + let peer_id = node.peer_id.map_or("-".to_string(), |p| p.to_string()); println!( "{:<20} {:<52} {}", node.service_name, @@ -139,11 +153,59 @@ pub async fn status( Ok(()) } +pub async fn remove( + node: &mut Node, + service_control: &dyn ServiceControl, + keep_directories: bool, +) -> Result<()> { + if let NodeStatus::Running = node.status { + if service_control.is_service_process_running( + node.pid + .ok_or_else(|| eyre!("The PID should be set before the node is removed"))?, + ) { + return Err(eyre!("A running node cannot be removed") + .suggestion("Stop the node first then try again")); + } else { + // If the node wasn't actually running, we should give the user an opportunity to + // check why it may have failed before removing everything. + node.pid = None; + node.status = NodeStatus::Stopped; + return Err( + eyre!("This node was marked as running but it had actually stopped") + .suggestion("You may want to check the logs for errors before removing it") + .suggestion("To remove the node, run the command again."), + ); + } + } + + service_control.uninstall(&node.service_name)?; + + if !keep_directories { + std::fs::remove_dir_all(node.data_dir_path.as_ref().ok_or_else(|| { + eyre!("The data directory should be set before the node is removed") + })?)?; + std::fs::remove_dir_all( + node.log_dir_path.as_ref().ok_or_else(|| { + eyre!("The log directory should be set before the node is removed") + })?, + )?; + node.data_dir_path = None; + node.log_dir_path = None; + } + + node.status = NodeStatus::Removed; + + println!("{} Service {} was removed", "✓".green(), node.service_name); + + Ok(()) +} + fn format_status(status: &NodeStatus) -> String { match status { NodeStatus::Running => "RUNNING".green().to_string(), NodeStatus::Stopped => "STOPPED".red().to_string(), NodeStatus::Added => "ADDED".yellow().to_string(), + NodeStatus::Removed => "REMOVED".red().to_string(), } } @@ -152,12 +214,14 @@ mod tests { use super::*; use crate::node::{Node, NodeStatus}; use crate::service::MockServiceControl; + use assert_fs::prelude::*; use assert_matches::assert_matches; use async_trait::async_trait; use libp2p_identity::PeerId; use mockall::mock; use mockall::predicate::*; use mockall::Sequence; + use predicates::prelude::*; use sn_node_rpc_client::{ NetworkInfo, NodeInfo, RecordAddress, Result as RpcResult, RpcActions, }; @@ -215,8 +279,8 @@ mod tests { status: NodeStatus::Added, pid: None, peer_id: None, - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; start(&mut node, &mock_service_control, &mock_rpc_client).await?; @@ -269,8 +333,8 @@ mod tests { peer_id: Some(PeerId::from_str( "12D3KooWAAqZWsjhdZTX7tniJ7Dwye3nEbp1dx1wE96sbgL51obs", )?), - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; start(&mut node, &mock_service_control, &mock_rpc_client).await?; @@ -323,8 +387,8 @@ mod tests { peer_id: Some(PeerId::from_str( "12D3KooWS2tpXGGTmg2AHFiDh57yPQnat49YHnyqoggzXZWpqkCR", )?), - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; start(&mut node, &mock_service_control, &mock_rpc_client).await?; @@ -368,8 +432,8 @@ mod tests { peer_id: Some(PeerId::from_str( "12D3KooWS2tpXGGTmg2AHFiDh57yPQnat49YHnyqoggzXZWpqkCR", )?), - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; start(&mut node, &mock_service_control, &mock_rpc_client).await?; @@ -406,8 +470,8 @@ mod tests { peer_id: Some(PeerId::from_str( "12D3KooWS2tpXGGTmg2AHFiDh57yPQnat49YHnyqoggzXZWpqkCR", )?), - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; stop(&mut node, &mock_service_control).await?; @@ -438,8 +502,8 @@ mod tests { status: NodeStatus::Added, pid: None, peer_id: None, - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; let result = stop(&mut node, &mock_service_control).await; @@ -472,8 +536,8 @@ mod tests { status: NodeStatus::Stopped, pid: None, peer_id: None, - log_dir_path: PathBuf::from("/var/log/safenode/safenode1"), - data_dir_path: PathBuf::from("/var/safenode-manager/services/safenode1"), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), }; stop(&mut node, &mock_service_control).await?; @@ -483,4 +547,167 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn remove_should_remove_an_added_node() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let log_dir = temp_dir.child("safenode1-logs"); + log_dir.create_dir_all()?; + let data_dir = temp_dir.child("safenode1-data"); + data_dir.create_dir_all()?; + + let mut mock_service_control = MockServiceControl::new(); + mock_service_control + .expect_uninstall() + .with(eq("safenode1")) + .times(1) + .returning(|_| Ok(())); + + let mut node = Node { + version: "0.98.1".to_string(), + service_name: "safenode1".to_string(), + user: "safe".to_string(), + number: 1, + port: 8080, + rpc_port: 8081, + status: NodeStatus::Stopped, + pid: None, + peer_id: None, + log_dir_path: Some(log_dir.to_path_buf()), + data_dir_path: Some(data_dir.to_path_buf()), + }; + + remove(&mut node, &mock_service_control, false).await?; + + assert_eq!(node.data_dir_path, None); + assert_eq!(node.log_dir_path, None); + assert_matches!(node.status, NodeStatus::Removed); + + log_dir.assert(predicate::path::missing()); + data_dir.assert(predicate::path::missing()); + + Ok(()) + } + + #[tokio::test] + async fn remove_should_return_an_error_if_attempting_to_remove_a_running_node() -> Result<()> { + let mut mock_service_control = MockServiceControl::new(); + mock_service_control + .expect_is_service_process_running() + .with(eq(1000)) + .times(1) + .returning(|_| true); + + let mut node = Node { + version: "0.98.1".to_string(), + service_name: "safenode1".to_string(), + user: "safe".to_string(), + number: 1, + port: 8080, + rpc_port: 8081, + status: NodeStatus::Running, + pid: Some(1000), + peer_id: Some(PeerId::from_str( + "12D3KooWS2tpXGGTmg2AHFiDh57yPQnat49YHnyqoggzXZWpqkCR", + )?), + log_dir_path: Some(PathBuf::from("/var/log/safenode/safenode1")), + data_dir_path: Some(PathBuf::from("/var/safenode-manager/services/safenode1")), + }; + + let result = remove(&mut node, &mock_service_control, false).await; + match result { + Ok(_) => panic!("This test should result in an error"), + Err(e) => assert_eq!("A running node cannot be removed", e.to_string()), + } + + Ok(()) + } + + #[tokio::test] + async fn remove_should_return_an_error_for_a_node_that_was_marked_running_but_was_not_actually_running( + ) -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let log_dir = temp_dir.child("safenode1-logs"); + log_dir.create_dir_all()?; + let data_dir = temp_dir.child("safenode1-data"); + data_dir.create_dir_all()?; + + let mut mock_service_control = MockServiceControl::new(); + mock_service_control + .expect_is_service_process_running() + .with(eq(1000)) + .times(1) + .returning(|_| false); + + let mut node = Node { + version: "0.98.1".to_string(), + service_name: "safenode1".to_string(), + user: "safe".to_string(), + number: 1, + port: 8080, + rpc_port: 8081, + status: NodeStatus::Running, + pid: Some(1000), + peer_id: Some(PeerId::from_str( + "12D3KooWS2tpXGGTmg2AHFiDh57yPQnat49YHnyqoggzXZWpqkCR", + )?), + log_dir_path: Some(log_dir.to_path_buf()), + data_dir_path: Some(data_dir.to_path_buf()), + }; + + let result = remove(&mut node, &mock_service_control, false).await; + match result { + Ok(_) => panic!("This test should result in an error"), + Err(e) => assert_eq!( + "This node was marked as running but it had actually stopped", + e.to_string() + ), + } + + assert_eq!(node.pid, None); + assert_matches!(node.status, NodeStatus::Stopped); + + Ok(()) + } + + #[tokio::test] + async fn remove_should_remove_an_added_node_and_keep_directories() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let log_dir = temp_dir.child("safenode1-logs"); + log_dir.create_dir_all()?; + let data_dir = temp_dir.child("safenode1-data"); + data_dir.create_dir_all()?; + + let mut mock_service_control = MockServiceControl::new(); + mock_service_control + .expect_uninstall() + .with(eq("safenode1")) + .times(1) + .returning(|_| Ok(())); + + let mut node = Node { + version: "0.98.1".to_string(), + service_name: "safenode1".to_string(), + user: "safe".to_string(), + number: 1, + port: 8080, + rpc_port: 8081, + status: NodeStatus::Stopped, + pid: None, + peer_id: None, + log_dir_path: Some(log_dir.to_path_buf()), + data_dir_path: Some(data_dir.to_path_buf()), + }; + + remove(&mut node, &mock_service_control, true).await?; + + assert_eq!(node.data_dir_path, Some(data_dir.to_path_buf())); + assert_eq!(node.log_dir_path, Some(log_dir.to_path_buf())); + assert_matches!(node.status, NodeStatus::Removed); + + log_dir.assert(predicate::path::is_dir()); + data_dir.assert(predicate::path::is_dir()); + + Ok(()) + } } diff --git a/src/main.rs b/src/main.rs index be65692..1d150f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,7 +14,7 @@ mod service; use crate::add_service::{add, AddServiceOptions}; use crate::config::{get_node_registry_path, get_service_data_dir_path, get_service_log_dir_path}; -use crate::control::{start, status, stop}; +use crate::control::{remove, start, status, stop}; use crate::node::NodeRegistry; use crate::service::{NodeServiceManager, ServiceControl}; use clap::{Parser, Subcommand}; @@ -77,9 +77,26 @@ pub enum SubCmd { #[clap(long)] version: Option, }, - /// Start an installed safenode service. + /// Remove a safenode service. /// - /// If no peer ID(s) or service name(s) are supplied, all installed services will be started. + /// Either a peer ID or the service name must be supplied. + /// + /// This command must run as the root/administrative user. + #[clap(name = "remove")] + Remove { + /// The peer ID of the service to remove. + #[clap(long)] + peer_id: Option, + /// The name of the service to remove. + #[clap(long)] + service_name: Option, + /// Set this flag to keep the nodes data and log directories. + #[clap(long)] + keep_directories: bool, + }, + /// Start a safenode service. + /// + /// If no peer ID or service name are supplied, all installed services will be started. /// /// This command must run as the root/administrative user. #[clap(name = "start")] @@ -91,16 +108,16 @@ pub enum SubCmd { #[clap(long)] service_name: Option, }, - /// Get the status of installed services. + /// Get the status of services. #[clap(name = "status")] Status { - /// Set to display more details + /// Set this flag to display more details #[clap(long)] details: bool, }, /// Stop an installed safenode service. /// - /// If no peer ID(s) or service name(s) are supplied, all installed services will be stopped. + /// If no peer ID or service name are supplied, all installed services will be stopped. /// /// This command must run as the root/administrative user. #[clap(name = "stop")] @@ -128,7 +145,7 @@ async fn main() -> Result<()> { version, } => { if !is_running_as_root() { - return Err(eyre!("The install command must run as the root user")); + return Err(eyre!("The add command must run as the root user")); } println!("================================================="); @@ -166,6 +183,50 @@ async fn main() -> Result<()> { Ok(()) } + SubCmd::Remove { + peer_id, + service_name, + keep_directories, + } => { + if !is_running_as_root() { + return Err(eyre!("The remove command must run as the root user")); + } + if peer_id.is_none() && service_name.is_none() { + return Err(eyre!("Either a peer ID or a service name must be supplied")); + } + validate_peer_id_and_service_name_args(service_name.clone(), peer_id.clone())?; + + println!("================================================="); + println!(" Remove Safenode Services "); + println!("================================================="); + + let mut node_registry = NodeRegistry::load(&get_node_registry_path()?)?; + if let Some(ref name) = service_name { + let node = node_registry + .nodes + .iter_mut() + .find(|x| x.service_name == *name) + .ok_or_else(|| eyre!("No service named '{name}'"))?; + remove(node, &NodeServiceManager {}, keep_directories).await?; + } else if let Some(ref peer_id) = peer_id { + let peer_id = PeerId::from_str(peer_id)?; + let node = node_registry + .nodes + .iter_mut() + .find(|x| x.peer_id == Some(peer_id)) + .ok_or_else(|| { + eyre!(format!( + "Could not find node with peer ID '{}'", + peer_id.to_string() + )) + })?; + remove(node, &NodeServiceManager {}, keep_directories).await?; + } + + node_registry.save(&get_node_registry_path()?)?; + + Ok(()) + } SubCmd::Start { peer_id, service_name, diff --git a/src/node.rs b/src/node.rs index e3dcb05..542753c 100644 --- a/src/node.rs +++ b/src/node.rs @@ -14,7 +14,7 @@ use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use std::str::FromStr; -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum NodeStatus { /// The node service has been added but not started for the first time Added, @@ -22,6 +22,8 @@ pub enum NodeStatus { Running, /// The node service has been stopped Stopped, + /// The node service has been removed + Removed, } fn serialize_peer_id(value: &Option, serializer: S) -> Result @@ -63,8 +65,8 @@ pub struct Node { deserialize_with = "deserialize_peer_id" )] pub peer_id: Option, - pub data_dir_path: PathBuf, - pub log_dir_path: PathBuf, + pub data_dir_path: Option, + pub log_dir_path: Option, } #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/src/service.rs b/src/service.rs index 5091dcb..fd3d328 100644 --- a/src/service.rs +++ b/src/service.rs @@ -13,6 +13,7 @@ use libp2p::Multiaddr; use mockall::automock; use service_manager::{ ServiceInstallCtx, ServiceLabel, ServiceManager, ServiceStartCtx, ServiceStopCtx, + ServiceUninstallCtx, }; use std::ffi::OsString; use std::net::{SocketAddr, TcpListener}; @@ -42,10 +43,11 @@ pub struct ServiceConfig { pub trait ServiceControl { fn create_service_user(&self, username: &str) -> Result<()>; fn get_available_port(&self) -> Result; - fn is_service_process_running(&self, pid: u32) -> bool; fn install(&self, config: ServiceConfig) -> Result<()>; + fn is_service_process_running(&self, pid: u32) -> bool; fn start(&self, service_name: &str) -> Result<()>; fn stop(&self, service_name: &str) -> Result<()>; + fn uninstall(&self, service_name: &str) -> Result<()>; fn wait(&self, delay: u64); } @@ -207,6 +209,13 @@ impl ServiceControl for NodeServiceManager { Ok(()) } + fn uninstall(&self, service_name: &str) -> Result<()> { + let label: ServiceLabel = service_name.parse()?; + let manager = ::native()?; + manager.uninstall(ServiceUninstallCtx { label })?; + Ok(()) + } + /// Provide a delay for the service to start or stop. /// /// This is wrapped mainly just for unit testing. diff --git a/tests/e2e.rs b/tests/e2e.rs index 9c38339..c6132a2 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -203,6 +203,21 @@ fn cross_platform_service_install_and_control() { assert_eq!(stop_status[2].name, "safenode3"); assert_eq!(stop_status[2].status, "STOPPED"); assert_eq!(stop_status[2].peer_id, peer_ids[2]); + + // Remove a single node. + let mut cmd = Command::cargo_bin("safenode-manager").unwrap(); + cmd.arg("remove") + .arg("--peer-id") + .arg(single_node_stop_status[0].peer_id.clone()) + .assert() + .success(); + let output = Command::cargo_bin("safenode-manager") + .unwrap() + .arg("status") + .output() + .expect("Could not retrieve service status"); + let remove_status = parse_service_status(&output.stdout); + assert_eq!(remove_status.len(), 2); } fn parse_service_status(output: &[u8]) -> Vec {