From bc044ed27518be3495262ba36a9c7f40a2a73e8f Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 18 Jun 2024 01:05:15 +1000 Subject: [PATCH] Fix `CommandLineTest` port conflicts on CI (#5908) * Fix port conflicts on CI. --- lighthouse/src/main.rs | 56 +++++++++++++++++---------------- lighthouse/tests/beacon_node.rs | 34 ++++++++++++++++---- lighthouse/tests/exec.rs | 18 ++++++++--- 3 files changed, 71 insertions(+), 37 deletions(-) diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 5743bedfd7..abee30737c 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -688,9 +688,15 @@ fn run( let executor = context.executor.clone(); let mut config = beacon_node::get_config::(matches, &context)?; config.logger_config = logger_config; - let shutdown_flag = matches.get_flag("immediate-shutdown"); // Dump configs if `dump-config` or `dump-chain-config` flags are set clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; + + let shutdown_flag = matches.get_flag("immediate-shutdown"); + if shutdown_flag { + info!(log, "Beacon node immediate shutdown triggered."); + return Ok(()); + } + executor.clone().spawn( async move { if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await { @@ -700,10 +706,6 @@ fn run( let _ = executor .shutdown_sender() .try_send(ShutdownReason::Failure("Failed to start beacon node")); - } else if shutdown_flag { - let _ = executor.shutdown_sender().try_send(ShutdownReason::Success( - "Beacon node immediate shutdown triggered.", - )); } }, "beacon_node", @@ -715,31 +717,31 @@ fn run( let executor = context.executor.clone(); let config = validator_client::Config::from_cli(matches, context.log()) .map_err(|e| format!("Unable to initialize validator config: {}", e))?; - let shutdown_flag = matches.get_flag("immediate-shutdown"); // Dump configs if `dump-config` or `dump-chain-config` flags are set clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; - if !shutdown_flag { - executor.clone().spawn( - async move { - if let Err(e) = ProductionValidatorClient::new(context, config) - .and_then(|mut vc| async move { vc.start_service().await }) - .await - { - crit!(log, "Failed to start validator client"; "reason" => e); - // Ignore the error since it always occurs during normal operation when - // shutting down. - let _ = executor.shutdown_sender().try_send(ShutdownReason::Failure( - "Failed to start validator client", - )); - } - }, - "validator_client", - ); - } else { - let _ = executor.shutdown_sender().try_send(ShutdownReason::Success( - "Validator client immediate shutdown triggered.", - )); + + let shutdown_flag = matches.get_flag("immediate-shutdown"); + if shutdown_flag { + info!(log, "Validator client immediate shutdown triggered."); + return Ok(()); } + + executor.clone().spawn( + async move { + if let Err(e) = ProductionValidatorClient::new(context, config) + .and_then(|mut vc| async move { vc.start_service().await }) + .await + { + crit!(log, "Failed to start validator client"; "reason" => e); + // Ignore the error since it always occurs during normal operation when + // shutting down. + let _ = executor + .shutdown_sender() + .try_send(ShutdownReason::Failure("Failed to start validator client")); + } + }, + "validator_client", + ); } _ => { crit!(log, "No subcommand supplied. See --help ."); diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 73badac913..caadf208e3 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -58,13 +58,22 @@ impl CommandLineTest { fn run_with_zero_port(&mut self) -> CompletedTest { // Required since Deneb was enabled on mainnet. - self.cmd.arg("--allow-insecure-genesis-sync"); - self.run_with_zero_port_and_no_genesis_sync() + self.set_allow_insecure_genesis_sync() + .run_with_zero_port_and_no_genesis_sync() } fn run_with_zero_port_and_no_genesis_sync(&mut self) -> CompletedTest { + self.set_zero_port().run() + } + + fn set_allow_insecure_genesis_sync(&mut self) -> &mut Self { + self.cmd.arg("--allow-insecure-genesis-sync"); + self + } + + fn set_zero_port(&mut self) -> &mut Self { self.cmd.arg("-z"); - self.run() + self } } @@ -101,9 +110,20 @@ fn staking_flag() { } #[test] -#[should_panic] fn allow_insecure_genesis_sync_default() { - CommandLineTest::new().run_with_zero_port_and_no_genesis_sync(); + CommandLineTest::new() + .run_with_zero_port_and_no_genesis_sync() + .with_config(|config| { + assert_eq!(config.allow_insecure_genesis_sync, false); + }); +} + +#[test] +#[should_panic] +fn insecure_genesis_sync_should_panic() { + CommandLineTest::new() + .set_zero_port() + .run_with_immediate_shutdown(false); } #[test] @@ -2252,7 +2272,9 @@ fn ensure_panic_on_failed_launch() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-chunk-size", Some("10")) - .run_with_zero_port() + .set_allow_insecure_genesis_sync() + .set_zero_port() + .run_with_immediate_shutdown(false) .with_config(|config| { let slasher_config = config .slasher diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index 61e0677ca8..9d6453908c 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -21,12 +21,19 @@ pub trait CommandLineTestExec { self } + fn run(&mut self) -> CompletedTest { + self.run_with_immediate_shutdown(true) + } + /// Executes the `Command` returned by `Self::cmd_mut` with temporary data directory, dumps - /// the configuration and shuts down immediately. + /// the configuration and shuts down immediately if `immediate_shutdown` is set to true. /// /// Options `--datadir`, `--dump-config`, `--dump-chain-config` and `--immediate-shutdown` must /// not be set on the command. - fn run(&mut self) -> CompletedTest { + fn run_with_immediate_shutdown( + &mut self, + immediate_shutdown: bool, + ) -> CompletedTest { // Setup temp directory. let tmp_dir = TempDir::new().expect("Unable to create temporary directory"); let tmp_config_path: PathBuf = tmp_dir.path().join("config.json"); @@ -39,8 +46,11 @@ pub trait CommandLineTestExec { .arg(format!("--{}", "dump-config")) .arg(tmp_config_path.as_os_str()) .arg(format!("--{}", "dump-chain-config")) - .arg(tmp_chain_config_path.as_os_str()) - .arg("--immediate-shutdown"); + .arg(tmp_chain_config_path.as_os_str()); + + if immediate_shutdown { + cmd.arg("--immediate-shutdown"); + } // Run the command. let output = output_result(cmd);