Improve environment building error handling

* Drop suites with failures
* Report error messages

CMK-14782

v2/robotmk/src/config/internal.rs

@@ -5,6 +5,8 @@ use crate::session::Session;
 use crate::termination::TerminationFlag;
 
 use camino::Utf8PathBuf;
+use log::{debug, warn};
+use std::collections::HashMap;
 use std::sync::Arc;
 use std::sync::Mutex;
 
@@ -58,6 +60,24 @@ pub fn from_external_config(
     )
 }
 
+pub fn drop_suites<'a>(
+    suites: Vec<Suite>,
+    suites_to_be_dropped: impl IntoIterator<Item = &'a String>,
+) -> Vec<Suite> {
+    let mut suites_by_name: HashMap<String, Suite> =
+        HashMap::from_iter(suites.into_iter().map(|suite| (suite.name.clone(), suite)));
+    for suite_name in suites_to_be_dropped {
+        if suites_by_name.remove(suite_name).is_some() {
+            debug!("Dropped suite {suite_name}")
+        } else {
+            warn!("Attempted to drop suite {suite_name}, but no suite with this name exists")
+        }
+    }
+    let mut suites = suites_by_name.into_values().collect::<Vec<Suite>>();
+    sort_suites_by_name(&mut suites);
+    suites
+}
+
 fn sort_suites_by_name(suites: &mut [Suite]) {
     suites.sort_by_key(|suite| suite.name.to_string());
 }
@@ -186,4 +206,38 @@ mod tests {
         );
         assert_eq!(suites[1].session, Session::Current(CurrentSession {}));
     }
+
+    #[test]
+    fn test_drop_suites() {
+        let (_global_config, suites) = from_external_config(
+            Config {
+                working_directory: Utf8PathBuf::from("/working"),
+                results_directory: Utf8PathBuf::from("/results"),
+                suites: HashMap::from([
+                    (String::from("system"), system_suite_config()),
+                    (String::from("rcc1"), rcc_suite_config()),
+                    (String::from("rcc2"), rcc_suite_config()),
+                ]),
+            },
+            TerminationFlag::new(),
+        );
+        let suites = drop_suites(suites, &vec!["rcc1".into()]);
+        assert_eq!(suites.len(), 2);
+        assert_eq!(suites[0].name, "rcc2");
+        assert_eq!(
+            suites[0].environment,
+            Environment::Rcc(RCCEnvironment {
+                binary_path: Utf8PathBuf::from("/bin/rcc"),
+                robot_yaml_path: Utf8PathBuf::from("/suite/rcc/robot.yaml"),
+                controller: "robotmk".into(),
+                space: "rcc2".into(),
+                build_timeout: 300,
+            })
+        );
+        assert_eq!(suites[1].name, "system");
+        assert_eq!(
+            suites[1].environment,
+            Environment::System(SystemEnvironment {}),
+        );
+    }
 }

v2/robotmk/src/environment.rs

@@ -1,8 +1,11 @@
 use super::child_process_supervisor::{ChildProcessOutcome, ChildProcessSupervisor, StdioPaths};
 use super::command_spec::CommandSpec;
 use super::config::external::EnvironmentConfig;
-use super::config::internal::{GlobalConfig, Suite};
-use super::results::{EnvironmentBuildStatesAdministrator, EnvironmentBuildStatus};
+use super::config::internal::{drop_suites, GlobalConfig, Suite};
+use super::logging::log_and_return_error;
+use super::results::{
+    EnvironmentBuildStatesAdministrator, EnvironmentBuildStatus, EnvironmentBuildStatusError,
+};
 
 use anyhow::{bail, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
@@ -12,7 +15,7 @@ pub fn environment_building_stdio_directory(working_directory: &Utf8Path) -> Utf
     working_directory.join("environment_building_stdio")
 }
 
-pub fn build_environments(global_config: &GlobalConfig, suites: &Vec<Suite>) -> Result<()> {
+pub fn build_environments(global_config: &GlobalConfig, suites: Vec<Suite>) -> Result<Vec<Suite>> {
     let mut environment_build_states_administrator =
         EnvironmentBuildStatesAdministrator::new_with_pending(
             suites.iter().map(|suite| &suite.name),
@@ -23,68 +26,89 @@ pub fn build_environments(global_config: &GlobalConfig, suites: &Vec<Suite>) ->
     let env_building_stdio_directory =
         environment_building_stdio_directory(&global_config.working_directory);
 
-    for suite in suites {
-        environment_build_states_administrator = build_environment(
+    let mut suites_to_be_dropped = vec![];
+    let mut drop_suite;
+    for suite in suites.iter() {
+        (environment_build_states_administrator, drop_suite) = build_environment(
             suite,
             environment_build_states_administrator,
             &env_building_stdio_directory,
         )?;
+
+        if drop_suite {
+            suites_to_be_dropped.push(suite.name.clone());
+        }
     }
 
-    Ok(())
+    Ok(drop_suites(suites, &suites_to_be_dropped))
 }
 
 fn build_environment<'a>(
     suite: &'a Suite,
     mut environment_build_states_administrator: EnvironmentBuildStatesAdministrator<'a>,
     stdio_directory: &Utf8Path,
-) -> Result<EnvironmentBuildStatesAdministrator<'a>> {
-    match suite.environment.build_instructions() {
+) -> Result<(EnvironmentBuildStatesAdministrator<'a>, bool)> {
+    let drop_suite = match suite.environment.build_instructions() {
         Some(build_instructions) => {
             info!("Building environment for suite {}", suite.name);
             environment_build_states_administrator
                 .insert_and_write_atomic(&suite.name, EnvironmentBuildStatus::InProgress)?;
-            environment_build_states_administrator.insert_and_write_atomic(
-                &suite.name,
-                run_environment_build(ChildProcessSupervisor {
-                    command_spec: &build_instructions.command_spec,
-                    stdio_paths: Some(StdioPaths {
-                        stdout: stdio_directory.join(format!("{}.stdout", suite.name)),
-                        stderr: stdio_directory.join(format!("{}.stderr", suite.name)),
-                    }),
-                    timeout: build_instructions.timeout,
-                    termination_flag: &suite.termination_flag,
-                })?,
-            )?;
+            let environment_build_status = run_environment_build(ChildProcessSupervisor {
+                command_spec: &build_instructions.command_spec,
+                stdio_paths: Some(StdioPaths {
+                    stdout: stdio_directory.join(format!("{}.stdout", suite.name)),
+                    stderr: stdio_directory.join(format!("{}.stderr", suite.name)),
+                }),
+                timeout: build_instructions.timeout,
+                termination_flag: &suite.termination_flag,
+            })?;
+            let drop_suite = matches!(environment_build_status, EnvironmentBuildStatus::Failure(_));
+            environment_build_states_administrator
+                .insert_and_write_atomic(&suite.name, environment_build_status)?;
+            drop_suite
         }
         None => {
             debug!("Nothing to do for suite {}", suite.name);
             environment_build_states_administrator
                 .insert_and_write_atomic(&suite.name, EnvironmentBuildStatus::NotNeeded)?;
+            false
         }
-    }
-    Ok(environment_build_states_administrator)
+    };
+    Ok((environment_build_states_administrator, drop_suite))
 }
 
 fn run_environment_build(
     build_process_supervisor: ChildProcessSupervisor,
 ) -> Result<EnvironmentBuildStatus> {
-    match build_process_supervisor
+    let child_process_outcome = match build_process_supervisor
         .run()
-        .context("Environment building failed")?
+        .context("Environment building failed, suite will be dropped")
+        .map_err(log_and_return_error)
     {
+        Ok(child_process_outcome) => child_process_outcome,
+        Err(error) => {
+            return Ok(EnvironmentBuildStatus::Failure(
+                EnvironmentBuildStatusError::Error(format!("{error:?}")),
+            ))
+        }
+    };
+    match child_process_outcome {
         ChildProcessOutcome::Exited(exit_status) => {
             if exit_status.success() {
                 debug!("Environmenent building succeeded");
                 Ok(EnvironmentBuildStatus::Success)
             } else {
-                error!("Environment building not sucessful, suite will most likely not execute");
-                Ok(EnvironmentBuildStatus::Failure)
+                error!("Environment building not sucessful, suite will be dropped");
+                Ok(EnvironmentBuildStatus::Failure(
+                    EnvironmentBuildStatusError::NonZeroExit,
+                ))
             }
         }
         ChildProcessOutcome::TimedOut => {
-            error!("Environment building timed out, suite will most likely not execute");
-            Ok(EnvironmentBuildStatus::Timeout)
+            error!("Environment building timed out, suite will be dropped");
+            Ok(EnvironmentBuildStatus::Failure(
+                EnvironmentBuildStatusError::Timeout,
+            ))
         }
         ChildProcessOutcome::Terminated => {
             bail!("Terminated")

v2/robotmk/src/main.rs

@@ -52,7 +52,7 @@ fn run() -> Result<()> {
     }
 
     info!("Starting environment building");
-    environment::build_environments(&global_config, &suites)?;
+    let suites = environment::build_environments(&global_config, suites)?;
     info!("Environment building finished");
 
     if global_config.termination_flag.should_terminate() {

v2/robotmk/src/results.rs

@@ -70,13 +70,19 @@ impl<'a> EnvironmentBuildStatesAdministrator<'a> {
 #[derive(Serialize)]
 pub enum EnvironmentBuildStatus {
     Success,
-    Failure,
-    Timeout,
+    Failure(EnvironmentBuildStatusError),
     NotNeeded,
     Pending,
     InProgress,
 }
 
+#[derive(Serialize)]
+pub enum EnvironmentBuildStatusError {
+    NonZeroExit,
+    Timeout,
+    Error(String),
+}
+
 #[derive(Serialize)]
 pub struct SuiteExecutionReport {
     pub suite_name: String,