Skip to content

Commit

Permalink
feat: provide --rpc-port arg for add cmd
Browse files Browse the repository at this point in the history 
This feature was requested in community feedback. It behaves the same way as the other port
arguments, in that it can be a range.

Some of the `add_node` tests were changed to remove the use of a custom RPC address argument because
the test case did not concern itself with customising the RPC address.
  • Loading branch information
@jacderida
jacderida committed Mar 20, 2024
1 parent 0868299 commit b904f0f
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 13 deletions.
2 changes: 1 addition & 1 deletion sn_node/src/bin/safenode/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
extern crate tracing;

mod rpc_service;
sn node bin

use clap::Parser;
use eyre::{eyre, Result};
use libp2p::{identity::Keypair, PeerId};
Expand Down
1 change: 1 addition & 0 deletions sn_node_manager/src/add_services/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub struct AddNodeServiceOptions {
pub metrics_port: Option<PortRange>,
pub node_port: Option<PortRange>,
pub rpc_address: Option<Ipv4Addr>,
pub rpc_port: Option<PortRange>,
pub safenode_src_path: PathBuf,
pub safenode_dir_path: PathBuf,
pub service_data_dir_path: PathBuf,
Expand Down
8 changes: 7 additions & 1 deletion sn_node_manager/src/add_services/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,14 @@ pub async fn add_node(
let mut node_number = current_node_count + 1;
let mut node_port = get_start_port_if_applicable(options.node_port);
let mut metrics_port = get_start_port_if_applicable(options.metrics_port);
let mut rpc_port = get_start_port_if_applicable(options.rpc_port);

while node_number <= target_node_count {
let rpc_free_port = service_control.get_available_port()?;
let rpc_free_port = if let Some(port) = rpc_port {
port
} else {
service_control.get_available_port()?
};
let rpc_socket_addr = if let Some(addr) = options.rpc_address {
SocketAddr::new(IpAddr::V4(addr), rpc_free_port)
} else {
Expand Down Expand Up @@ -190,6 +195,7 @@ pub async fn add_node(
node_number += 1;
node_port = increment_port_option(node_port);
metrics_port = increment_port_option(metrics_port);
rpc_port = increment_port_option(rpc_port);
}

std::fs::remove_file(options.safenode_src_path)?;
Expand Down
213 changes: 203 additions & 10 deletions sn_node_manager/src/add_services/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ async fn add_genesis_node_should_use_latest_version_and_add_one_service() -> Res
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(),
Expand Down Expand Up @@ -234,6 +235,7 @@ async fn add_genesis_node_should_return_an_error_if_there_is_already_a_genesis_n
metrics_port: None,
node_port: None,
rpc_address: Some(custom_rpc_address),
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(),
Expand Down Expand Up @@ -280,8 +282,6 @@ async fn add_genesis_node_should_return_an_error_if_count_is_greater_than_1() ->
let safenode_download_path = temp_dir.child(SAFENODE_FILE_NAME);
safenode_download_path.write_binary(b"fake safenode bin")?;

let custom_rpc_address = Ipv4Addr::new(127, 0, 0, 1);

let result = add_node(
AddNodeServiceOptions {
bootstrap_peers: vec![],
Expand All @@ -291,7 +291,8 @@ async fn add_genesis_node_should_return_an_error_if_count_is_greater_than_1() ->
local: true,
metrics_port: None,
node_port: None,
rpc_address: Some(custom_rpc_address),
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(),
Expand Down Expand Up @@ -447,6 +448,7 @@ async fn add_node_should_use_latest_version_and_add_three_services() -> Result<(
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(),
Expand Down Expand Up @@ -585,7 +587,8 @@ async fn add_node_should_update_the_bootstrap_peers_inside_node_registry() -> Re
genesis: false,
metrics_port: None,
node_port: None,
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -698,7 +701,8 @@ async fn add_node_should_update_the_environment_variables_inside_node_registry()
local: false,
metrics_port: None,
node_port: None,
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -822,6 +826,7 @@ async fn add_new_node_should_add_another_service() -> Result<()> {
metrics_port: None,
node_port: None,
rpc_address: None,
rpc_port: None,
safenode_src_path: safenode_download_path.to_path_buf(),
safenode_dir_path: temp_dir.to_path_buf(),
service_data_dir_path: node_data_dir.to_path_buf(),
Expand Down Expand Up @@ -925,7 +930,8 @@ async fn add_node_should_use_custom_ports_for_one_service() -> Result<()> {
local: false,
metrics_port: None,
node_port: Some(PortRange::Single(custom_port)),
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -1135,7 +1141,8 @@ async fn add_node_should_use_a_custom_port_range() -> Result<()> {
local: false,
metrics_port: None,
node_port: Some(PortRange::Range(12000, 12002)),
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -1188,7 +1195,8 @@ async fn add_node_should_return_an_error_if_port_and_node_count_do_not_match() -
local: false,
metrics_port: None,
node_port: Some(PortRange::Range(12000, 12002)),
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -1247,7 +1255,8 @@ async fn add_node_should_return_an_error_if_multiple_services_are_specified_with
local: false,
metrics_port: None,
node_port: Some(PortRange::Single(12000)),
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand Down Expand Up @@ -1444,7 +1453,8 @@ async fn add_node_should_use_a_custom_port_range_for_metrics_server() -> Result<
local: false,
metrics_port: Some(PortRange::Range(12000, 12002)),
node_port: None,
rpc_address: Some(Ipv4Addr::new(127, 0, 0, 1)),
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(),
Expand All @@ -1466,6 +1476,189 @@ async fn add_node_should_use_a_custom_port_range_for_metrics_server() -> Result<
Ok(())
}

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

// First service
mock_service_control
.expect_install()
.times(1)
.with(eq(ServiceInstallCtx {
args: vec![
OsString::from("--rpc"),
OsString::from("127.0.0.1:20000"),
OsString::from("--root-dir"),
OsString::from(
node_data_dir
.to_path_buf()
.join("safenode1")
.to_string_lossy()
.to_string(),
),
OsString::from("--log-output-dest"),
OsString::from(
node_logs_dir
.to_path_buf()
.join("safenode1")
.to_string_lossy()
.to_string(),
),
],
contents: None,
environment: None,
label: "safenode1".parse()?,
program: node_data_dir
.to_path_buf()
.join("safenode1")
.join(SAFENODE_FILE_NAME),
username: Some(get_username()),
working_directory: None,
}))
.returning(|_| Ok(()))
.in_sequence(&mut seq);

// Second service
mock_service_control
.expect_install()
.times(1)
.with(eq(ServiceInstallCtx {
args: vec![
OsString::from("--rpc"),
OsString::from("127.0.0.1:20001"),
OsString::from("--root-dir"),
OsString::from(
node_data_dir
.to_path_buf()
.join("safenode2")
.to_string_lossy()
.to_string(),
),
OsString::from("--log-output-dest"),
OsString::from(
node_logs_dir
.to_path_buf()
.join("safenode2")
.to_string_lossy()
.to_string(),
),
],
contents: None,
environment: None,
label: "safenode2".parse()?,
program: node_data_dir
.to_path_buf()
.join("safenode2")
.join(SAFENODE_FILE_NAME),
username: Some(get_username()),
working_directory: None,
}))
.returning(|_| Ok(()))
.in_sequence(&mut seq);

// Third service
mock_service_control
.expect_install()
.times(1)
.with(eq(ServiceInstallCtx {
args: vec![
OsString::from("--rpc"),
OsString::from("127.0.0.1:20002"),
OsString::from("--root-dir"),
OsString::from(
node_data_dir
.to_path_buf()
.join("safenode3")
.to_string_lossy()
.to_string(),
),
OsString::from("--log-output-dest"),
OsString::from(
node_logs_dir
.to_path_buf()
.join("safenode3")
.to_string_lossy()
.to_string(),
),
],
contents: None,
environment: None,
label: "safenode3".parse()?,
program: node_data_dir
.to_path_buf()
.join("safenode3")
.join(SAFENODE_FILE_NAME),
username: Some(get_username()),
working_directory: None,
}))
.returning(|_| Ok(()))
.in_sequence(&mut seq);

add_node(
AddNodeServiceOptions {
bootstrap_peers: vec![],
count: Some(3),
env_variables: None,
genesis: false,
local: false,
metrics_port: None,
node_port: None,
rpc_address: None,
rpc_port: Some(PortRange::Range(20000, 20002)),
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::missing());
node_data_dir.assert(predicate::path::is_dir());
node_logs_dir.assert(predicate::path::is_dir());
assert_eq!(node_registry.nodes.len(), 3);
assert_eq!(
node_registry.nodes[0].rpc_socket_addr,
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 20000)
);
assert_eq!(
node_registry.nodes[1].rpc_socket_addr,
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 20001)
);
assert_eq!(
node_registry.nodes[2].rpc_socket_addr,
SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 20002)
);
Ok(())
}

#[tokio::test]
async fn add_faucet_should_add_a_faucet_service() -> Result<()> {
let tmp_data_dir = assert_fs::TempDir::new()?;
Expand Down
13 changes: 12 additions & 1 deletion sn_node_manager/src/bin/cli/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,22 @@ pub enum SubCmd {
path: Option<PathBuf>,
#[command(flatten)]
peers: PeersArgs,
#[clap(long)]
/// Specify an Ipv4Addr for the node's RPC server to run on.
///
/// Useful if you want to expose the RPC server pubilcly. Ports are assigned automatically.
///
/// If not set, the RPC server is run locally.
#[clap(long)]
rpc_address: Option<Ipv4Addr>,
/// Specify a port for the RPC service(s).
///
/// If not used, ports will be selected at random.
///
/// If multiple services are being added and this argument is used, you must specify a
/// range. For example, '12000-12004'. The length of the range must match the number of
/// services, which in this case would be 5. The range must also go from lower to higher.
#[clap(long, value_parser = parse_port_range)]
rpc_port: Option<PortRange>,
/// Provide a safenode binary using a URL.
///
/// The binary must be inside a zip or gzipped tar archive.
Expand Down Expand Up @@ -526,6 +535,7 @@ async fn main() -> Result<()> {
path,
peers,
rpc_address,
rpc_port,
url,
user,
version,
Expand All @@ -540,6 +550,7 @@ async fn main() -> Result<()> {
node_port,
peers,
rpc_address,
rpc_port,
path,
url,
user,
Expand Down
Loading

0 comments on commit b904f0f

Please sign in to comment.