From c04cabdf80f79aa80206c29fb5c1ddfef7d0fe57 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Mon, 28 Sep 2026 23:51:58 +0200
Subject: [PATCH] hive-c0re: warn on a du failure that isn't just a missing
 path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

du_bytes returned None uniformly for every du failure, and both
du_bytes and measure_agent_disk had no logging at all, so a container
rootfs du couldn't read (permission, I/O error, unparseable output)
silently reported as 0 disk with no signal in the journal.

Distinguish the expected case — the path plain doesn't exist, e.g. a
destroyed-but-kept agent's rootfs or a not-yet-created state dir — from
an actual read failure, and warn only on the latter, with the path plus
whichever of the failure (spawn error, exit status, stderr, unparseable
stdout) applies.
---
 hive-c0re/src/stats/container_stats.rs | 79 ++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 6 deletions(-)

diff --git a/hive-c0re/src/stats/container_stats.rs b/hive-c0re/src/stats/container_stats.rs
index 24d579ad..26f677d1 100644
--- a/hive-c0re/src/stats/container_stats.rs
+++ b/hive-c0re/src/stats/container_stats.rs
@@ -86,21 +86,57 @@ fn disk_cache() -> &'static RwLock<HashMap<String, u64>> {
 /// `du -sxb <path>` → apparent total bytes. `-s` summary, `-b` apparent
 /// size, `-x` stay on one filesystem so the walk never descends into the
 /// bind-mounted read-only shared nix store (or any other bind mount) —
-/// exactly the per-container-only measure we want. Best-effort: a
-/// missing/unreadable path (e.g. a stopped container with no rootfs)
-/// yields `None`, which the caller treats as 0.
+/// exactly the per-container-only measure we want. Best-effort: the
+/// caller treats `None` as a 0 contribution either way, but the two
+/// `None` causes are logged differently. A path that plain doesn't exist
+/// (`kept_state_names` includes destroyed-but-kept tombstones with no
+/// rootfs, and a fresh agent has no state dir yet) is the expected,
+/// silent case — every sampler pass would otherwise warn on it forever.
+/// A path that exists but `du` still can't read — permission, I/O error,
+/// unparseable output — is warn!'d, since that 0 is not "nothing there",
+/// it's a read we should have gotten.
 async fn du_bytes(path: &std::path::Path) -> Option<u64> {
-    let out = tokio::process::Command::new("du")
+    match std::fs::symlink_metadata(path) {
+        Ok(_) => {}
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => return None,
+        Err(e) => {
+            tracing::warn!(path = %path.display(), error = %e, "du: path stat failed");
+            return None;
+        }
+    }
+    let out = match tokio::process::Command::new("du")
         .arg("-sxb")
         .arg(path)
         .output()
         .await
-        .ok()?;
+    {
+        Ok(out) => out,
+        Err(e) => {
+            tracing::warn!(path = %path.display(), error = %e, "du: failed to spawn");
+            return None;
+        }
+    };
     if !out.status.success() {
+        tracing::warn!(
+            path = %path.display(),
+            status = %out.status,
+            stderr = %String::from_utf8_lossy(&out.stderr),
+            "du: exited non-zero"
+        );
         return None;
     }
     let stdout = String::from_utf8_lossy(&out.stdout);
-    stdout.split_whitespace().next()?.parse::<u64>().ok()
+    let Some(first) = stdout.split_whitespace().next() else {
+        tracing::warn!(path = %path.display(), stdout = %stdout, "du: empty output");
+        return None;
+    };
+    match first.parse::<u64>() {
+        Ok(bytes) => Some(bytes),
+        Err(e) => {
+            tracing::warn!(path = %path.display(), stdout = %stdout, error = %e, "du: unparseable output");
+            None
+        }
+    }
 }
 
 /// Total per-agent disk: the agent's host-side state dir
@@ -300,6 +336,8 @@ pub async fn gather() -> Vec<ContainerResource> {
 
 #[cfg(test)]
 mod tests {
+    use std::os::unix::fs::PermissionsExt;
+
     use super::*;
 
     #[test]
@@ -317,4 +355,33 @@ mod tests {
             PathBuf::from("/sys/fs/cgroup/machine.slice/container@h-foo_bar.service")
         );
     }
+
+    #[tokio::test]
+    async fn du_bytes_missing_path_is_none() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        assert_eq!(du_bytes(&dir.path().join("does-not-exist")).await, None);
+    }
+
+    #[tokio::test]
+    async fn du_bytes_reads_a_real_dir() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        std::fs::write(dir.path().join("f"), b"hello").expect("write");
+        assert!(du_bytes(dir.path()).await.is_some_and(|b| b > 0));
+    }
+
+    #[tokio::test]
+    async fn du_bytes_unreadable_dir_is_none() {
+        // A dir that exists but can't be entered (mode 000) is the case
+        // that must be distinguished from "doesn't exist" — both currently
+        // become a 0 contribution, but only this one is worth a warn.
+        let dir = tempfile::tempdir().expect("tempdir");
+        let unreadable = dir.path().join("locked");
+        std::fs::create_dir(&unreadable).expect("mkdir");
+        std::fs::set_permissions(&unreadable, std::fs::Permissions::from_mode(0o000))
+            .expect("chmod");
+        let result = du_bytes(&unreadable).await;
+        std::fs::set_permissions(&unreadable, std::fs::Permissions::from_mode(0o755))
+            .expect("restore perms so tempdir cleanup can remove it");
+        assert_eq!(result, None);
+    }
 }