Skip to content

Commit

Permalink
feat: provide --upnp flag for add command
Browse files Browse the repository at this point in the history 
The `--upnp` flag that is available on the node is now also available on the node manager `add`
command.

The flag only applies to a node that was built with the `upnp` feature. Rather than also using the
feature in the node manager, the command is documented to specify that it should only be used with
a `safenode` binary that has the feature. So for now, this option will probably be used in
conjunction with `--path`, to provide a node that has been built with `upnp` enabled.
  • Loading branch information
@jacderida
jacderida committed May 16, 2024
1 parent 9812d39 commit e3caac5
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 1 deletion.
5 changes: 5 additions & 0 deletions sn_node_manager/src/add_services/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub struct InstallNodeServiceCtxBuilder {
pub rpc_socket_addr: SocketAddr,
pub safenode_path: PathBuf,
pub service_user: Option<String>,
pub upnp: bool,
}

impl InstallNodeServiceCtxBuilder {
Expand All @@ -77,6 +78,9 @@ impl InstallNodeServiceCtxBuilder {
if self.local {
args.push(OsString::from("--local"));
}
if self.upnp {
args.push(OsString::from("--upnp"));
}
if let Some(node_port) = self.node_port {
args.push(OsString::from("--port"));
args.push(OsString::from(node_port.to_string()));
Expand Down Expand Up @@ -125,6 +129,7 @@ pub struct AddNodeServiceOptions {
pub safenode_dir_path: PathBuf,
pub service_data_dir_path: PathBuf,
pub service_log_dir_path: PathBuf,
pub upnp: bool,
pub user: Option<String>,
pub user_mode: bool,
pub version: String,
Expand Down
2 changes: 2 additions & 0 deletions sn_node_manager/src/add_services/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ pub async fn add_node(
rpc_socket_addr,
safenode_path: service_safenode_path.clone(),
service_user: options.user.clone(),
upnp: options.upnp,
}
.build()?;

Expand Down Expand Up @@ -209,6 +210,7 @@ pub async fn add_node(
safenode_path: service_safenode_path,
service_name,
status: ServiceStatus::Added,
upnp: options.upnp,
user: options.user.clone(),
user_mode: options.user_mode,
version: options.version.clone(),
Expand Down
124 changes: 124 additions & 0 deletions sn_node_manager/src/add_services/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ async fn add_genesis_node_should_use_latest_version_and_add_one_service() -> Res
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;
mock_service_control
Expand All @@ -148,6 +149,7 @@ async fn add_genesis_node_should_use_latest_version_and_add_one_service() -> Res
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -214,6 +216,7 @@ async fn add_genesis_node_should_return_an_error_if_there_is_already_a_genesis_n
status: ServiceStatus::Added,
safenode_path: PathBuf::from("/var/safenode-manager/services/safenode1/safenode"),
service_name: "safenode1".to_string(),
upnp: false,
user: Some("safe".to_string()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -250,6 +253,7 @@ async fn add_genesis_node_should_return_an_error_if_there_is_already_a_genesis_n
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -310,6 +314,7 @@ async fn add_genesis_node_should_return_an_error_if_count_is_greater_than_1() ->
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -379,6 +384,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -412,6 +418,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
.join("safenode2")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -445,6 +452,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
.join("safenode3")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -472,6 +480,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -590,6 +599,7 @@ async fn add_node_should_update_the_bootstrap_peers_inside_node_registry() -> Re
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;
mock_service_control
Expand All @@ -616,6 +626,7 @@ async fn add_node_should_update_the_bootstrap_peers_inside_node_registry() -> Re
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -708,6 +719,7 @@ async fn add_node_should_update_the_environment_variables_inside_node_registry()
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;
mock_service_control
Expand All @@ -734,6 +746,7 @@ async fn add_node_should_update_the_environment_variables_inside_node_registry()
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -799,6 +812,7 @@ async fn add_new_node_should_add_another_service() -> Result<()> {
safenode_path: PathBuf::from("/var/safenode-manager/services/safenode1/safenode"),
service_name: "safenode1".to_string(),
status: ServiceStatus::Added,
upnp: false,
user: Some("safe".to_string()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -838,6 +852,7 @@ async fn add_new_node_should_add_another_service() -> Result<()> {
.join("safenode2")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -865,6 +880,7 @@ async fn add_new_node_should_add_another_service() -> Result<()> {
safenode_dir_path: temp_dir.to_path_buf(),
service_data_dir_path: node_data_dir.to_path_buf(),
service_log_dir_path: node_logs_dir.to_path_buf(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -947,6 +963,7 @@ async fn add_node_should_use_custom_ports_for_one_service() -> Result<()> {
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -974,6 +991,7 @@ async fn add_node_should_use_custom_ports_for_one_service() -> Result<()> {
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -1197,6 +1215,7 @@ async fn add_node_should_use_a_custom_port_range() -> Result<()> {
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -1254,6 +1273,7 @@ async fn add_node_should_return_an_error_if_port_and_node_count_do_not_match() -
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -1317,6 +1337,7 @@ async fn add_node_should_return_an_error_if_multiple_services_are_specified_with
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -1527,6 +1548,7 @@ async fn add_node_should_use_a_custom_port_range_for_metrics_server() -> Result<
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -1711,6 +1733,7 @@ async fn add_node_should_use_a_custom_port_range_for_the_rpc_server() -> Result<
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -2075,6 +2098,7 @@ async fn add_node_should_not_delete_the_source_binary_if_path_arg_is_used() -> R
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -2102,6 +2126,7 @@ async fn add_node_should_not_delete_the_source_binary_if_path_arg_is_used() -> R
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -2168,6 +2193,7 @@ async fn add_node_should_apply_the_home_network_flag_if_it_is_used() -> Result<(
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -2195,6 +2221,7 @@ async fn add_node_should_apply_the_home_network_flag_if_it_is_used() -> Result<(
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(),
upnp: false,
user: Some(get_username()),
user_mode: false,
version: latest_version.to_string(),
Expand Down Expand Up @@ -2261,6 +2288,7 @@ async fn add_node_should_add_the_node_in_user_mode() -> Result<()> {
.join("safenode1")
.join(SAFENODE_FILE_NAME),
service_user: Some(get_username()),
upnp: false,
}
.build()?;

Expand Down Expand Up @@ -2288,6 +2316,7 @@ async fn add_node_should_add_the_node_in_user_mode() -> Result<()> {
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(),
upnp: false,
user: Some(get_username()),
user_mode: true,
version: latest_version.to_string(),
Expand All @@ -2300,3 +2329,98 @@ async fn add_node_should_add_the_node_in_user_mode() -> Result<()> {

Ok(())
}

#[tokio::test]
async fn add_node_should_add_the_node_with_upnp_enabled() -> 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();

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,
home_network: true,
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: Some(get_username()),
upnp: true,
}
.build()?;

mock_service_control
.expect_install()
.times(1)
.with(eq(install_ctx), eq(true))
.returning(|_, _| Ok(()))
.in_sequence(&mut seq);

add_node(
AddNodeServiceOptions {
bootstrap_peers: vec![],
count: Some(1),
delete_safenode_src: false,
env_variables: None,
genesis: false,
home_network: true,
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(),
upnp: true,
user: Some(get_username()),
user_mode: true,
version: latest_version.to_string(),
},
&mut node_registry,
&mock_service_control,
VerbosityLevel::Normal,
)
.await?;

assert_eq!(node_registry.nodes.len(), 1);
assert!(node_registry.nodes[0].upnp);

Ok(())
}
Loading

0 comments on commit e3caac5

Please sign in to comment.