diff --git a/validator/src/cli.rs b/validator/src/cli.rs index 15c673f349c9b5..c8f0ef4a1db9be 100644 --- a/validator/src/cli.rs +++ b/validator/src/cli.rs @@ -78,7 +78,7 @@ pub fn app<'a>(version: &'a str, default_args: &'a DefaultArgs) -> App<'a, 'a> { .subcommand(commands::set_identity::command(default_args)) .subcommand(commands::set_log_filter::command(default_args)) .subcommand(commands::staked_nodes_overrides::command(default_args)) - .subcommand(commands::wait_for_restart_window::command(default_args)) + .subcommand(commands::wait_for_restart_window::command()) .subcommand(commands::set_public_address::command(default_args)); commands::run::add_args(app, default_args) @@ -436,10 +436,6 @@ pub struct DefaultArgs { pub exit_min_idle_time: String, pub exit_max_delinquent_stake: String, - // Wait subcommand - pub wait_for_restart_window_min_idle_time: String, - pub wait_for_restart_window_max_delinquent_stake: String, - pub banking_trace_dir_byte_limit: String, pub wen_restart_path: String, @@ -537,8 +533,6 @@ impl DefaultArgs { rpc_max_request_body_size: MAX_REQUEST_BODY_SIZE.to_string(), exit_min_idle_time: "10".to_string(), exit_max_delinquent_stake: "5".to_string(), - wait_for_restart_window_min_idle_time: "10".to_string(), - wait_for_restart_window_max_delinquent_stake: "5".to_string(), banking_trace_dir_byte_limit: BANKING_TRACE_DIR_DEFAULT_BYTE_LIMIT.to_string(), wen_restart_path: "wen_restart_progress.proto".to_string(), thread_args: DefaultThreadArgs::default(), diff --git a/validator/src/commands/wait_for_restart_window/mod.rs b/validator/src/commands/wait_for_restart_window/mod.rs index cbebd4d9d8eb19..4ac7095592ec2e 100644 --- a/validator/src/commands/wait_for_restart_window/mod.rs +++ b/validator/src/commands/wait_for_restart_window/mod.rs @@ -1,5 +1,8 @@ use { - crate::{admin_rpc_service, cli::DefaultArgs, new_spinner_progress_bar, println_name_value}, + crate::{ + admin_rpc_service, commands::FromClapArgMatches, new_spinner_progress_bar, + println_name_value, + }, clap::{value_t_or_exit, App, Arg, ArgMatches, SubCommand}, console::style, solana_clap_utils::{ @@ -20,8 +23,34 @@ use { }, }; -pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> { - SubCommand::with_name("wait-for-restart-window") +const COMMAND: &str = "wait-for-restart-window"; + +const DEFAULT_MIN_IDLE_TIME: &str = "10"; +const DEFAULT_MAX_DELINQUENT_STAKE: &str = "5"; + +#[derive(Debug, PartialEq)] +pub struct WaitForRestartWindowArgs { + pub min_idle_time: usize, + pub identity: Option, + pub max_delinquent_stake: u8, + pub skip_new_snapshot_check: bool, + pub skip_health_check: bool, +} + +impl FromClapArgMatches for WaitForRestartWindowArgs { + fn from_clap_arg_match(matches: &ArgMatches) -> Result { + Ok(WaitForRestartWindowArgs { + min_idle_time: value_t_or_exit!(matches, "min_idle_time", usize), + identity: pubkey_of(matches, "identity"), + max_delinquent_stake: value_t_or_exit!(matches, "max_delinquent_stake", u8), + skip_new_snapshot_check: matches.is_present("skip_new_snapshot_check"), + skip_health_check: matches.is_present("skip_health_check"), + }) + } +} + +pub(crate) fn command<'a>() -> App<'a, 'a> { + SubCommand::with_name(COMMAND) .about("Monitor the validator for a good time to restart") .arg( Arg::with_name("min_idle_time") @@ -29,7 +58,7 @@ pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> { .takes_value(true) .validator(is_parsable::) .value_name("MINUTES") - .default_value(&default_args.wait_for_restart_window_min_idle_time) + .default_value(DEFAULT_MIN_IDLE_TIME) .help( "Minimum time that the validator should not be leader before restarting", ), @@ -47,8 +76,8 @@ pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> { .long("max-delinquent-stake") .takes_value(true) .validator(is_valid_percentage) - .default_value(&default_args.wait_for_restart_window_max_delinquent_stake) .value_name("PERCENT") + .default_value(DEFAULT_MAX_DELINQUENT_STAKE) .help("The maximum delinquent stake % permitted for a restart"), ) .arg( @@ -67,19 +96,15 @@ pub(crate) fn command(default_args: &DefaultArgs) -> App<'_, '_> { } pub fn execute(matches: &ArgMatches, ledger_path: &Path) -> Result<(), String> { - let min_idle_time = value_t_or_exit!(matches, "min_idle_time", usize); - let identity = pubkey_of(matches, "identity"); - let max_delinquent_stake = value_t_or_exit!(matches, "max_delinquent_stake", u8); - let skip_new_snapshot_check = matches.is_present("skip_new_snapshot_check"); - let skip_health_check = matches.is_present("skip_health_check"); + let wait_for_restart_window_args = WaitForRestartWindowArgs::from_clap_arg_match(matches)?; wait_for_restart_window( ledger_path, - identity, - min_idle_time, - max_delinquent_stake, - skip_new_snapshot_check, - skip_health_check, + wait_for_restart_window_args.identity, + wait_for_restart_window_args.min_idle_time, + wait_for_restart_window_args.max_delinquent_stake, + wait_for_restart_window_args.skip_new_snapshot_check, + wait_for_restart_window_args.skip_health_check, ) .map_err(|err| format!("failed to wait for restart window: {err}")) } @@ -335,3 +360,99 @@ pub fn wait_for_restart_window( println!("{}", style("Ready to restart").green()); Ok(()) } + +#[cfg(test)] +mod tests { + use {super::*, crate::commands::tests::verify_args_struct_by_command, std::str::FromStr}; + + impl Default for WaitForRestartWindowArgs { + fn default() -> Self { + WaitForRestartWindowArgs { + min_idle_time: DEFAULT_MIN_IDLE_TIME + .parse() + .expect("invalid DEFAULT_MIN_IDLE_TIME"), + identity: None, + max_delinquent_stake: DEFAULT_MAX_DELINQUENT_STAKE + .parse() + .expect("invalid DEFAULT_MAX_DELINQUENT_STAKE"), + skip_new_snapshot_check: false, + skip_health_check: false, + } + } + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_default() { + verify_args_struct_by_command( + command(), + vec![COMMAND], + WaitForRestartWindowArgs::default(), + ); + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_skip_new_snapshot_check() { + verify_args_struct_by_command( + command(), + vec![COMMAND, "--skip-new-snapshot-check"], + WaitForRestartWindowArgs { + skip_new_snapshot_check: true, + ..WaitForRestartWindowArgs::default() + }, + ); + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_skip_health_check() { + verify_args_struct_by_command( + command(), + vec![COMMAND, "--skip-health-check"], + WaitForRestartWindowArgs { + skip_health_check: true, + ..WaitForRestartWindowArgs::default() + }, + ); + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_min_idle_time() { + verify_args_struct_by_command( + command(), + vec![COMMAND, "--min-idle-time", "60"], + WaitForRestartWindowArgs { + min_idle_time: 60, + ..WaitForRestartWindowArgs::default() + }, + ); + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_identity() { + verify_args_struct_by_command( + command(), + vec![ + COMMAND, + "--identity", + "ch1do11111111111111111111111111111111111111", + ], + WaitForRestartWindowArgs { + identity: Some( + Pubkey::from_str("ch1do11111111111111111111111111111111111111").unwrap(), + ), + ..WaitForRestartWindowArgs::default() + }, + ); + } + + #[test] + fn verify_args_struct_by_command_wait_for_restart_window_max_delinquent_stake() { + verify_args_struct_by_command( + command(), + vec![COMMAND, "--max-delinquent-stake", "10"], + WaitForRestartWindowArgs { + max_delinquent_stake: 10, + ..WaitForRestartWindowArgs::default() + }, + ); + } +}