public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH proxmox-datacenter-manager 03/15] task cache: add basic test for TaskCache
Date: Tue, 28 Jan 2025 13:25:08 +0100	[thread overview]
Message-ID: <20250128122520.167796-4-l.wagner@proxmox.com> (raw)
In-Reply-To: <20250128122520.167796-1-l.wagner@proxmox.com>

This one helps us to verify that we did not break the core functionality
in the upcoming changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/task_cache.rs | 100 +++++++++++++++++++++++++++++++--------
 1 file changed, 79 insertions(+), 21 deletions(-)

diff --git a/server/src/task_cache.rs b/server/src/task_cache.rs
index 7ded540..211beb4 100644
--- a/server/src/task_cache.rs
+++ b/server/src/task_cache.rs
@@ -25,8 +25,13 @@ pub async fn get_tasks(max_age: i64, filters: TaskFilters) -> Result<Vec<TaskLis
 
     let mut all_tasks = Vec::new();
 
+    let api_uid = pdm_config::api_user()?.uid;
+    let api_gid = pdm_config::api_group()?.gid;
+
+    let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
+
     let cache_path = Path::new(pdm_buildcfg::PDM_CACHE_DIR).join("taskcache.json");
-    let mut cache = TaskCache::new(cache_path)?;
+    let mut cache = TaskCache::new(cache_path, file_options)?;
 
     // Force a refresh for all tasks of a remote if a task is finished.
     // Not super nice, but saves us from persisting finished tasks. Also,
@@ -246,27 +251,30 @@ struct TaskCache {
 
     /// File-location at which the cached tasks are stored.
     cachefile_path: PathBuf,
+
+    /// File mode/owner/group for the cache file.
+    cachefile_options: CreateOptions,
 }
 
 impl TaskCache {
     /// Create a new tasks cache instance by loading
     /// the cache from disk.
-    fn new(cachefile_path: PathBuf) -> Result<Self, Error> {
+    fn new(cachefile_path: PathBuf, cachefile_options: CreateOptions) -> Result<Self, Error> {
         Ok(Self {
-            content: Self::load_content()?,
+            content: Self::load_content(&cachefile_path)?,
             new_or_updated: Default::default(),
             dirty: false,
             cachefile_path,
+            cachefile_options,
         })
     }
 
     /// Load the task cache contents from disk.
-    fn load_content() -> Result<TaskCacheContent, Error> {
-        let taskcache_path = Path::new(pdm_buildcfg::PDM_CACHE_DIR).join("taskcache.json");
-        let content = proxmox_sys::fs::file_read_optional_string(taskcache_path)?;
+    fn load_content(path: &Path) -> Result<TaskCacheContent, Error> {
+        let content = proxmox_sys::fs::file_read_optional_string(path)?;
 
         let content = if let Some(content) = content {
-            serde_json::from_str(&content)?
+            serde_json::from_str(&content).unwrap_or_default()
         } else {
             Default::default()
         };
@@ -293,7 +301,7 @@ impl TaskCache {
         let _guard = self.lock(Duration::from_secs(5))?;
 
         // Read content again, in case somebody has changed it in the meanwhile
-        let mut content = Self::load_content()?;
+        let mut content = Self::load_content(&self.cachefile_path)?;
 
         for (remote_name, entry) in self.new_or_updated.remote_tasks.drain() {
             if let Some(existing_entry) = content.remote_tasks.get_mut(&remote_name) {
@@ -308,12 +316,12 @@ impl TaskCache {
 
         let bytes = serde_json::to_vec_pretty(&content)?;
 
-        let api_uid = pdm_config::api_user()?.uid;
-        let api_gid = pdm_config::api_group()?.gid;
-
-        let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
-
-        proxmox_sys::fs::replace_file(&self.cachefile_path, &bytes, file_options, true)?;
+        proxmox_sys::fs::replace_file(
+            &self.cachefile_path,
+            &bytes,
+            self.cachefile_options.clone(),
+            true,
+        )?;
 
         self.dirty = false;
 
@@ -358,22 +366,23 @@ impl TaskCache {
     // without a lock, since the cache file is replaced atomically
     // when updating.
     fn lock(&self, duration: Duration) -> Result<File, Error> {
-        let api_uid = pdm_config::api_user()?.uid;
-        let api_gid = pdm_config::api_group()?.gid;
-
-        let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
-        proxmox_sys::fs::open_file_locked(self.lockfile_path(), duration, true, file_options)
+        proxmox_sys::fs::open_file_locked(
+            self.lockfile_path(),
+            duration,
+            true,
+            self.cachefile_options.clone(),
+        )
     }
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize)]
 /// Per-remote entry in the task cache.
 struct TaskCacheEntry {
     timestamp: i64,
     tasks: Vec<TaskListItem>,
 }
 
-#[derive(Default, Serialize, Deserialize)]
+#[derive(Debug, Default, Serialize, Deserialize)]
 /// Content of the task cache file.
 struct TaskCacheContent {
     remote_tasks: HashMap<String, TaskCacheEntry>,
@@ -522,3 +531,52 @@ pub fn tasktype(status: &str) -> TaskStateType {
         TaskStateType::Error
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_support::temp::NamedTempFile;
+
+    #[test]
+    fn basic_task_cache() -> Result<(), Error> {
+        let options = CreateOptions::new()
+            .owner(nix::unistd::Uid::effective())
+            .group(nix::unistd::Gid::effective())
+            .perm(nix::sys::stat::Mode::from_bits_truncate(0o600));
+
+        let temp_file = NamedTempFile::new(options.clone())?;
+
+        let mut cache = TaskCache::new(temp_file.path().into(), options.clone())?;
+
+        let mut tasks = Vec::new();
+
+        let now = proxmox_time::epoch_i64();
+        for i in (0..20).rev() {
+            let upid: PveUpid =
+                "UPID:pve-node:0000C530:001C9BEC:677E934A:stopall::root@pam:".parse()?;
+
+            tasks.push(TaskListItem {
+                upid: upid.to_string(),
+                node: upid.node,
+                pid: upid.pid as i64,
+                pstart: upid.pstart,
+                starttime: now - 10 * i,
+                worker_type: upid.worker_type,
+                worker_id: upid.worker_id,
+                user: upid.auth_id,
+                endtime: None,
+                status: None,
+            });
+        }
+
+        cache.set_tasks("some-remote", tasks.clone(), now);
+        cache.save()?;
+
+        let cache = TaskCache::new(temp_file.path().into(), options)?;
+
+        let res = cache.get_tasks("some-remote", now, 10000).unwrap();
+        assert_eq!(tasks, res);
+
+        Ok(())
+    }
+}
-- 
2.39.5



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


  parent reply	other threads:[~2025-01-28 12:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28 12:25 [pdm-devel] [PATCH proxmox-datacenter-manager 00/15] change task cache mechanism from time-based to max-size FIFO Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 01/15] pdm-api-types: derive Debug and PartialEq for TaskListItem Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 02/15] test support: add NamedTempFile helper Lukas Wagner
2025-01-28 12:25 ` Lukas Wagner [this message]
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 04/15] task cache: remove max-age machanism Lukas Wagner
2025-01-29 18:27   ` Thomas Lamprecht
2025-01-30  8:01     ` Lukas Wagner
2025-01-30 16:06       ` Thomas Lamprecht
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 05/15] task cache: add FIFO cache replacement policy Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 06/15] remote tasks: move to dir based module Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 07/15] task cache: move to its own submodule Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 08/15] task cache: fetch every 5mins, requesting only missing tasks Lukas Wagner
2025-01-31 13:42   ` Wolfgang Bumiller
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 09/15] remote tasks: return tasks in stable order Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 10/15] remote tasks: allow to force-fetch latest tasks Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 11/15] fake remote: add missing fields to make the debug feature compile again Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 12/15] fake remote: generate fake task data Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 13/15] task cache: tests: improve test coverage Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 14/15] remote tasks: fix unused variable warning Lukas Wagner
2025-01-28 12:25 ` [pdm-devel] [PATCH proxmox-datacenter-manager 15/15] remote-tasks: restrict function visibility Lukas Wagner
2025-01-31  9:35 ` [pdm-devel] [PATCH proxmox-datacenter-manager 00/15] change task cache mechanism from time-based to max-size FIFO Lukas Wagner
2025-01-31 13:36   ` Wolfgang Bumiller
2025-01-31 13:51     ` Wolfgang Bumiller
2025-02-05 15:34   ` Thomas Lamprecht
2025-02-06 10:13     ` Lukas Wagner
2025-02-12  9:19       ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250128122520.167796-4-l.wagner@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal