From 0afab09333ce75ca4bd0a65020b77360390929b1 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 10:28:14 +0000 Subject: [PATCH 1/6] refactor: [#647] extract strcut RunOptions --- src/e2e/docker.rs | 12 +++++++++--- src/e2e/runner.rs | 27 ++++++++++++++------------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/e2e/docker.rs b/src/e2e/docker.rs index 419e6138..75c67d64 100644 --- a/src/e2e/docker.rs +++ b/src/e2e/docker.rs @@ -23,6 +23,12 @@ impl Drop for RunningContainer { } } +/// `docker run` command options. +pub struct RunOptions { + pub env_vars: Vec<(String, String)>, + pub ports: Vec, +} + impl Docker { /// Builds a Docker image from a given Dockerfile. /// @@ -55,7 +61,7 @@ impl Docker { /// # Errors /// /// Will fail if the docker run command fails. - pub fn run(image: &str, container: &str, env_vars: &[(String, String)], ports: &[String]) -> io::Result { + pub fn run(image: &str, container: &str, options: &RunOptions) -> io::Result { let initial_args = vec![ "run".to_string(), "--detach".to_string(), @@ -65,14 +71,14 @@ impl Docker { // Add environment variables let mut env_var_args: Vec = vec![]; - for (key, value) in env_vars { + for (key, value) in &options.env_vars { env_var_args.push("--env".to_string()); env_var_args.push(format!("{key}={value}")); } // Add port mappings let mut port_args: Vec = vec![]; - for port in ports { + for port in &options.ports { port_args.push("--publish".to_string()); port_args.push(port.to_string()); } diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index eee2805a..a5965989 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -10,7 +10,7 @@ use rand::distributions::Alphanumeric; use rand::Rng; use super::docker::RunningContainer; -use crate::e2e::docker::Docker; +use crate::e2e::docker::{Docker, RunOptions}; use crate::e2e::logs_parser::RunningServices; use crate::e2e::temp_dir::Handler; @@ -43,15 +43,17 @@ pub fn run() { // code-review: if we want to use port 0 we don't know which ports we have to open. // Besides, if we don't use port 0 we should get the port numbers from the tracker configuration. // We could not use docker, but the intention was to create E2E tests including containerization. - let env_vars = [("TORRUST_TRACKER_CONFIG".to_string(), tracker_config.to_string())]; - let ports = [ - "6969:6969/udp".to_string(), - "7070:7070/tcp".to_string(), - "1212:1212/tcp".to_string(), - "1313:1313/tcp".to_string(), - ]; - - let container = run_tracker_container(&container_name, &env_vars, &ports); + let options = RunOptions { + env_vars: vec![("TORRUST_TRACKER_CONFIG".to_string(), tracker_config.to_string())], + ports: vec![ + "6969:6969/udp".to_string(), + "7070:7070/tcp".to_string(), + "1212:1212/tcp".to_string(), + "1313:1313/tcp".to_string(), + ], + }; + + let container = run_tracker_container(CONTAINER_TAG, &container_name, &options); let running_services = parse_running_services_from_logs(&container); @@ -144,11 +146,10 @@ fn generate_random_container_name(prefix: &str) -> String { format!("{prefix}{rand_string}") } -fn run_tracker_container(container_name: &str, env_vars: &[(String, String)], ports: &[String]) -> RunningContainer { +fn run_tracker_container(image: &str, container_name: &str, options: &RunOptions) -> RunningContainer { info!("Running docker tracker image: {container_name} ..."); - let container = - Docker::run(CONTAINER_TAG, container_name, env_vars, ports).expect("A tracker local docker image should be running"); + let container = Docker::run(image, container_name, options).expect("A tracker local docker image should be running"); info!("Waiting for the container {container_name} to be healthy ..."); From 670927c1c57b9381c90082af9856dfa11aba93ad Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 10:35:10 +0000 Subject: [PATCH 2/6] ci: [#647] E2E tests. Make sure we run at least one service per type We want to run all services in the E2E tests env. At least one running service per type: - HTTP tracker - UDP tracker - HealthCheck endpoint --- src/e2e/runner.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index a5965989..058ebed4 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -57,6 +57,8 @@ pub fn run() { let running_services = parse_running_services_from_logs(&container); + assert_there_is_at_least_one_service_per_type(&running_services); + let tracker_checker_config = serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON"); @@ -170,6 +172,21 @@ fn parse_running_services_from_logs(container: &RunningContainer) -> RunningServ RunningServices::parse_from_logs(&logs) } +fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServices) { + assert!( + !running_services.udp_trackers.is_empty(), + "At least one UDP tracker should be enabled in E2E tests configuration" + ); + assert!( + !running_services.http_trackers.is_empty(), + "At least one HTTP tracker should be enabled in E2E tests configuration" + ); + assert!( + !running_services.health_checks.is_empty(), + "At least one Health Check should be enabled in E2E tests configuration" + ); +} + fn write_tracker_checker_config_file(config_file_path: &Path, config: &str) { let mut file = File::create(config_file_path).expect("Tracker checker config file to be created"); From ddad4a432b0602067f9d7a227d3e82e5398f8baf Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 11:05:54 +0000 Subject: [PATCH 3/6] ci: [#647] E2E tests. Make sure there are not panics in logs --- src/e2e/docker.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++-- src/e2e/runner.rs | 32 ++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/e2e/docker.rs b/src/e2e/docker.rs index 75c67d64..6eb64783 100644 --- a/src/e2e/docker.rs +++ b/src/e2e/docker.rs @@ -18,8 +18,12 @@ impl Drop for RunningContainer { /// Ensures that the temporary container is stopped and removed when the /// struct goes out of scope. fn drop(&mut self) { - let _unused = Docker::stop(self); - let _unused = Docker::remove(&self.name); + if Docker::is_container_running(&self.name) { + let _unused = Docker::stop(self); + } + if Docker::container_exist(&self.name) { + let _unused = Docker::remove(&self.name); + } } } @@ -180,4 +184,50 @@ impl Docker { false } + + /// Checks if a Docker container is running. + /// + /// # Arguments + /// + /// * `container` - The name of the Docker container. + /// + /// # Returns + /// + /// `true` if the container is running, `false` otherwise. + #[must_use] + pub fn is_container_running(container: &str) -> bool { + match Command::new("docker") + .args(["ps", "-f", &format!("name={container}"), "--format", "{{.Names}}"]) + .output() + { + Ok(output) => { + let output_str = String::from_utf8_lossy(&output.stdout); + output_str.contains(container) + } + Err(_) => false, + } + } + + /// Checks if a Docker container exists. + /// + /// # Arguments + /// + /// * `container` - The name of the Docker container. + /// + /// # Returns + /// + /// `true` if the container exists, `false` otherwise. + #[must_use] + pub fn container_exist(container: &str) -> bool { + match Command::new("docker") + .args(["ps", "-a", "-f", &format!("name={container}"), "--format", "{{.Names}}"]) + .output() + { + Ok(output) => { + let output_str = String::from_utf8_lossy(&output.stdout); + output_str.contains(container) + } + Err(_) => false, + } + } } diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index 058ebed4..37006954 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -57,6 +57,8 @@ pub fn run() { let running_services = parse_running_services_from_logs(&container); + assert_there_are_no_panics_in_logs(&container); + assert_there_is_at_least_one_service_per_type(&running_services); let tracker_checker_config = @@ -69,9 +71,12 @@ pub fn run() { run_tracker_checker(&tracker_checker_config_path).expect("Tracker checker should check running services"); - // More E2E tests could be executed here in the future. For example: `cargo test ...`. + // More E2E tests could be added here in the future. + // For example: `cargo test ...` for only E2E tests, using this shared test env. + + stop_tracker_container(&container); - info!("Running container `{}` will be automatically removed", container.name); + remove_tracker_container(&container_name); } fn setup_runner_logging(level: LevelFilter) { @@ -164,6 +169,29 @@ fn run_tracker_container(image: &str, container_name: &str, options: &RunOptions container } +fn stop_tracker_container(container: &RunningContainer) { + info!("Stopping docker tracker image: {} ...", container.name); + Docker::stop(container).expect("Container should be stopped"); + assert_there_are_no_panics_in_logs(container); +} + +fn remove_tracker_container(container_name: &str) { + info!("Removing docker tracker image: {container_name} ..."); + Docker::remove(container_name).expect("Container should be removed"); +} + +fn assert_there_are_no_panics_in_logs(container: &RunningContainer) -> RunningServices { + let logs = Docker::logs(&container.name).expect("Logs should be captured from running container"); + + assert!( + !(logs.contains(" panicked at ") || logs.contains("RUST_BACKTRACE=1")), + "{}", + format!("Panics found is logs:\n{logs}") + ); + + RunningServices::parse_from_logs(&logs) +} + fn parse_running_services_from_logs(container: &RunningContainer) -> RunningServices { let logs = Docker::logs(&container.name).expect("Logs should be captured from running container"); From 68f71be7ccb437e6befbbc400d3c46b9ba2eabf8 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 11:22:48 +0000 Subject: [PATCH 4/6] refactor: [#647] E2E tests. Extract strcut TrackerContainer --- src/e2e/docker.rs | 13 ++-- src/e2e/mod.rs | 1 + src/e2e/runner.rs | 117 +++++++++-------------------- src/e2e/tracker_container.rs | 138 +++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 90 deletions(-) create mode 100644 src/e2e/tracker_container.rs diff --git a/src/e2e/docker.rs b/src/e2e/docker.rs index 6eb64783..c024efba 100644 --- a/src/e2e/docker.rs +++ b/src/e2e/docker.rs @@ -4,26 +4,26 @@ use std::process::{Command, Output}; use std::thread::sleep; use std::time::{Duration, Instant}; -use log::debug; +use log::{debug, info}; /// Docker command wrapper. pub struct Docker {} +#[derive(Clone, Debug)] pub struct RunningContainer { + pub image: String, pub name: String, pub output: Output, } impl Drop for RunningContainer { - /// Ensures that the temporary container is stopped and removed when the - /// struct goes out of scope. + /// Ensures that the temporary container is stopped when the struct goes out + /// of scope. fn drop(&mut self) { + info!("Dropping running container: {}", self.name); if Docker::is_container_running(&self.name) { let _unused = Docker::stop(self); } - if Docker::container_exist(&self.name) { - let _unused = Docker::remove(&self.name); - } } } @@ -95,6 +95,7 @@ impl Docker { if output.status.success() { Ok(RunningContainer { + image: image.to_owned(), name: container.to_owned(), output, }) diff --git a/src/e2e/mod.rs b/src/e2e/mod.rs index 6745d49c..deba8971 100644 --- a/src/e2e/mod.rs +++ b/src/e2e/mod.rs @@ -2,3 +2,4 @@ pub mod docker; pub mod logs_parser; pub mod runner; pub mod temp_dir; +pub mod tracker_container; diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index 37006954..23b534ee 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -2,20 +2,26 @@ use std::fs::File; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::Command; -use std::time::Duration; use std::{env, io}; use log::{debug, info, LevelFilter}; -use rand::distributions::Alphanumeric; -use rand::Rng; -use super::docker::RunningContainer; -use crate::e2e::docker::{Docker, RunOptions}; +use super::tracker_container::TrackerContainer; +use crate::e2e::docker::RunOptions; use crate::e2e::logs_parser::RunningServices; use crate::e2e::temp_dir::Handler; +/* code-review: + - We use always the same docker image name. Should we use a random image name (tag)? + - We use the name image name we use in other workflows `torrust-tracker:local`. + Should we use a different one like `torrust-tracker:e2e`? + - We remove the container after running tests but not the container image. + Should we remove the image too? +*/ + pub const NUMBER_OF_ARGUMENTS: usize = 2; -const CONTAINER_TAG: &str = "torrust-tracker:local"; +const CONTAINER_IMAGE: &str = "torrust-tracker:local"; +const CONTAINER_NAME_PREFIX: &str = "tracker_"; const TRACKER_CHECKER_CONFIG_FILE: &str = "tracker_checker.json"; pub struct Arguments { @@ -34,11 +40,9 @@ pub fn run() { let tracker_config = load_tracker_configuration(&args.tracker_config_path); - build_tracker_container_image(CONTAINER_TAG); - - let temp_dir = create_temp_dir(); + let mut tracker_container = TrackerContainer::new(CONTAINER_IMAGE, CONTAINER_NAME_PREFIX); - let container_name = generate_random_container_name("tracker_"); + tracker_container.build_image(); // code-review: if we want to use port 0 we don't know which ports we have to open. // Besides, if we don't use port 0 we should get the port numbers from the tracker configuration. @@ -53,30 +57,32 @@ pub fn run() { ], }; - let container = run_tracker_container(CONTAINER_TAG, &container_name, &options); + tracker_container.run(&options); - let running_services = parse_running_services_from_logs(&container); - - assert_there_are_no_panics_in_logs(&container); + let running_services = tracker_container.running_services(); assert_there_is_at_least_one_service_per_type(&running_services); let tracker_checker_config = serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON"); + let temp_dir = create_temp_dir(); + let mut tracker_checker_config_path = PathBuf::from(&temp_dir.temp_dir.path()); tracker_checker_config_path.push(TRACKER_CHECKER_CONFIG_FILE); write_tracker_checker_config_file(&tracker_checker_config_path, &tracker_checker_config); - run_tracker_checker(&tracker_checker_config_path).expect("Tracker checker should check running services"); + run_tracker_checker(&tracker_checker_config_path).expect("All tracker services should be running correctly"); // More E2E tests could be added here in the future. // For example: `cargo test ...` for only E2E tests, using this shared test env. - stop_tracker_container(&container); + tracker_container.stop(); + + tracker_container.remove(); - remove_tracker_container(&container_name); + info!("Tracker container final state:\n{:#?}", tracker_container); } fn setup_runner_logging(level: LevelFilter) { @@ -125,11 +131,6 @@ fn read_file(path: &str) -> String { std::fs::read_to_string(path).unwrap_or_else(|_| panic!("Can't read file {path}")) } -fn build_tracker_container_image(tag: &str) { - info!("Building tracker container image with tag: {} ...", tag); - Docker::build("./Containerfile", tag).expect("A tracker local docker image should be built"); -} - fn create_temp_dir() -> Handler { debug!( "Current dir: {:?}", @@ -143,63 +144,6 @@ fn create_temp_dir() -> Handler { temp_dir_handler } -fn generate_random_container_name(prefix: &str) -> String { - let rand_string: String = rand::thread_rng() - .sample_iter(&Alphanumeric) - .take(20) - .map(char::from) - .collect(); - - format!("{prefix}{rand_string}") -} - -fn run_tracker_container(image: &str, container_name: &str, options: &RunOptions) -> RunningContainer { - info!("Running docker tracker image: {container_name} ..."); - - let container = Docker::run(image, container_name, options).expect("A tracker local docker image should be running"); - - info!("Waiting for the container {container_name} to be healthy ..."); - - let is_healthy = Docker::wait_until_is_healthy(container_name, Duration::from_secs(10)); - - assert!(is_healthy, "Unhealthy tracker container: {container_name}"); - - debug!("Container {container_name} is healthy ..."); - - container -} - -fn stop_tracker_container(container: &RunningContainer) { - info!("Stopping docker tracker image: {} ...", container.name); - Docker::stop(container).expect("Container should be stopped"); - assert_there_are_no_panics_in_logs(container); -} - -fn remove_tracker_container(container_name: &str) { - info!("Removing docker tracker image: {container_name} ..."); - Docker::remove(container_name).expect("Container should be removed"); -} - -fn assert_there_are_no_panics_in_logs(container: &RunningContainer) -> RunningServices { - let logs = Docker::logs(&container.name).expect("Logs should be captured from running container"); - - assert!( - !(logs.contains(" panicked at ") || logs.contains("RUST_BACKTRACE=1")), - "{}", - format!("Panics found is logs:\n{logs}") - ); - - RunningServices::parse_from_logs(&logs) -} - -fn parse_running_services_from_logs(container: &RunningContainer) -> RunningServices { - let logs = Docker::logs(&container.name).expect("Logs should be captured from running container"); - - debug!("Logs after starting the container:\n{logs}"); - - RunningServices::parse_from_logs(&logs) -} - fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServices) { assert!( !running_services.udp_trackers.is_empty(), @@ -216,15 +160,20 @@ fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServi } fn write_tracker_checker_config_file(config_file_path: &Path, config: &str) { + info!( + "Writing Tracker Checker configuration file: {:?} \n{config}", + config_file_path + ); + let mut file = File::create(config_file_path).expect("Tracker checker config file to be created"); file.write_all(config.as_bytes()) .expect("Tracker checker config file to be written"); - - info!("Tracker checker configuration file: {:?} \n{config}", config_file_path); } -/// Runs the tracker checker +/// Runs the Tracker Checker. +/// +/// For example: /// /// ```text /// cargo run --bin tracker_checker "./share/default/config/tracker_checker.json" @@ -239,7 +188,7 @@ fn write_tracker_checker_config_file(config_file_path: &Path, config: &str) { /// Will panic if the config path is not a valid string. pub fn run_tracker_checker(config_path: &Path) -> io::Result<()> { info!( - "Running tacker checker: cargo --bin tracker_checker {}", + "Running Tracker Checker: cargo --bin tracker_checker {}", config_path.display() ); @@ -254,7 +203,7 @@ pub fn run_tracker_checker(config_path: &Path) -> io::Result<()> { } else { Err(io::Error::new( io::ErrorKind::Other, - format!("Failed to run tracker checker with config file {path}"), + format!("Failed to run Tracker Checker with config file {path}"), )) } } diff --git a/src/e2e/tracker_container.rs b/src/e2e/tracker_container.rs new file mode 100644 index 00000000..3e70942b --- /dev/null +++ b/src/e2e/tracker_container.rs @@ -0,0 +1,138 @@ +use std::time::Duration; + +use log::{debug, error, info}; +use rand::distributions::Alphanumeric; +use rand::Rng; + +use super::docker::{RunOptions, RunningContainer}; +use super::logs_parser::RunningServices; +use crate::e2e::docker::Docker; + +#[derive(Debug)] +pub struct TrackerContainer { + pub image: String, + pub name: String, + pub running: Option, +} + +impl Drop for TrackerContainer { + /// Ensures that the temporary container is removed when the + /// struct goes out of scope. + fn drop(&mut self) { + info!("Dropping tracker container: {}", self.name); + if Docker::container_exist(&self.name) { + let _unused = Docker::remove(&self.name); + } + } +} + +impl TrackerContainer { + #[must_use] + pub fn new(tag: &str, container_name_prefix: &str) -> Self { + Self { + image: tag.to_owned(), + name: Self::generate_random_container_name(container_name_prefix), + running: None, + } + } + + /// # Panics + /// + /// Will panic if it can't build the docker image. + pub fn build_image(&self) { + info!("Building tracker container image with tag: {} ...", self.image); + Docker::build("./Containerfile", &self.image).expect("A tracker local docker image should be built"); + } + + /// # Panics + /// + /// Will panic if it can't run the container. + pub fn run(&mut self, options: &RunOptions) { + info!("Running docker tracker image: {} ...", self.name); + + let container = Docker::run(&self.image, &self.name, options).expect("A tracker local docker image should be running"); + + info!("Waiting for the container {} to be healthy ...", self.name); + + let is_healthy = Docker::wait_until_is_healthy(&self.name, Duration::from_secs(10)); + + assert!(is_healthy, "Unhealthy tracker container: {}", &self.name); + + info!("Container {} is healthy ...", &self.name); + + self.running = Some(container); + + self.assert_there_are_no_panics_in_logs(); + } + + /// # Panics + /// + /// Will panic if it can't get the logs from the running container. + #[must_use] + pub fn running_services(&self) -> RunningServices { + let logs = Docker::logs(&self.name).expect("Logs should be captured from running container"); + + debug!("Parsing running services from logs. Logs :\n{logs}"); + + RunningServices::parse_from_logs(&logs) + } + + /// # Panics + /// + /// Will panic if it can't stop the container. + pub fn stop(&mut self) { + match &self.running { + Some(container) => { + info!("Stopping docker tracker container: {} ...", self.name); + + Docker::stop(container).expect("Container should be stopped"); + + self.assert_there_are_no_panics_in_logs(); + } + None => { + if Docker::is_container_running(&self.name) { + error!("Tracker container {} was started manually", self.name); + } else { + info!("Docker tracker container is not running: {} ...", self.name); + } + } + } + + self.running = None; + } + + /// # Panics + /// + /// Will panic if it can't remove the container. + pub fn remove(&self) { + match &self.running { + Some(_running_container) => { + error!("Can't remove running container: {} ...", self.name); + } + None => { + info!("Removing docker tracker container: {} ...", self.name); + Docker::remove(&self.name).expect("Container should be removed"); + } + } + } + + fn generate_random_container_name(prefix: &str) -> String { + let rand_string: String = rand::thread_rng() + .sample_iter(&Alphanumeric) + .take(20) + .map(char::from) + .collect(); + + format!("{prefix}{rand_string}") + } + + fn assert_there_are_no_panics_in_logs(&self) { + let logs = Docker::logs(&self.name).expect("Logs should be captured from running container"); + + assert!( + !(logs.contains(" panicked at ") || logs.contains("RUST_BACKTRACE=1")), + "{}", + format!("Panics found is logs:\n{logs}") + ); + } +} From e5cd81bdd1ab7867c240b7a91f30414f604b0318 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 15:14:49 +0000 Subject: [PATCH 5/6] refactor: [#647] E2E tests. Extract function --- src/e2e/runner.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index 23b534ee..81821088 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -63,16 +63,13 @@ pub fn run() { assert_there_is_at_least_one_service_per_type(&running_services); - let tracker_checker_config = - serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON"); - let temp_dir = create_temp_dir(); - let mut tracker_checker_config_path = PathBuf::from(&temp_dir.temp_dir.path()); - tracker_checker_config_path.push(TRACKER_CHECKER_CONFIG_FILE); - - write_tracker_checker_config_file(&tracker_checker_config_path, &tracker_checker_config); + let tracker_checker_config_path = + create_tracker_checker_config_file(&running_services, temp_dir.temp_dir.path(), TRACKER_CHECKER_CONFIG_FILE); + // todo: inject the configuration with an env variable so that we don't have + // to create the temporary directory/file. run_tracker_checker(&tracker_checker_config_path).expect("All tracker services should be running correctly"); // More E2E tests could be added here in the future. @@ -159,6 +156,18 @@ fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServi ); } +fn create_tracker_checker_config_file(running_services: &RunningServices, config_path: &Path, config_name: &str) -> PathBuf { + let tracker_checker_config = + serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON"); + + let mut tracker_checker_config_path = PathBuf::from(&config_path); + tracker_checker_config_path.push(config_name); + + write_tracker_checker_config_file(&tracker_checker_config_path, &tracker_checker_config); + + tracker_checker_config_path +} + fn write_tracker_checker_config_file(config_file_path: &Path, config: &str) { info!( "Writing Tracker Checker configuration file: {:?} \n{config}", From f18e68cc498fa98ebb2b23b0605dd8ec60dbf579 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 26 Jan 2024 16:22:56 +0000 Subject: [PATCH 6/6] fix: tracker checker return error code when it fails This command: ``` cargo run --bin tracker_checker "./share/default/config/tracker_checker.json" && echo "OK" ``` should not print OK when it fails. --- src/bin/tracker_checker.rs | 2 +- src/checker/app.rs | 10 +++++++--- src/checker/service.rs | 36 +++++++++++++++++++++++++++++++----- src/e2e/runner.rs | 2 +- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/src/bin/tracker_checker.rs b/src/bin/tracker_checker.rs index 3a0e0ee8..d2f67609 100644 --- a/src/bin/tracker_checker.rs +++ b/src/bin/tracker_checker.rs @@ -7,5 +7,5 @@ use torrust_tracker::checker::app; #[tokio::main] async fn main() { - app::run().await; + app::run().await.expect("Some checks fail"); } diff --git a/src/checker/app.rs b/src/checker/app.rs index e9237349..22ed61ba 100644 --- a/src/checker/app.rs +++ b/src/checker/app.rs @@ -2,18 +2,22 @@ use std::sync::Arc; use super::config::Configuration; use super::console::Console; +use super::service::{CheckError, Service}; use crate::checker::config::parse_from_json; -use crate::checker::service::Service; pub const NUMBER_OF_ARGUMENTS: usize = 2; +/// # Errors +/// +/// If some checks fails it will return a vector with all failing checks. +/// /// # Panics /// /// Will panic if: /// /// - It can't read the json configuration file. /// - The configuration file is invalid. -pub async fn run() { +pub async fn run() -> Result<(), Vec> { let args = parse_arguments(); let config = setup_config(&args); let console_printer = Console {}; @@ -22,7 +26,7 @@ pub async fn run() { console: console_printer, }; - service.run_checks().await; + service.run_checks().await } pub struct Arguments { diff --git a/src/checker/service.rs b/src/checker/service.rs index 92902deb..25471637 100644 --- a/src/checker/service.rs +++ b/src/checker/service.rs @@ -14,12 +14,24 @@ pub struct Service { pub(crate) console: Console, } +#[derive(Debug)] +pub enum CheckError { + UdpError, + HttpError, + HealthCheckError { url: Url }, +} + impl Service { - pub async fn run_checks(&self) { + /// # Errors + /// + /// Will return OK is all checks pass or an array with the check errors. + pub async fn run_checks(&self) -> Result<(), Vec> { self.console.println("Running checks for trackers ..."); + self.check_udp_trackers(); self.check_http_trackers(); - self.run_health_checks().await; + + self.run_health_checks().await } fn check_udp_trackers(&self) { @@ -38,11 +50,22 @@ impl Service { } } - async fn run_health_checks(&self) { + async fn run_health_checks(&self) -> Result<(), Vec> { self.console.println("Health checks ..."); + let mut check_errors = vec![]; + for health_check_url in &self.config.health_checks { - self.run_health_check(health_check_url.clone()).await; + match self.run_health_check(health_check_url.clone()).await { + Ok(()) => {} + Err(err) => check_errors.push(err), + } + } + + if check_errors.is_empty() { + Ok(()) + } else { + Err(check_errors) } } @@ -62,7 +85,7 @@ impl Service { .println(&format!("{} - HTTP tracker at {} is OK (TODO)", "✓".green(), url)); } - async fn run_health_check(&self, url: Url) { + async fn run_health_check(&self, url: Url) -> Result<(), CheckError> { let client = Client::builder().timeout(Duration::from_secs(5)).build().unwrap(); match client.get(url.clone()).send().await { @@ -70,14 +93,17 @@ impl Service { if response.status().is_success() { self.console .println(&format!("{} - Health API at {} is OK", "✓".green(), url)); + Ok(()) } else { self.console .eprintln(&format!("{} - Health API at {} failing: {:?}", "✗".red(), url, response)); + Err(CheckError::HealthCheckError { url }) } } Err(err) => { self.console .eprintln(&format!("{} - Health API at {} failing: {:?}", "✗".red(), url, err)); + Err(CheckError::HealthCheckError { url }) } } } diff --git a/src/e2e/runner.rs b/src/e2e/runner.rs index 81821088..aaac0e91 100644 --- a/src/e2e/runner.rs +++ b/src/e2e/runner.rs @@ -68,7 +68,7 @@ pub fn run() { let tracker_checker_config_path = create_tracker_checker_config_file(&running_services, temp_dir.temp_dir.path(), TRACKER_CHECKER_CONFIG_FILE); - // todo: inject the configuration with an env variable so that we don't have + // todo: inject the configuration with an env variable so that we don't have // to create the temporary directory/file. run_tracker_checker(&tracker_checker_config_path).expect("All tracker services should be running correctly");