From 8eb9c5fa104e3a429f63f0aa85c06d1b6db3a373 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Mon, 27 Apr 2026 16:07:35 +1000 Subject: [PATCH] fix(daemon): root local state at DataDir not BaseDir LocalStatePath was falling back to BaseDir when DataDir was empty, putting the file at /var/lib/spinifex/spinifex/state/instance-state.json (under the daemon's own subdir) instead of the documented /var/lib/spinifex/state/instance-state.json (the platform-shared root). Drop the BaseDir fallback. Empty DataDir resolves to DefaultDataDir (/var/lib/spinifex). Tests now set DataDir alongside BaseDir on their config fixtures. --- spinifex/daemon/daemon.go | 4 ++-- spinifex/daemon/daemon_test.go | 21 ++++++++++++--------- spinifex/daemon/local_state.go | 21 +++++++++------------ spinifex/daemon/local_state_test.go | 13 ++++--------- spinifex/daemon/state_test.go | 4 ++-- 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 618b8170c..9eee9f995 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -1492,9 +1492,9 @@ const kvSyncTimeout = time.Second // localStatePath returns the on-disk path to this daemon's instance state file. func (d *Daemon) localStatePath() string { if d.config == nil { - return LocalStatePath("", "") + return LocalStatePath("") } - return LocalStatePath(d.config.DataDir, d.config.BaseDir) + return LocalStatePath(d.config.DataDir) } // WriteState persists the instance state. Local file is the source of truth; diff --git a/spinifex/daemon/daemon_test.go b/spinifex/daemon/daemon_test.go index 74d3b7efa..abe8d016b 100644 --- a/spinifex/daemon/daemon_test.go +++ b/spinifex/daemon/daemon_test.go @@ -52,6 +52,7 @@ func createTestDaemon(t *testing.T, natsURL string) *Daemon { cfg := &config.Config{ BaseDir: tmpDir, + DataDir: tmpDir, WalDir: tmpDir, NATS: config.NATSConfig{ Host: natsURL, @@ -364,6 +365,7 @@ func TestDaemon_Initialization(t *testing.T) { cfg := &config.Config{ BaseDir: tmpDir, + DataDir: tmpDir, WalDir: tmpDir, NATS: config.NATSConfig{ Host: "nats://localhost:4222", @@ -898,11 +900,11 @@ func TestDaemon_BootAllocation(t *testing.T) { // Create daemon with NATS connection clusterCfg := &config.ClusterConfig{ Node: "node-1", - Nodes: map[string]config.Config{"node-1": {BaseDir: tmpDir}}, + Nodes: map[string]config.Config{"node-1": {BaseDir: tmpDir, DataDir: tmpDir}}, } daemon, err := NewDaemon(clusterCfg) require.NoError(t, err) - daemon.config = &config.Config{BaseDir: tmpDir} + daemon.config = &config.Config{BaseDir: tmpDir, DataDir: tmpDir} // Connect to NATS and initialize JetStream nc, err := nats.Connect(natsURL) @@ -3133,12 +3135,12 @@ func TestDaemon_WriteState_NilJSManager(t *testing.T) { tmpDir := t.TempDir() d := &Daemon{ jsManager: nil, - config: &config.Config{BaseDir: tmpDir}, + config: &config.Config{DataDir: tmpDir}, Instances: vm.Instances{VMS: map[string]*vm.VM{"i-1": {ID: "i-1"}}}, } require.NoError(t, d.WriteState()) - state, err := ReadLocalState(LocalStatePath("", tmpDir)) + state, err := ReadLocalState(LocalStatePath(tmpDir)) require.NoError(t, err) require.NotNil(t, state) assert.Contains(t, state.VMS, "i-1") @@ -3146,13 +3148,13 @@ func TestDaemon_WriteState_NilJSManager(t *testing.T) { func TestDaemon_LoadState_NilJSManager(t *testing.T) { tmpDir := t.TempDir() - require.NoError(t, WriteLocalState(LocalStatePath("", tmpDir), &vm.Instances{ + require.NoError(t, WriteLocalState(LocalStatePath(tmpDir), &vm.Instances{ VMS: map[string]*vm.VM{"i-seed": {ID: "i-seed"}}, })) d := &Daemon{ jsManager: nil, - config: &config.Config{BaseDir: tmpDir}, + config: &config.Config{DataDir: tmpDir}, } require.NoError(t, d.LoadState()) assert.Contains(t, d.Instances.VMS, "i-seed") @@ -3161,7 +3163,7 @@ func TestDaemon_LoadState_NilJSManager(t *testing.T) { func TestDaemon_LoadState_MissingFileIsFreshInstall(t *testing.T) { d := &Daemon{ jsManager: nil, - config: &config.Config{BaseDir: t.TempDir()}, + config: &config.Config{DataDir: t.TempDir()}, } require.NoError(t, d.LoadState()) assert.Empty(t, d.Instances.VMS) @@ -3169,13 +3171,13 @@ func TestDaemon_LoadState_MissingFileIsFreshInstall(t *testing.T) { func TestDaemon_LoadState_CorruptFileFatal(t *testing.T) { tmpDir := t.TempDir() - path := LocalStatePath("", tmpDir) + path := LocalStatePath(tmpDir) require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o750)) require.NoError(t, os.WriteFile(path, []byte("{not json"), 0o600)) d := &Daemon{ jsManager: nil, - config: &config.Config{BaseDir: tmpDir}, + config: &config.Config{DataDir: tmpDir}, } err := d.LoadState() require.Error(t, err) @@ -4061,6 +4063,7 @@ func TestClusterManager_TLSServesHTTPS(t *testing.T) { Nodes: map[string]config.Config{ "node-1": { BaseDir: tmpDir, + DataDir: tmpDir, Daemon: config.DaemonConfig{ Host: addr, TLSCert: certPath, diff --git a/spinifex/daemon/local_state.go b/spinifex/daemon/local_state.go index 64a1d91fe..09b2c0832 100644 --- a/spinifex/daemon/local_state.go +++ b/spinifex/daemon/local_state.go @@ -34,19 +34,16 @@ type LocalState struct { VMS map[string]*vm.VM `json:"vms"` } -// LocalStatePath returns the absolute path to the per-node instance state file. -// Resolution order: dataDir, then baseDir, then DefaultDataDir. The first -// non-empty value wins. Production sets dataDir (/var/lib/spinifex); tests -// typically set baseDir to a t.TempDir(). -func LocalStatePath(dataDir, baseDir string) string { - root := dataDir - if root == "" { - root = baseDir +// LocalStatePath returns the absolute path to the per-node instance state file +// rooted at dataDir. Empty dataDir falls back to DefaultDataDir +// (/var/lib/spinifex), the platform-shared root. The state directory sits at +// the platform root so it lives next to other shared state (predastore, +// viperblock data dirs) rather than nested under the daemon's own subdir. +func LocalStatePath(dataDir string) string { + if dataDir == "" { + dataDir = DefaultDataDir } - if root == "" { - root = DefaultDataDir - } - return filepath.Join(root, DefaultLocalStateDir, LocalStateFileName) + return filepath.Join(dataDir, DefaultLocalStateDir, LocalStateFileName) } // WriteLocalState atomically writes the instance state to path. diff --git a/spinifex/daemon/local_state_test.go b/spinifex/daemon/local_state_test.go index 8b577b5cc..6238bf608 100644 --- a/spinifex/daemon/local_state_test.go +++ b/spinifex/daemon/local_state_test.go @@ -11,18 +11,13 @@ import ( "github.com/stretchr/testify/require" ) -func TestLocalStatePath_PrefersDataDir(t *testing.T) { - got := LocalStatePath("/data", "/base") +func TestLocalStatePath_UsesDataDir(t *testing.T) { + got := LocalStatePath("/data") assert.Equal(t, "/data/state/instance-state.json", got) } -func TestLocalStatePath_FallsBackToBaseDir(t *testing.T) { - got := LocalStatePath("", "/base") - assert.Equal(t, "/base/state/instance-state.json", got) -} - -func TestLocalStatePath_DefaultWhenBothEmpty(t *testing.T) { - got := LocalStatePath("", "") +func TestLocalStatePath_DefaultWhenEmpty(t *testing.T) { + got := LocalStatePath("") assert.Equal(t, "/var/lib/spinifex/state/instance-state.json", got) } diff --git a/spinifex/daemon/state_test.go b/spinifex/daemon/state_test.go index 8f9760e1c..0cbc56750 100644 --- a/spinifex/daemon/state_test.go +++ b/spinifex/daemon/state_test.go @@ -28,11 +28,11 @@ func createDaemonWithJetStream(t *testing.T) *Daemon { clusterCfg := &config.ClusterConfig{ Node: "node-1", - Nodes: map[string]config.Config{"node-1": {BaseDir: tmpDir}}, + Nodes: map[string]config.Config{"node-1": {BaseDir: tmpDir, DataDir: tmpDir}}, } daemon, err := NewDaemon(clusterCfg) require.NoError(t, err) - daemon.config = &config.Config{BaseDir: tmpDir} + daemon.config = &config.Config{BaseDir: tmpDir, DataDir: tmpDir} daemon.natsConn = nc daemon.jsManager, err = NewJetStreamManager(nc, 1)