fixes for storage managament

internal/hoststats/hoststats.go

@@ -7,6 +7,8 @@ package hoststats
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"sync"
 	"time"
 )
@@ -14,12 +16,18 @@ import (
 // Snapshot is the last successfully sampled host-resource reading. Ready is
 // false until the first sample completes, so callers can distinguish "0% CPU"
 // from "no data yet" instead of rendering a misleading zero on startup.
+// DiskReady is a separate flag: disk stats are sampled from a configured
+// path (see NewPoller) that can fail independently of CPU/memory sampling
+// (wrong working directory, path not created yet, etc) -- without it, a
+// failed disk read looked identical to "this server's disk is completely
+// full" (0 free bytes) instead of "we don't have a reading right now".
 type Snapshot struct {
 	CPUPercent     float64
 	MemUsedBytes   int64
 	MemTotalBytes  int64
 	DiskFreeBytes  int64
 	DiskTotalBytes int64
+	DiskReady      bool
 	Ready          bool
 }
 
@@ -29,6 +37,10 @@ type Snapshot struct {
 // for the local blob-storage free-space guard.
 type Poller struct {
 	diskPath string
+	// diskFreeBytesFn defaults to the platform diskFreeBytes function;
+	// overridable in tests to simulate a failing/succeeding disk read
+	// without touching the real filesystem/OS call.
+	diskFreeBytesFn func(path string) (free, total int64, err error)
 
 	mu   sync.RWMutex
 	snap Snapshot
@@ -38,12 +50,31 @@ type Poller struct {
 
 // NewPoller creates a poller that reports free/total disk space for the
 // filesystem containing diskPath (pass the server's data/blob directory, or
-// "." if it doesn't matter which volume).
+// "." if it doesn't matter which volume). diskPath is resolved to an
+// absolute path up front (a relative path depends on the process's current
+// directory, which callers shouldn't have to reason about here) and, if it
+// doesn't exist yet -- e.g. an S3-backend deployment whose local blob
+// staging directory is only created on first upload -- walked up to the
+// nearest existing ancestor, since GetDiskFreeSpaceEx/statfs need a real
+// path and every ancestor is on the same volume anyway.
 func NewPoller(diskPath string) *Poller {
 	if diskPath == "" {
 		diskPath = "."
 	}
-	return &Poller{diskPath: diskPath}
+	if abs, err := filepath.Abs(diskPath); err == nil {
+		diskPath = abs
+	}
+	for {
+		if _, err := os.Stat(diskPath); err == nil {
+			break
+		}
+		parent := filepath.Dir(diskPath)
+		if parent == diskPath {
+			break
+		}
+		diskPath = parent
+	}
+	return &Poller{diskPath: diskPath, diskFreeBytesFn: diskFreeBytes}
 }
 
 // Snapshot returns the last sample. Safe to call concurrently with Run.
@@ -72,13 +103,27 @@ func (p *Poller) Run(ctx context.Context, interval time.Duration) {
 }
 
 func (p *Poller) sampleOnce() {
+	p.mu.RLock()
+	prevFree, prevTotal := p.snap.DiskFreeBytes, p.snap.DiskTotalBytes
+	p.mu.RUnlock()
+
 	var snap Snapshot
 	snap.CPUPercent = p.cpu.sample()
 	if used, total, err := memStats(); err == nil {
 		snap.MemUsedBytes, snap.MemTotalBytes = used, total
 	}
-	if free, total, err := diskFreeBytes(p.diskPath); err == nil {
+	if free, total, err := p.diskFreeBytesFn(p.diskPath); err == nil {
 		snap.DiskFreeBytes, snap.DiskTotalBytes = free, total
+		snap.DiskReady = true
+	} else {
+		// Keep the last known-good byte values stored (harmless, and a
+		// reasonable fallback for any future caller that wants "last known"
+		// over nothing) but DiskReady reflects THIS sample, not a stale one
+		// -- a failure must show as "no current reading", not silently keep
+		// claiming Ready while quietly reusing old numbers forever if the
+		// underlying path became permanently unreadable.
+		snap.DiskFreeBytes, snap.DiskTotalBytes = prevFree, prevTotal
+		snap.DiskReady = false
 	}
 	snap.Ready = true
 

internal/hoststats/hoststats_test.go

@@ -0,0 +1,106 @@
+package hoststats
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// TestSampleOncePreservesLastGoodDiskReadingOnFailure guards the bug
+// reported live: a failed disk-space sample used to silently reset
+// DiskFreeBytes/DiskTotalBytes to 0 while still marking the overall
+// snapshot Ready -- rendering as "0 bytes free" (indistinguishable from an
+// actually-full disk) instead of "no reading right now". A failed sample
+// must keep the last known-good reading and report DiskReady=false.
+func TestSampleOncePreservesLastGoodDiskReadingOnFailure(t *testing.T) {
+	p := &Poller{diskFreeBytesFn: func(string) (int64, int64, error) {
+		return 1234, 5678, nil
+	}}
+	p.sampleOnce()
+	first := p.Snapshot()
+	if !first.DiskReady || first.DiskFreeBytes != 1234 || first.DiskTotalBytes != 5678 {
+		t.Fatalf("first snapshot = %+v, want a successful disk reading", first)
+	}
+
+	p.diskFreeBytesFn = func(string) (int64, int64, error) {
+		return 0, 0, errors.New("disk stat failed")
+	}
+	p.sampleOnce()
+	second := p.Snapshot()
+	if second.DiskReady {
+		t.Fatal("DiskReady = true after a failed sample, want false")
+	}
+	if second.DiskFreeBytes != 1234 || second.DiskTotalBytes != 5678 {
+		t.Fatalf("disk fields after failed sample = (%d, %d), want the preserved (1234, 5678)", second.DiskFreeBytes, second.DiskTotalBytes)
+	}
+	// CPU/memory sampling must still complete and mark Ready, independent
+	// of the disk failure.
+	if !second.Ready {
+		t.Fatal("Ready = false after a disk-only failure, want true (CPU/mem still sampled)")
+	}
+}
+
+// TestSampleOnceNeverReadyWithoutAnyPriorSuccess confirms a disk read that
+// has NEVER succeeded (not just failed after a prior success) still reports
+// DiskReady=false with zero-value fields, not a fabricated 0-bytes-free
+// reading.
+func TestSampleOnceNeverReadyWithoutAnyPriorSuccess(t *testing.T) {
+	p := &Poller{diskFreeBytesFn: func(string) (int64, int64, error) {
+		return 0, 0, errors.New("never worked")
+	}}
+	p.sampleOnce()
+	snap := p.Snapshot()
+	if snap.DiskReady {
+		t.Fatal("DiskReady = true with no successful sample ever, want false")
+	}
+	if !snap.Ready {
+		t.Fatal("Ready = false, want true (CPU/mem sampling is independent of disk)")
+	}
+}
+
+// TestNewPollerWalksUpToNearestExistingAncestor guards the other half of the
+// fix: a configured disk path that doesn't exist yet (e.g. an S3-backend
+// deployment's local blob staging directory, only created on first upload)
+// must not permanently break disk stats -- NewPoller walks up to the
+// nearest existing ancestor instead of handing GetDiskFreeSpaceEx/statfs a
+// path they will always fail to stat.
+func TestNewPollerWalksUpToNearestExistingAncestor(t *testing.T) {
+	tmp := t.TempDir()
+	missing := filepath.Join(tmp, "not-created-yet", "nested", "deeper")
+
+	p := NewPoller(missing)
+
+	if p.diskPath != tmp {
+		t.Fatalf("resolved disk path = %q, want the nearest existing ancestor %q", p.diskPath, tmp)
+	}
+	if _, err := os.Stat(p.diskPath); err != nil {
+		t.Fatalf("resolved disk path %q does not exist: %v", p.diskPath, err)
+	}
+}
+
+// TestNewPollerResolvesRelativePathToAbsolute confirms a relative diskPath
+// (as configured today, e.g. "data/blobs") no longer silently depends on
+// whatever the process's current directory happens to be at NewPoller time.
+func TestNewPollerResolvesRelativePathToAbsolute(t *testing.T) {
+	p := NewPoller(".")
+	if !filepath.IsAbs(p.diskPath) {
+		t.Fatalf("resolved disk path %q is not absolute", p.diskPath)
+	}
+}
+
+// TestRunSamplesImmediatelyOnStart confirms Run's documented behavior
+// (sampleOnce before entering the ticker loop) without relying on any
+// ticker firing or a busy-wait: cancel the context right away and check the
+// synchronous first sample already landed by the time Run returns.
+func TestRunSamplesImmediatelyOnStart(t *testing.T) {
+	p := &Poller{diskFreeBytesFn: func(string) (int64, int64, error) { return 1, 1, nil }}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	p.Run(ctx, time.Hour)
+	if !p.Snapshot().Ready {
+		t.Fatal("Run did not produce a ready snapshot from its initial sample")
+	}
+}