ya-provider clean cmd deletes only exe-unit old cache/work files (#2552)

* ya-provider clean deletes only exe-unit old cache/work files

* ya-provider clean tests

* ya-provider clean help message update

---------

Co-authored-by: Przemyslaw Walski <pwalski@users.noreply.github.com>

.github/workflows/integration-test-hybrid-net.yml

@@ -30,7 +30,7 @@ jobs:
 
   integration-test:
     name: Integration Tests (hybrid-net)
-    runs-on: goth
+    runs-on: [goth, ubuntu-18.04]
     needs: test_check
     defaults:
       run:

.github/workflows/integration-test-nightly.yml

@@ -41,7 +41,7 @@ jobs:
     strategy:
       matrix: ${{ fromJson(needs.prepare-matrix.outputs.matrix-json) }}
       fail-fast: false
-    runs-on: goth
+    runs-on: [goth, ubuntu-18.04]
     name: Integration Tests (nightly) @ ${{ matrix.branch }}
     defaults:
       run:

.github/workflows/integration-test.yml

@@ -30,7 +30,7 @@ jobs:
 
   integration-test:
     name: Integration Tests
-    runs-on: goth
+    runs-on: [goth, ubuntu-18.04]
     needs: test_check
     defaults:
       run:

agent/provider/Cargo.toml

@@ -81,5 +81,6 @@ test-case = "2.1"
 predicates = "2.1"
 serial_test = "0.9"
 shlex = "1.1"
+tempfile = "3"
 
 ya-manifest-test-utils = "0.1"

agent/provider/src/cli/clean.rs

@@ -6,11 +6,11 @@ use crate::startup_config::ProviderConfig;
 #[derive(StructOpt, Clone, Debug)]
 #[structopt(rename_all = "kebab-case")]
 pub struct CleanConfig {
-    /// Expression in the following format:
+    /// Age of files which will be removed expressed in the following format:
     /// <number>P, e.g. 30d
     /// where P: s|m|h|d|w|M|y or empty for days
     #[structopt(default_value = "30d")]
-    pub expr: String,
+    pub age: String,
     /// Perform a dry run
     #[structopt(long)]
     pub dry_run: bool,
@@ -21,7 +21,7 @@ impl CleanConfig {
         let data_dir = config.data_dir.get_or_create()?;
         println!("Using data dir: {}", data_dir.display());
 
-        let freed = clean_provider_dir(&data_dir, &self.expr, true, self.dry_run)?;
+        let freed = clean_provider_dir(&data_dir, &self.age, true, self.dry_run)?;
         let human_freed = bytesize::to_string(freed, false);
 
         if self.dry_run {

agent/provider/src/dir.rs

@@ -1,9 +1,11 @@
-use crate::startup_config::{CERT_DIR, GLOBALS_JSON, HARDWARE_JSON, PRESETS_JSON};
+use crate::startup_config::{GLOBALS_JSON, HARDWARE_JSON, PRESETS_JSON};
 use anyhow::{bail, Result};
 use std::path::Path;
 use std::time::{Duration, SystemTime};
 use walkdir::WalkDir;
 
+/// Cleans Provider DATA_DIR and returns amount of freed disk space (in bytes).
+/// Fails if DATA_DIR does not contain Provider's config files.
 pub fn clean_provider_dir<P: AsRef<Path>, S: AsRef<str>>(
     dir: P,
     expr: S,
@@ -14,15 +16,14 @@ pub fn clean_provider_dir<P: AsRef<Path>, S: AsRef<str>>(
     if check_dir && !is_provider_dir(&dir)? {
         bail!("Not a provider data directory: {}", dir.as_ref().display());
     }
-    Ok(clean_dir(dir, 2, lifetime, dry_run))
+    Ok(clean_data_dir(dir, lifetime, dry_run))
 }
 
 fn is_provider_dir<P: AsRef<Path>>(dir: P) -> Result<bool> {
     let mut files = vec![
         (HARDWARE_JSON, false),
         (PRESETS_JSON, false),
         (GLOBALS_JSON, false),
-        (CERT_DIR, false),
     ];
 
     dir.as_ref()
@@ -40,12 +41,18 @@ fn is_provider_dir<P: AsRef<Path>>(dir: P) -> Result<bool> {
     Ok(files.iter().all(|pair| pair.1))
 }
 
-fn clean_dir<P: AsRef<Path>>(dir: P, min_depth: usize, lifetime: Duration, dry_run: bool) -> u64 {
+fn clean_data_dir<P: AsRef<Path>>(data_dir: P, lifetime: Duration, dry_run: bool) -> u64 {
+    let work_dir = crate::execution::exe_unit_work_dir(&data_dir);
+    let cache_dir = crate::execution::exe_unit_cache_dir(&data_dir);
+    clean_dir(work_dir, lifetime, dry_run) + clean_dir(cache_dir, lifetime, dry_run)
+}
+
+fn clean_dir<P: AsRef<Path>>(dir: P, lifetime: Duration, dry_run: bool) -> u64 {
     let mut dirs = Vec::new();
     let deadline = SystemTime::now() - lifetime;
 
     let total_bytes = WalkDir::new(dir.as_ref())
-        .min_depth(min_depth)
+        .min_depth(1)
         .into_iter()
         .filter_map(|result| result.ok())
         .filter_map(|entry| match entry.metadata() {
@@ -84,3 +91,137 @@ fn clean_dir<P: AsRef<Path>>(dir: P, min_depth: usize, lifetime: Duration, dry_r
 
     total_bytes
 }
+
+#[cfg(test)]
+mod tests {
+    use super::clean_provider_dir;
+    use crate::startup_config::{GLOBALS_JSON, HARDWARE_JSON, PRESETS_JSON};
+    use std::{
+        fs::File,
+        io::Write,
+        path::{Path, PathBuf},
+    };
+
+    #[test]
+    fn test_empty_dir_fail() {
+        let dir = tempfile::tempdir().unwrap().into_path();
+        let expected = anyhow::anyhow!("Not a provider data directory: {}", dir.display());
+        let error = clean_provider_dir(&dir, "1d", true, false);
+        assert_eq!(expected.to_string(), error.err().unwrap().to_string());
+        assert!(dir.exists());
+    }
+
+    #[test]
+    fn test_empty_exe_unit_dir_cleanup() {
+        let dirs = create_data_dir_w_exe_unit();
+        let removed_bytes =
+            clean_provider_dir(dirs.data_dir.dir, "1d", true, false).expect("Is ok");
+
+        assert_eq!(0, removed_bytes);
+        assert!(dirs.data_dir.globals_json.exists());
+        assert!(dirs.data_dir.hardware_json.exists());
+        assert!(dirs.data_dir.presets_json.exists());
+        assert!(dirs.work_dir.exists());
+        assert!(dirs.cache_dir.exists());
+    }
+
+    #[test]
+    fn test_exe_unit_dir_cleanup_of_old_files() {
+        let dirs = create_data_dir_w_exe_unit();
+        let work_dir_file = create_file(&dirs.work_dir, "a.txt", "a");
+        let work_dir_dir = create_dir(&dirs.work_dir, "a");
+        let cache_dir_file = create_file(&dirs.cache_dir, "b.txt", "b");
+        let cache_dir_dir = create_dir(&dirs.cache_dir, "b");
+        let removed_bytes: u64 =
+            clean_provider_dir(dirs.data_dir.dir, "0us", true, false).expect("Is ok");
+
+        assert_eq!(2, removed_bytes);
+        assert!(dirs.data_dir.globals_json.exists());
+        assert!(dirs.data_dir.hardware_json.exists());
+        assert!(dirs.data_dir.presets_json.exists());
+        assert!(dirs.work_dir.exists());
+        assert!(dirs.cache_dir.exists());
+        assert!(!work_dir_dir.exists());
+        assert!(!work_dir_file.exists());
+        assert!(!cache_dir_dir.exists());
+        assert!(!cache_dir_file.exists());
+    }
+
+    #[test]
+    fn test_exe_unit_dir_cleanup_does_not_remove_files() {
+        let dirs = create_data_dir_w_exe_unit();
+        let work_dir_file = create_file(&dirs.work_dir, "a.txt", "a");
+        let work_dir_dir = create_dir(&dirs.work_dir, "a");
+        let cache_dir_file = create_file(&dirs.cache_dir, "b.txt", "b");
+        let cache_dir_dir = create_dir(&dirs.cache_dir, "b");
+        let cache_dir_dir_file = create_file(&cache_dir_dir, "c.txt", "c");
+        let removed_bytes: u64 =
+            clean_provider_dir(dirs.data_dir.dir, "1h", true, false).expect("Is ok");
+
+        assert_eq!(0, removed_bytes);
+        assert!(dirs.data_dir.globals_json.exists());
+        assert!(dirs.data_dir.hardware_json.exists());
+        assert!(dirs.data_dir.presets_json.exists());
+        assert!(dirs.work_dir.exists());
+        assert!(dirs.cache_dir.exists());
+        assert!(!work_dir_dir.exists(), "Empty directory removed.");
+        assert!(work_dir_file.exists());
+        assert!(cache_dir_dir.exists());
+        assert!(cache_dir_file.exists());
+        assert!(cache_dir_dir_file.exists());
+    }
+
+    struct DataDir {
+        dir: PathBuf,
+        hardware_json: PathBuf,
+        presets_json: PathBuf,
+        globals_json: PathBuf,
+    }
+
+    struct ExeUnitDirs {
+        data_dir: DataDir,
+        cache_dir: PathBuf,
+        work_dir: PathBuf,
+    }
+
+    fn create_data_dir_w_exe_unit() -> ExeUnitDirs {
+        let data_dir = create_data_dir();
+        let work_dir = crate::execution::exe_unit_work_dir(&data_dir.dir);
+        std::fs::create_dir_all(&work_dir).unwrap();
+        let cache_dir = crate::execution::exe_unit_cache_dir(&data_dir.dir);
+        std::fs::create_dir_all(&cache_dir).unwrap();
+        ExeUnitDirs {
+            data_dir,
+            cache_dir,
+            work_dir,
+        }
+    }
+
+    fn create_data_dir() -> DataDir {
+        let dir = tempfile::tempdir().unwrap().into_path();
+        let hardware_json = create_file(&dir, HARDWARE_JSON, "a");
+        let presets_json = create_file(&dir, PRESETS_JSON, "b");
+        let globals_json = create_file(&dir, GLOBALS_JSON, "c");
+        DataDir {
+            dir,
+            hardware_json,
+            presets_json,
+            globals_json,
+        }
+    }
+
+    fn create_dir(dir: &Path, name: &str) -> PathBuf {
+        let path = dir.join(name);
+        std::fs::create_dir_all(&path).unwrap();
+        path
+    }
+
+    fn create_file(dir: &Path, name: &str, content: &str) -> PathBuf {
+        let path = dir.join(name);
+        let mut file = File::create(&path).unwrap();
+        if !content.is_empty() {
+            file.write_all(content.as_bytes()).unwrap();
+        }
+        path
+    }
+}

agent/provider/src/execution.rs

@@ -5,6 +5,8 @@ pub use task_runner::{
 
 pub use self::registry::Configuration;
 pub use self::registry::{ExeUnitDesc, ExeUnitsRegistry};
+pub use self::task_runner::exe_unit_cache_dir;
+pub use self::task_runner::exe_unit_work_dir;
 
 mod exeunit_instance;
 mod registry;

agent/provider/src/execution/task_runner.rs

@@ -31,6 +31,10 @@ use crate::market::provider_market::NewAgreement;
 use crate::market::Preset;
 use crate::tasks::{AgreementBroken, AgreementClosed};
 
+const EXE_UNIT_DIR: &str = "exe-unit";
+const WORK_DIR: &str = "work";
+const CACHE_DIR: &str = "cache";
+
 // =========================================== //
 // Public exposed messages
 // =========================================== //
@@ -161,8 +165,8 @@ impl TaskRunner {
         data_dir: P,
     ) -> Result<TaskRunner> {
         let data_dir = data_dir.as_ref();
-        let tasks_dir = data_dir.join("exe-unit").join("work");
-        let cache_dir = data_dir.join("exe-unit").join("cache");
+        let tasks_dir = exe_unit_work_dir(data_dir);
+        let cache_dir = exe_unit_cache_dir(data_dir);
 
         log::debug!("TaskRunner config: {:?}", config);
 
@@ -515,6 +519,16 @@ impl TaskRunner {
     }
 }
 
+pub fn exe_unit_work_dir<P: AsRef<Path>>(data_dir: P) -> PathBuf {
+    let data_dir = data_dir.as_ref();
+    data_dir.join(EXE_UNIT_DIR).join(WORK_DIR)
+}
+
+pub fn exe_unit_cache_dir<P: AsRef<Path>>(data_dir: P) -> PathBuf {
+    let data_dir = data_dir.as_ref();
+    data_dir.join(EXE_UNIT_DIR).join(CACHE_DIR)
+}
+
 fn exe_unit_name_from(agreement: &AgreementView) -> Result<String> {
     let runtime_key_str = "/offer/properties/golem/runtime/name";
     Ok(agreement.pointer_typed::<String>(runtime_key_str)?)

agent/provider/src/startup_config.rs

@@ -236,7 +236,7 @@ pub enum Commands {
     Keystore(KeystoreConfig),
     /// Manage domain whitelist
     Whitelist(WhitelistConfig),
-    /// Clean up disk space
+    /// Free up disk space by removing old exe-unit files
     Clean(CleanConfig),
 }