[RFC datacenter-manager 0/4] add generic, per-remote (and global) cache for remote API responses

[RFC datacenter-manager 0/4] add generic, per-remote (and global) cache for remote API responses

[Lukas Wagner]

The main intention is to avoid a sprawl of different caching approaches by
establishing a simple, easy to use cache implementation that can be used to
persistently cache API responses from remotes (and derived aggregations).

Open questions: 
  - is the per-namespace lock too coarse? Should we rather lock
    per key? Anyways, one should not hold the lock during longer periods of time
    (e.g. when doing API requests), so the namespace-level lock seemed fine to me.
    A per-namespace (so, per-remote) lock is nicer when one wants to update several
    keys in one go.

  - Base directory for cache, currently it is
    /var/cache/proxmox-datacenter-manager/cache
    But this seems both redundant and generic to me, so maybe
    'api-cache'?

  - Went with a max_age param on `get` instead of a `set` with an expiry time,
    I think it's quite common to have cache readers with different requirements
    to value freshness, so this might be a better fit.
    Also, we use the max-age mechanism in the API already, so this
    is a seamless fit then. Does this make sense? Or this we rather have
    redis-style `set` with expiry time/TTL?


The `namespaced_cache` module is pretty generic and can be moved to proxmox.git
(maybe in proxmox-shared-cache) once it has sufficiently stabilized.


proxmox-datacenter-manager:

Lukas Wagner (4):
  add persistent, generic, namespaced key-value cache implementation
  add pdm_cache cache as a specialized wrapper around the namespaced
    cache
  api: resources: subscriptions: switch over to pdm_cache
  remote-updates: switch over to pdm_cache

 Cargo.toml                                    |   1 +
 server/Cargo.toml                             |   2 +
 server/src/api/resources.rs                   |  82 ++---
 .../bin/proxmox-datacenter-privileged-api.rs  |   7 +
 server/src/lib.rs                             |   2 +
 server/src/namespaced_cache.rs                | 324 ++++++++++++++++++
 server/src/pdm_cache.rs                       |  69 ++++
 server/src/remote_updates.rs                  |  52 +--
 8 files changed, 452 insertions(+), 87 deletions(-)
 create mode 100644 server/src/namespaced_cache.rs
 create mode 100644 server/src/pdm_cache.rs


Summary over all repositories:
  8 files changed, 452 insertions(+), 87 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH datacenter-manager 1/4] add persistent, generic, namespaced key-value cache implementation

[Lukas Wagner]

Namespaces map to directories inside a base directory. Cache contents
are stored as a single JSON-file per key inside the namespace directory.

Value expiry is implemented via a max_age parameter when retrieving
values. Callers might have different requirements to the freshness of
entries, so this seemed a better fit than statically setting an expiry
time when *setting* the value.

The current implementation is pretty naive about namespace names and key
names; these must come from verified, path-safe identifiers, as they are
used directly as components of the path of the resulting directories and
JSON files. This should be fixed before using this implementation
elsewhere.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    This module might be well suited to be moved to proxmox.git when it has
    stabilized enough. Kept here for now to ease development.

 Cargo.toml                     |   1 +
 server/Cargo.toml              |   2 +
 server/src/lib.rs              |   1 +
 server/src/namespaced_cache.rs | 324 +++++++++++++++++++++++++++++++++
 4 files changed, 328 insertions(+)
 create mode 100644 server/src/namespaced_cache.rs

diff --git a/Cargo.toml b/Cargo.toml
index 9806a4f0..5cf05b41 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -125,6 +125,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_cbor = "0.11.1"
 serde_json = "1.0"
 serde_plain = "1"
+tempfile = "3.15"
 thiserror = "1.0"
 tokio = "1.6"
 tracing = "0.1"
diff --git a/server/Cargo.toml b/server/Cargo.toml
index 3f185bbc..6d6dd583 100644
--- a/server/Cargo.toml
+++ b/server/Cargo.toml
@@ -29,6 +29,8 @@ openssl.workspace = true
 percent-encoding.workspace = true
 serde.workspace = true
 serde_json.workspace = true
+tempfile.workspace = true
+thiserror.workspace = true
 tokio = { workspace = true, features = [ "fs", "io-util", "io-std", "macros", "net", "parking_lot", "process", "rt", "rt-multi-thread", "signal", "time" ] }
 tracing.workspace = true
 url.workspace = true
diff --git a/server/src/lib.rs b/server/src/lib.rs
index 5ed10d69..0b7642ab 100644
--- a/server/src/lib.rs
+++ b/server/src/lib.rs
@@ -7,6 +7,7 @@ pub mod context;
 pub mod env;
 pub mod jobstate;
 pub mod metric_collection;
+pub mod namespaced_cache;
 pub mod parallel_fetcher;
 pub mod remote_cache;
 pub mod remote_tasks;
diff --git a/server/src/namespaced_cache.rs b/server/src/namespaced_cache.rs
new file mode 100644
index 00000000..89ed5b7f
--- /dev/null
+++ b/server/src/namespaced_cache.rs
@@ -0,0 +1,324 @@
+//! Generic namespaced cache implementation with optional value expiry.
+//!
+//! NOTE:
+//! The current implementation is pretty naive about namespace names and key
+//! names; these must come from verified, path-safe identifiers, as they are
+//! used directly as components of the path of the resulting directories and
+//! JSON files. This should be fixed before using this implementation
+//! elsewhere.
+
+use std::fs::File;
+use std::io::ErrorKind;
+use std::path::{Path, PathBuf};
+use std::time::Duration;
+
+use proxmox_sys::fs::CreateOptions;
+use serde::{de::DeserializeOwned, Deserialize, Serialize};
+
+#[derive(thiserror::Error, Debug)]
+pub enum CacheError {
+    #[error("IO error: {0}")]
+    Io(#[from] std::io::Error),
+
+    #[error("serialization error: {0}")]
+    Serde(#[from] serde_json::Error),
+
+    #[error("error: {0}")]
+    Other(#[from] anyhow::Error),
+}
+
+#[derive(Serialize, Deserialize, Debug)]
+struct CacheEntry<T> {
+    timestamp: i64,
+    value: T,
+}
+
+impl<T> CacheEntry<T> {
+    fn is_expired(&self, now: i64, max_age: i64) -> bool {
+        if max_age == 0 {
+            return true;
+        }
+
+        let diff = now - self.timestamp;
+        diff >= max_age || diff < 0
+    }
+}
+
+pub struct NamespacedCache {
+    base_path: PathBuf,
+    file_options: CreateOptions,
+    dir_options: CreateOptions,
+}
+
+impl NamespacedCache {
+    /// Create a new cache instance.
+    pub fn new<P: AsRef<Path>>(
+        base_path: P,
+        dir_options: CreateOptions,
+        file_options: CreateOptions,
+    ) -> Self {
+        Self {
+            base_path: base_path.as_ref().into(),
+            dir_options,
+            file_options,
+        }
+    }
+
+    /// Lock a namespace for writing.
+    pub fn write(
+        &self,
+        namespace: &str,
+        timeout: Duration,
+    ) -> Result<WritableCacheNamespace, CacheError> {
+        let path = get_lockfile(&self.base_path, namespace);
+
+        let lock = proxmox_sys::fs::open_file_locked(&path, timeout, true, self.file_options)?;
+
+        Ok(WritableCacheNamespace {
+            _lock: lock,
+            namespace: namespace.to_string(),
+            base_path: self.base_path.clone(),
+            dir_options: self.dir_options,
+            file_options: self.file_options,
+        })
+    }
+
+    /// Lock a namespace for reading.
+    pub fn read(
+        &self,
+        namespace: &str,
+        timeout: Duration,
+    ) -> Result<ReadableCacheNamespace, CacheError> {
+        let path = get_lockfile(&self.base_path, namespace);
+
+        let lock = proxmox_sys::fs::open_file_locked(&path, timeout, false, self.file_options)?;
+
+        Ok(ReadableCacheNamespace {
+            _lock: lock,
+            namespace: namespace.to_string(),
+            base_path: self.base_path.clone(),
+        })
+    }
+}
+
+/// A readable cache namespace.
+pub struct ReadableCacheNamespace {
+    _lock: File,
+    namespace: String,
+    base_path: PathBuf,
+}
+
+impl ReadableCacheNamespace {
+    /// Read a value from the cache.
+    ///
+    /// # Errors:
+    ///   - The file associated with this key could not be read
+    ///   - The file could not be deserialized (e.g. invalid format)
+    pub fn get<T: Serialize + DeserializeOwned>(&self, key: &str) -> Result<Option<T>, CacheError> {
+        get_impl(&self.base_path, &self.namespace, key, None)
+    }
+
+    /// Read a value from the cache, given a maximum age of the cache entry.
+    ///
+    /// # Errors:
+    ///   - The file associated with this key could not be read
+    ///   - The file could not be deserialized (e.g. invalid format)
+    pub fn get_with_max_age<T: Serialize + DeserializeOwned>(
+        &self,
+        key: &str,
+        max_age: i64,
+    ) -> Result<Option<T>, CacheError> {
+        get_impl(&self.base_path, &self.namespace, key, Some(max_age))
+    }
+}
+
+/// A writable cache namespace.
+pub struct WritableCacheNamespace {
+    _lock: File,
+    namespace: String,
+    base_path: PathBuf,
+    dir_options: CreateOptions,
+    file_options: CreateOptions,
+}
+
+impl WritableCacheNamespace {
+    /// Remote a cache entry.
+    ///
+    /// This returns `Ok(())` if the key does not exist.
+    ///
+    /// # Errors:
+    ///   - The file could not be deleted due to insufficient privileges.
+    pub fn remove(&self, key: &str) -> Result<(), CacheError> {
+        let path = get_path(&self.base_path, &self.namespace, key);
+
+        if let Err(err) = std::fs::remove_file(path) {
+            if err.kind() == ErrorKind::NotFound {
+                return Ok(());
+            }
+
+            return Err(err.into());
+        }
+
+        Ok(())
+    }
+
+    /// Set a cache entry.
+    ///
+    /// # Errors
+    ///   - `value` could not be serialized
+    ///   - The namespace directory could not be created
+    ///   - The cache file could not be written to or atomically replaced
+    pub fn set<T: Serialize + DeserializeOwned>(
+        &self,
+        key: &str,
+        entry: &T,
+    ) -> Result<(), CacheError> {
+        self.set_with_timestamp(key, entry, proxmox_time::epoch_i64())
+    }
+
+    /// Set a cache entry with an explicitly provided timestamp.
+    ///
+    /// # Errors
+    ///   - `value` could not be serialized
+    ///   - The namespace directory could not be created
+    ///   - The cache file could not be written to or atomically replaced
+    pub fn set_with_timestamp<T: Serialize + DeserializeOwned>(
+        &self,
+        key: &str,
+        value: &T,
+        timestamp: i64,
+    ) -> Result<(), CacheError> {
+        let path = get_path(&self.base_path, &self.namespace, key);
+
+        proxmox_sys::fs::create_path(
+            path.parent().unwrap(),
+            Some(self.dir_options),
+            Some(self.dir_options),
+        )?;
+
+        let entry = CacheEntry { timestamp, value };
+
+        let data = serde_json::to_vec(&entry)?;
+        proxmox_sys::fs::replace_file(path, &data, self.file_options, true)?;
+
+        Ok(())
+    }
+
+    /// Read a value from the cache.
+    ///
+    /// # Errors:
+    ///   - The file associated with this key could not be read
+    ///   - The file could not be deserialized (e.g. invalid format)
+    pub fn get<T: Serialize + DeserializeOwned>(&self, key: &str) -> Result<Option<T>, CacheError> {
+        get_impl(&self.base_path, &self.namespace, key, None)
+    }
+
+    /// Read a value from the cache, given a maximum age of the cache entry.
+    ///
+    /// # Errors:
+    ///   - The file associated with this key could not be read
+    ///   - The file could not be deserialized (e.g. invalid format)
+    pub fn get_with_max_age<T: Serialize + DeserializeOwned>(
+        &self,
+        key: &str,
+        max_age: i64,
+    ) -> Result<Option<T>, CacheError> {
+        get_impl(&self.base_path, &self.namespace, key, Some(max_age))
+    }
+}
+
+fn get_impl<T: Serialize + DeserializeOwned>(
+    base: &Path,
+    namespace: &str,
+    key: &str,
+    max_age: Option<i64>,
+) -> Result<Option<T>, CacheError> {
+    let path = get_path(base, namespace, key);
+    let content = proxmox_sys::fs::file_read_optional_string(path)?;
+
+    if let Some(content) = content {
+        let val = serde_json::from_str::<CacheEntry<T>>(&content)?;
+
+        if let Some(max_age) = max_age {
+            if val.is_expired(proxmox_time::epoch_i64(), max_age) {
+                return Ok(None);
+            }
+        }
+        return Ok(Some(val.value));
+    }
+
+    return Ok(None);
+}
+
+fn get_path(base: &Path, namespace: &str, key: &str) -> PathBuf {
+    let mut path = base.join(namespace).join(key);
+    path.set_extension("json");
+    path
+}
+
+fn get_lockfile(base: &Path, namespace: &str) -> PathBuf {
+    let path = base.join(format!(".{namespace}.lock"));
+    path
+}
+
+#[cfg(test)]
+mod tests {
+    use tempfile::TempDir;
+
+    use super::*;
+
+    fn make_cache() -> (TempDir, NamespacedCache) {
+        let dir = tempfile::tempdir().unwrap();
+
+        let cache = NamespacedCache::new(dir.as_ref(), CreateOptions::new(), CreateOptions::new());
+
+        (dir, cache)
+    }
+
+    #[test]
+    fn test_cache() {
+        let (_dir, cache) = make_cache();
+
+        let write_guard = cache.write("remote-a", Duration::from_secs(1)).unwrap();
+        write_guard.set("val1", &1).unwrap();
+        write_guard.set("val2", &1).unwrap();
+
+        assert_eq!(write_guard.get::<i32>("val1").unwrap().unwrap(), 1);
+
+        write_guard.remove("val1").unwrap();
+        assert!(write_guard.get::<String>("val1").unwrap().is_none());
+
+        drop(write_guard);
+
+        let read_guard = cache.read("remote-a", Duration::from_secs(1)).unwrap();
+
+        assert_eq!(read_guard.get::<i32>("val2").unwrap().unwrap(), 1);
+    }
+
+    #[test]
+    fn test_delete_nonexisting() {
+        let (_dir, cache) = make_cache();
+
+        let a = cache.write("remote-a", Duration::from_secs(1)).unwrap();
+        a.set("val", &1).unwrap();
+
+        // Deleting a key that does not exist is okay and should not error.
+        assert!(a.remove("val").is_ok());
+    }
+
+    #[test]
+    fn test_expiration() {
+        let entry = CacheEntry {
+            value: (),
+            timestamp: 1000,
+        };
+
+        assert!(!entry.is_expired(1000, 100));
+        assert!(!entry.is_expired(1099, 100));
+        assert!(entry.is_expired(1100, 100));
+        assert!(entry.is_expired(1101, 100));
+
+        // if max-age is 0, the entry is never fresh
+        assert!(entry.is_expired(1000, 0));
+    }
+}
-- 
2.47.3

[PATCH datacenter-manager 2/4] add pdm_cache cache as a specialized wrapper around the namespaced cache

[Lukas Wagner]

This is a thin wrapper around the previously introduced namespaced
key-value cache, but introducing PDM-specific concepts.

Instead of the higher-level read/write methods for locking a namespace,
this wrapper provides {read,write}_remote and {read,write}_global, for
accessing remote-specific and globally cached values. The
cache-namespaces are 'global' and 'remote-<remote-name>'.

The base directory for the cache is /var/cache/proxmox-datacenter-manager/cache

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Not sure about the base directory /var/cache/proxmox-datacenter-manager/cache
    
    Maybe 'api-cache' could be a nicer fit, since realistically, we probably
    only ever cache API responses and aggregations thereof?

 .../bin/proxmox-datacenter-privileged-api.rs  |  7 ++
 server/src/lib.rs                             |  1 +
 server/src/pdm_cache.rs                       | 69 +++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 server/src/pdm_cache.rs

diff --git a/server/src/bin/proxmox-datacenter-privileged-api.rs b/server/src/bin/proxmox-datacenter-privileged-api.rs
index 6b490f2b..6e8ba611 100644
--- a/server/src/bin/proxmox-datacenter-privileged-api.rs
+++ b/server/src/bin/proxmox-datacenter-privileged-api.rs
@@ -102,6 +102,13 @@ fn create_directories() -> Result<(), Error> {
         0o755,
     )?;
 
+    pdm_config::setup::mkdir_perms(
+        concat!(pdm_buildcfg::PDM_CACHE_DIR_M!(), "/cache"),
+        api_user.uid,
+        api_user.gid,
+        0o755,
+    )?;
+
     server::jobstate::create_jobstate_dir()?;
 
     Ok(())
diff --git a/server/src/lib.rs b/server/src/lib.rs
index 0b7642ab..5e8c0b64 100644
--- a/server/src/lib.rs
+++ b/server/src/lib.rs
@@ -9,6 +9,7 @@ pub mod jobstate;
 pub mod metric_collection;
 pub mod namespaced_cache;
 pub mod parallel_fetcher;
+pub mod pdm_cache;
 pub mod remote_cache;
 pub mod remote_tasks;
 pub mod remote_updates;
diff --git a/server/src/pdm_cache.rs b/server/src/pdm_cache.rs
new file mode 100644
index 00000000..a7370632
--- /dev/null
+++ b/server/src/pdm_cache.rs
@@ -0,0 +1,69 @@
+use std::{
+    path::{Path, PathBuf},
+    sync::LazyLock,
+    time::Duration,
+};
+
+use nix::sys::stat::Mode;
+use proxmox_sys::fs::CreateOptions;
+
+use crate::namespaced_cache::{
+    CacheError, NamespacedCache, ReadableCacheNamespace, WritableCacheNamespace,
+};
+
+static CACHE_INSTANCE: LazyLock<PdmCache> = LazyLock::new(|| {
+    let file_options = proxmox_product_config::default_create_options();
+    let dir_options = file_options.perm(Mode::from_bits_truncate(0o750));
+
+    PdmCache::new(
+        // FIXME: `/cache` seems slightly redundant, come up with something else...
+        PathBuf::from(concat!(pdm_buildcfg::PDM_CACHE_DIR_M!(), "/cache")),
+        dir_options,
+        file_options,
+    )
+});
+
+/// Return a handle to the global [`PdmCache`] instance.
+pub fn instance() -> &'static PdmCache {
+    &CACHE_INSTANCE
+}
+
+/// Cache for storing the results of API requests, as well as aggregations thereof.
+pub struct PdmCache(NamespacedCache);
+
+impl PdmCache {
+    /// Create a new cache instance.
+    ///
+    /// # Note
+    /// Most likely, you want to access the single global instance via [`pdm_cache::instance()`]
+    /// instead of calling `new` yourself.
+    fn new<P: AsRef<Path>>(
+        base_path: P,
+        dir_options: CreateOptions,
+        file_options: CreateOptions,
+    ) -> Self {
+        Self(NamespacedCache::new(base_path, dir_options, file_options))
+    }
+
+    /// Lock the cache for reading remote-specific data.
+    pub fn read_remote(&self, remote: &str) -> Result<ReadableCacheNamespace, CacheError> {
+        let namespace = format!("remote-{remote}");
+        self.0.read(&namespace, Duration::from_secs(10))
+    }
+
+    /// Lock the cache for writing remote-specific data.
+    pub fn write_remote(&self, remote: &str) -> Result<WritableCacheNamespace, CacheError> {
+        let namespace = format!("remote-{remote}");
+        self.0.write(&namespace, Duration::from_secs(10))
+    }
+
+    /// Lock the cache for reading global data.
+    pub fn read_global(&self) -> Result<ReadableCacheNamespace, CacheError> {
+        self.0.read("global", Duration::from_secs(10))
+    }
+
+    /// Lock the cache for writing global data.
+    pub fn write_global(&self) -> Result<WritableCacheNamespace, CacheError> {
+        self.0.write("global", Duration::from_secs(10))
+    }
+}
-- 
2.47.3

[PATCH datacenter-manager 3/4] api: resources: subscriptions: switch over to pdm_cache

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/api/resources.rs | 82 ++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 51 deletions(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 50315b11..c22e33c5 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -30,9 +30,10 @@ use proxmox_schema::{api, parse_boolean};
 use proxmox_sortable_macro::sortable;
 use proxmox_subscription::SubscriptionStatus;
 use pve_api_types::{ClusterResource, ClusterResourceNetworkType, ClusterResourceType};
+use serde::{Deserialize, Serialize};
 
 use crate::metric_collection::top_entities;
-use crate::{connection, views};
+use crate::{connection, pdm_cache, views};
 
 pub const ROUTER: Router = Router::new()
     .get(&list_subdirs_api_method!(SUBDIRS))
@@ -798,15 +799,11 @@ async fn get_top_entities(
     Ok(res)
 }
 
-#[derive(Clone)]
+#[derive(Clone, Serialize, Deserialize)]
 struct CachedSubscriptionState {
     node_info: HashMap<String, Option<NodeSubscriptionInfo>>,
-    timestamp: i64,
 }
 
-static SUBSCRIPTION_CACHE: LazyLock<RwLock<HashMap<String, CachedSubscriptionState>>> =
-    LazyLock::new(|| RwLock::new(HashMap::new()));
-
 /// Get the subscription state for a given remote.
 ///
 /// If recent enough cached data is available, it is returned
@@ -815,66 +812,49 @@ pub async fn get_subscription_info_for_remote(
     remote: &Remote,
     max_age: u64,
 ) -> Result<HashMap<String, Option<NodeSubscriptionInfo>>, Error> {
-    if let Some(cached_subscription) = get_cached_subscription_info(&remote.id, max_age) {
+    if let Some(cached_subscription) =
+        get_cached_subscription_info(remote.id.clone(), max_age).await?
+    {
         Ok(cached_subscription.node_info)
     } else {
         let node_info = fetch_remote_subscription_info(remote).await?;
-        let now = proxmox_time::epoch_i64();
-        update_cached_subscription_info(&remote.id, &node_info, now);
+        update_cached_subscription_info(remote.id.clone(), node_info.clone()).await?;
         Ok(node_info)
     }
 }
 
-fn get_cached_subscription_info(remote: &str, max_age: u64) -> Option<CachedSubscriptionState> {
-    let cache = SUBSCRIPTION_CACHE
-        .read()
-        .expect("subscription mutex poisoned");
+const SUBSCRIPTION_STATE_CACHE_KEY: &str = "subscription-state";
 
-    if max_age == 0 {
-        return None;
-    }
-    if let Some(cached_subscription) = cache.get(remote) {
-        let now = proxmox_time::epoch_i64();
-        let diff = now - cached_subscription.timestamp;
+async fn get_cached_subscription_info(
+    remote: String,
+    max_age: u64,
+) -> Result<Option<CachedSubscriptionState>, Error> {
+    tokio::task::spawn_blocking(move || {
+        let cache = pdm_cache::instance().read_remote(&remote)?;
 
-        if diff >= max_age as i64 || diff < 0 {
-            // value is too old or from the future
-            None
-        } else {
-            Some(cached_subscription.clone())
-        }
-    } else {
-        None
-    }
+        Ok(cache.get_with_max_age(SUBSCRIPTION_STATE_CACHE_KEY, max_age as i64)?)
+    })
+    .await?
 }
 
 /// Update cached subscription data.
 ///
 /// If the cache already contains more recent data we don't insert the passed resources.
-fn update_cached_subscription_info(
-    remote: &str,
-    node_info: &HashMap<String, Option<NodeSubscriptionInfo>>,
-    now: i64,
-) {
-    // there is no good way to recover from this, so panicking should be fine
-    let mut cache = SUBSCRIPTION_CACHE
-        .write()
-        .expect("subscription mutex poisoned");
+async fn update_cached_subscription_info(
+    remote: String,
+    node_info: HashMap<String, Option<NodeSubscriptionInfo>>,
+) -> Result<(), Error> {
+    tokio::task::spawn_blocking(move || {
+        let cache = pdm_cache::instance().write_remote(&remote)?;
 
-    if let Some(cached_resource) = cache.get(remote) {
-        // skip updating if the data is new enough
-        if cached_resource.timestamp >= now {
-            return;
-        }
-    }
-
-    cache.insert(
-        remote.into(),
-        CachedSubscriptionState {
-            node_info: node_info.clone(),
-            timestamp: now,
-        },
-    );
+        Ok(cache.set(
+            SUBSCRIPTION_STATE_CACHE_KEY,
+            &CachedSubscriptionState {
+                node_info: node_info,
+            },
+        )?)
+    })
+    .await?
 }
 
 /// Maps a list of node subscription infos into a single [`RemoteSubscriptionState`]
-- 
2.47.3

[PATCH datacenter-manager 4/4] remote-updates: switch over to pdm_cache

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Should probably add clean-up code for the old cache-file before this is
    applied

 server/src/remote_updates.rs | 52 +++++++++++-------------------------
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/server/src/remote_updates.rs b/server/src/remote_updates.rs
index 7aaacc46..e0f61da4 100644
--- a/server/src/remote_updates.rs
+++ b/server/src/remote_updates.rs
@@ -1,6 +1,3 @@
-use std::fs::File;
-use std::io::ErrorKind;
-
 use anyhow::{bail, Error};
 use serde::{Deserialize, Serialize};
 
@@ -12,12 +9,11 @@ use pdm_api_types::remote_updates::{
 };
 use pdm_api_types::remotes::{Remote, RemoteType};
 use pdm_api_types::RemoteUpid;
-use pdm_buildcfg::PDM_CACHE_DIR_M;
 
-use crate::connection;
 use crate::parallel_fetcher::ParallelFetcher;
+use crate::{connection, pdm_cache};
 
-pub const UPDATE_CACHE: &str = concat!(PDM_CACHE_DIR_M!(), "/remote-updates.json");
+const UPDATE_SUMMARY_CACHE_KEY: &str = "remote-updates";
 
 #[derive(Clone, Default, Debug, Deserialize, Serialize)]
 #[serde(rename_all = "kebab-case")]
@@ -157,21 +153,10 @@ pub fn get_available_updates_for_remote(remote: &str) -> Result<RemoteUpdateSumm
 }
 
 fn get_cached_summary_or_default() -> Result<UpdateSummary, Error> {
-    match File::open(UPDATE_CACHE) {
-        Ok(file) => {
-            let content = match serde_json::from_reader(file) {
-                Ok(cache_content) => cache_content,
-                Err(err) => {
-                    log::error!("failed to deserialize remote update cache: {err:#}");
-                    Default::default()
-                }
-            };
-
-            Ok(content)
-        }
-        Err(err) if err.kind() == ErrorKind::NotFound => Ok(Default::default()),
-        Err(err) => Err(err.into()),
-    }
+    Ok(pdm_cache::instance()
+        .read_global()?
+        .get::<UpdateSummary>(UPDATE_SUMMARY_CACHE_KEY)?
+        .unwrap_or_default())
 }
 
 async fn update_cached_summary_for_node(
@@ -179,10 +164,11 @@ async fn update_cached_summary_for_node(
     node: String,
     node_data: NodeUpdateSummary,
 ) -> Result<(), Error> {
-    let mut file = File::open(UPDATE_CACHE)?;
-    let mut cache_content: UpdateSummary = serde_json::from_reader(&mut file)?;
-    let remote_entry =
-        cache_content
+    let cache = pdm_cache::instance().write_global()?;
+    let cache_content = cache.get::<UpdateSummary>(UPDATE_SUMMARY_CACHE_KEY)?;
+
+    if let Some(mut entry) = cache_content {
+        let remote_entry = entry
             .remotes
             .entry(remote.id)
             .or_insert_with(|| RemoteUpdateSummary {
@@ -191,15 +177,9 @@ async fn update_cached_summary_for_node(
                 status: RemoteUpdateStatus::Success,
             });
 
-    remote_entry.nodes.insert(node, node_data);
-
-    let options = proxmox_product_config::default_create_options();
-    proxmox_sys::fs::replace_file(
-        UPDATE_CACHE,
-        &serde_json::to_vec(&cache_content)?,
-        options,
-        true,
-    )?;
+        remote_entry.nodes.insert(node, node_data);
+        cache.set(UPDATE_SUMMARY_CACHE_KEY, &entry)?;
+    }
 
     Ok(())
 }
@@ -275,8 +255,8 @@ pub async fn refresh_update_summary_cache(remotes: Vec<Remote>) -> Result<(), Er
         }
     }
 
-    let options = proxmox_product_config::default_create_options();
-    proxmox_sys::fs::replace_file(UPDATE_CACHE, &serde_json::to_vec(&content)?, options, true)?;
+    let cache = pdm_cache::instance().write_global()?;
+    cache.set(UPDATE_SUMMARY_CACHE_KEY, &content)?;
 
     Ok(())
 }
-- 
2.47.3