Stop deleting all plan working directories at scheduler start

.github/workflows/rcc.yaml

@@ -40,7 +40,7 @@ jobs:
             ruby-version: ${{ env.RUBY_VERSION }}
       - run: CGO_ENABLED=0 rake build
       - run: rake test
-      - run: ldd build/linux64/rcc | grep "statically linked"
+      - run: file build/linux64/rcc | grep "statically linked"
       - uses: actions/cache/save@v4
         with:
           key: rcc-${{ env.RCC_TAG }}-${{ env.GO_VERSION }}-${{ env.RUBY_VERSION }}

.github/workflows/system_tests.yaml

@@ -33,12 +33,14 @@ jobs:
 
       - run: mkdir C:\managed_robots
       - run: tar --create -z --directory tests\minimal_suite\ --file C:\managed_robots\minimal_suite.tar.gz *
+      - run: net user "test_user" "uCjV*NRE#XH2a" /add
       - run: cargo test --target=x86_64-pc-windows-gnu --test test_scheduler -- --nocapture --ignored
         env:
           TEST_DIR: C:\test_scheduler
           RCC_BINARY_PATH: C:\windows64\rcc.exe
           MANAGED_ROBOT_ARCHIVE_PATH: C:\managed_robots\minimal_suite.tar.gz
           N_SECONDS_RUN_MAX: 300
+          TEST_USER: test_user
       - uses: actions/upload-artifact@v4
         if: success() || failure()
         with:

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "robotmk"
-version = "3.0.0-alpha-6"
+version = "3.0.0-alpha-7"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

src/bin/scheduler/internal_config.rs

@@ -9,7 +9,7 @@ use robotmk::rf::robot::Robot;
 use robotmk::section::Host;
 use robotmk::session::Session;
 
-use camino::Utf8PathBuf;
+use camino::{Utf8Path, Utf8PathBuf};
 use tokio_util::sync::CancellationToken;
 
 pub struct GlobalConfig {
@@ -80,9 +80,7 @@ pub fn from_external_config(
             plans.push(Plan {
                 id: plan_config.id.clone(),
                 source,
-                working_directory: external_config
-                    .working_directory
-                    .join("plans")
+                working_directory: plans_working_directory(&external_config.working_directory)
                     .join(&plan_config.id),
                 results_file: plan_results_directory(&external_config.results_directory)
                     .join(format!("{}.json", plan_config.id)),
@@ -155,6 +153,10 @@ pub fn sort_plans_by_grouping(plans: &mut [Plan]) {
     });
 }
 
+pub fn plans_working_directory(working_directory: &Utf8Path) -> Utf8PathBuf {
+    working_directory.join("plans")
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/bin/scheduler/setup/general.rs

@@ -1,7 +1,9 @@
 use super::plans_by_sessions;
 use super::rcc::rcc_setup_working_directory;
 use crate::build::environment_building_working_directory;
-use crate::internal_config::{sort_plans_by_grouping, GlobalConfig, Plan, Source};
+use crate::internal_config::{
+    plans_working_directory, sort_plans_by_grouping, GlobalConfig, Plan, Source,
+};
 
 use anyhow::{anyhow, Context, Result as AnyhowResult};
 use camino::{Utf8Path, Utf8PathBuf};
@@ -16,10 +18,21 @@ pub fn setup(
     global_config: &GlobalConfig,
     plans: Vec<Plan>,
 ) -> Result<(Vec<Plan>, Vec<SetupFailure>), Terminate> {
-    if global_config.working_directory.exists() {
-        remove_dir_all(&global_config.working_directory)?;
-    }
     create_dir_all(&global_config.working_directory)?;
+    create_dir_all(plans_working_directory(&global_config.working_directory))?;
+    for working_sub_dir in [
+        rcc_setup_working_directory(&global_config.working_directory),
+        environment_building_working_directory(&global_config.working_directory),
+    ] {
+        if working_sub_dir.exists() {
+            remove_dir_all(&working_sub_dir)?;
+        }
+        create_dir_all(&working_sub_dir)?;
+    }
+    clean_up_file_system_entries(
+        plans.iter().map(|plan| &plan.working_directory),
+        top_level_directories(&plans_working_directory(&global_config.working_directory))?.iter(),
+    )?;
     if global_config.managed_directory.exists() {
         remove_dir_all(&global_config.managed_directory)?;
     }
@@ -73,10 +86,27 @@ fn setup_plans_working_directory(plans: Vec<Plan>) -> (Vec<Plan>, Vec<SetupFailu
         }
         #[cfg(windows)]
         {
-            use super::windows_permissions::grant_full_access;
+            use super::windows_permissions::{grant_full_access, reset_access};
             use log::info;
             use robotmk::session::Session;
 
+            info!("Resetting permissions for {}", &plan.working_directory);
+            if let Err(e) = reset_access(&plan.working_directory) {
+                let error = anyhow!(e);
+                error!(
+                    "Plan {}: Failed to reset permissions for working directory. \
+                     Plan won't be scheduled.
+                     Error: {error:?}",
+                    plan.id
+                );
+                failures.push(SetupFailure {
+                    plan_id: plan.id.clone(),
+                    summary: "Failed to reset permissions for working directory".to_string(),
+                    details: format!("{error:?}"),
+                });
+                continue;
+            };
+
             if let Session::User(user_session) = &plan.session {
                 info!(
                     "Granting full access for {} to user `{}`.",
@@ -331,47 +361,62 @@ fn clean_up_results_directory(
     for path in top_level_files(&global_config.results_directory)? {
         remove_file(path)?;
     }
-    clean_up_plan_results_directory(
-        &plan_results_directory(&global_config.results_directory),
-        plans,
+    clean_up_file_system_entries(
+        plans.iter().map(|plan| &plan.results_file),
+        top_level_files(&plan_results_directory(&global_config.results_directory))?.iter(),
     )?;
     Ok(results_directory_lock.release()?)
 }
 
-fn clean_up_plan_results_directory(
-    plan_results_directory: &Utf8Path,
-    plans: &[Plan],
-) -> AnyhowResult<()> {
-    let result_files_to_keep =
-        HashSet::<Utf8PathBuf>::from_iter(plans.iter().map(|plan| plan.results_file.clone()));
-    let currently_present_result_files =
-        HashSet::<Utf8PathBuf>::from_iter(top_level_files(plan_results_directory)?);
-    for path in currently_present_result_files.difference(&result_files_to_keep) {
-        remove_file(path)?;
-    }
-    Ok(())
+fn top_level_directories(directory: &Utf8Path) -> AnyhowResult<Vec<Utf8PathBuf>> {
+    Ok(top_level_directory_entries(directory)?
+        .into_iter()
+        .filter(|path| path.is_dir())
+        .collect())
 }
 
 fn top_level_files(directory: &Utf8Path) -> AnyhowResult<Vec<Utf8PathBuf>> {
-    let mut result_files = vec![];
+    Ok(top_level_directory_entries(directory)?
+        .into_iter()
+        .filter(|path| path.is_file())
+        .collect())
+}
 
-    for dir_entry in directory.read_dir_utf8().context(format!(
-        "Failed to read entries of results directory {directory}",
-    ))? {
-        let dir_entry = dir_entry.context(format!(
-            "Failed to read entries of results directory {directory}",
-        ))?;
-        if dir_entry
-            .file_type()
-            .context(format!(
-                "Failed to determine file type of {}",
-                dir_entry.path()
-            ))?
-            .is_file()
-        {
-            result_files.push(dir_entry.path().to_path_buf())
-        }
+fn top_level_directory_entries(directory: &Utf8Path) -> AnyhowResult<Vec<Utf8PathBuf>> {
+    let mut entries = vec![];
+
+    for dir_entry in directory
+        .read_dir_utf8()
+        .context(format!("Failed to read entries of directory {directory}",))?
+    {
+        entries.push(
+            dir_entry
+                .context(format!("Failed to read entries of directory {directory}",))?
+                .path()
+                .to_path_buf(),
+        )
     }
 
-    Ok(result_files)
+    Ok(entries)
+}
+
+fn clean_up_file_system_entries<P>(
+    entries_to_keep: impl IntoIterator<Item = P>,
+    currently_present_entries: impl IntoIterator<Item = P>,
+) -> AnyhowResult<()>
+where
+    P: AsRef<Utf8Path>,
+    P: std::cmp::Eq,
+    P: std::hash::Hash,
+{
+    for entry in HashSet::<P>::from_iter(currently_present_entries)
+        .difference(&HashSet::from_iter(entries_to_keep))
+    {
+        if entry.as_ref().is_file() {
+            remove_file(entry)?
+        } else {
+            remove_dir_all(entry)?
+        }
+    }
+    Ok(())
 }

src/bin/scheduler/setup/windows_permissions.rs

@@ -86,6 +86,14 @@ pub fn grant_full_access(user: &str, target_path: &Utf8Path) -> anyhow::Result<(
     })
 }
 
+pub fn reset_access(target_path: &Utf8Path) -> anyhow::Result<()> {
+    let arguments = [target_path.as_ref(), "/reset", "/T"];
+    run_icacls_command(arguments).map_err(|e| {
+        let message = format!("Resetting permissions of {target_path} failed");
+        e.context(message)
+    })
+}
+
 pub fn adjust_rcc_file_permissions(
     rcc_config: &RCCConfig,
     rcc_plans: Vec<Plan>,

tests/test_scheduler.rs

@@ -29,7 +29,28 @@ use walkdir::WalkDir;
 #[ignore]
 async fn test_scheduler() -> AnyhowResult<()> {
     let test_dir = Utf8PathBuf::from(var("TEST_DIR")?);
+    let unconfigured_plan_working_dir = test_dir
+        .join("working")
+        .join("plans")
+        .join("should_be_removed_during_scheduler_setup");
+    let configured_plan_working_dir = test_dir.join("working").join("plans").join("rcc_headless");
+    let configured_plan_previous_execution_dir =
+        configured_plan_working_dir.join("should_still_exist_after_scheduler_run");
     create_dir_all(&test_dir)?;
+    create_dir_all(&unconfigured_plan_working_dir)?;
+    create_dir_all(&configured_plan_previous_execution_dir)?;
+    #[cfg(windows)]
+    let test_user = var("TEST_USER")?;
+    #[cfg(windows)]
+    {
+        grant_full_access(&test_user, &configured_plan_working_dir).await?;
+        assert_permissions(
+            &configured_plan_working_dir,
+            &format!("{test_user}:(OI)(CI)(F)"),
+        )
+        .await?;
+    }
+
     #[cfg(windows)]
     let current_user_name = var("UserName")?;
     let config = create_config(
@@ -59,6 +80,12 @@ async fn test_scheduler() -> AnyhowResult<()> {
         &current_user_name,
     )
     .await?;
+    assert!(!unconfigured_plan_working_dir.exists());
+    assert!(configured_plan_previous_execution_dir.is_dir());
+    #[cfg(windows)]
+    assert!(!get_permissions(&configured_plan_working_dir)
+        .await?
+        .contains(&test_user));
     assert_results_directory(&config.results_directory);
     assert_managed_directory(
         &config.managed_directory,
@@ -78,6 +105,16 @@ async fn test_scheduler() -> AnyhowResult<()> {
     Ok(())
 }
 
+#[cfg(windows)]
+async fn grant_full_access(user: &str, target_path: &Utf8Path) -> tokio::io::Result<()> {
+    let mut icacls_command = Command::new("icacls.exe");
+    icacls_command
+        .arg(target_path)
+        .args(["/grant", &format!("{user}:(OI)(CI)F"), "/T"]);
+    assert!(icacls_command.output().await?.status.success());
+    Ok(())
+}
+
 fn create_custom_rcc_profile(test_dir: &Utf8Path) -> AnyhowResult<CustomRCCProfileConfig> {
     let rcc_profile_path = test_dir.join("rcc_profile.yaml");
     write(
@@ -479,10 +516,16 @@ async fn assert_working_directory(
 }
 
 #[cfg(windows)]
-async fn assert_permissions(path: impl AsRef<OsStr>, permissions: &str) -> AnyhowResult<()> {
+async fn get_permissions(path: impl AsRef<OsStr>) -> AnyhowResult<String> {
     let mut icacls_command = Command::new("icacls.exe");
     icacls_command.arg(path);
-    assert!(String::from_utf8(icacls_command.output().await?.stdout)?.contains(permissions));
+    let permissions = String::from_utf8(icacls_command.output().await?.stdout)?;
+    Ok(permissions)
+}
+
+#[cfg(windows)]
+async fn assert_permissions(path: impl AsRef<OsStr>, permissions: &str) -> AnyhowResult<()> {
+    assert!(get_permissions(path).await?.contains(permissions));
     Ok(())
 }