Stop a failed system settings read from resetting settings silently

A boot-time LoadSystemSettings error fell into the missing-file branch:
no log line, every persisted system setting reverted to defaults for
the run, and a default system.json was written over the file on disk.
Observed once in practice (2026-08-20): a restart came up with a
persisted toggle unset while system.json still held the correct value.

Distinguish the error from the genuinely-missing file: retry the read
once (transient failures at boot were the observed shape), then warn
with the error and file path and run on defaults without touching
system.json. Tests cover the warning, the untouched file, and the
retry-once behavior.
This commit is contained in:
rcourtman
2026-08-20 14:34:05 +01:00
parent a4d37447a0
commit cf5b86e624
2 changed files with 116 additions and 1 deletions
+22 -1
View File
@@ -759,6 +759,23 @@ func parseDurationOverrideEnv(envName string, minDuration time.Duration) (time.D
return 0, false
}
// systemSettingsRetryDelay is how long load waits before retrying a failed
// system settings read. Kept as a var so tests can shorten it.
var systemSettingsRetryDelay = 250 * time.Millisecond
// loadSystemSettingsWithRetry retries a failed system settings read once: a
// transient read error at boot would otherwise revert every persisted system
// setting to defaults for the whole run.
func loadSystemSettingsWithRetry(loadSettings func() (*SystemSettings, error)) (*SystemSettings, error) {
settings, err := loadSettings()
if err == nil {
return settings, nil
}
log.Warn().Err(err).Msg("System settings read failed; retrying once")
time.Sleep(systemSettingsRetryDelay)
return loadSettings()
}
// Load reads configuration from encrypted persistence files.
func Load() (*Config, error) {
return load(true)
@@ -857,7 +874,7 @@ func load(initLogging bool) (*Config, error) {
}
// Load system configuration
if systemSettings, err := persistence.LoadSystemSettings(); err == nil && systemSettings != nil {
if systemSettings, err := loadSystemSettingsWithRetry(persistence.LoadSystemSettings); err == nil && systemSettings != nil {
// Load polling intervals if configured
if systemSettings.PVEPollingInterval > 0 {
cfg.PVEPollingInterval = time.Duration(systemSettings.PVEPollingInterval) * time.Second
@@ -956,6 +973,10 @@ func load(initLogging bool) (*Config, error) {
Dur("dnsCacheTimeout", cfg.DNSCacheTimeout).
Int("metricsRetentionDailyDays", cfg.MetricsRetentionDailyDays).
Msg("Loaded system configuration")
} else if err != nil {
// A read failure is not a missing file: leave system.json on disk
// untouched and run on defaults rather than recreating it.
log.Warn().Err(err).Str("file", persistence.systemFile).Msg("Failed to load system settings; using defaults for this run")
} else {
// No system.json exists - create default one
log.Info().Msg("No system.json found, creating default")
+94
View File
@@ -1,6 +1,7 @@
package config
import (
"bytes"
"encoding/base64"
"errors"
"os"
@@ -9,6 +10,8 @@ import (
"testing"
"time"
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -569,3 +572,94 @@ func TestLoad_IgnoresLegacyAutoUpdateScheduleFields(t *testing.T) {
assert.True(t, cfg.AutoUpdateEnabled, "autoUpdateEnabled from a legacy system.json must still apply")
assert.Equal(t, "stable", cfg.UpdateChannel)
}
func captureConfigLogs(t *testing.T) *bytes.Buffer {
t.Helper()
var buf bytes.Buffer
origLogger := log.Logger
origLevel := zerolog.GlobalLevel()
log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel)
zerolog.SetGlobalLevel(zerolog.DebugLevel)
t.Cleanup(func() {
log.Logger = origLogger
zerolog.SetGlobalLevel(origLevel)
})
return &buf
}
// A system.json that exists but cannot be read must not be confused with a
// missing one: the failure has to be logged, the run continues on defaults,
// and the file on disk must not be replaced with a default system.json.
func TestLoad_SystemSettingsReadFailureWarnsAndKeepsFileIntact(t *testing.T) {
dataDir := t.TempDir()
t.Setenv("PULSE_DATA_DIR", dataDir)
// A directory named system.json makes the read fail with an error that
// is not os.IsNotExist — the same shape as a transient read failure.
require.NoError(t, os.Mkdir(filepath.Join(dataDir, "system.json"), 0o700))
prevDelay := systemSettingsRetryDelay
systemSettingsRetryDelay = 0
t.Cleanup(func() { systemSettingsRetryDelay = prevDelay })
logOutput := captureConfigLogs(t)
cfg, err := LoadWithoutLoggingInit()
require.NoError(t, err)
assert.True(t, cfg.TemperatureMonitoringEnabled, "run should continue on defaults")
assert.Contains(t, logOutput.String(), "Failed to load system settings",
"a failing system settings read must be logged, not skipped silently")
assert.Contains(t, logOutput.String(), filepath.Join(dataDir, "system.json"),
"the warning should name the settings file")
assert.NotContains(t, logOutput.String(), "No system.json found",
"a read failure must not take the missing-file create-default path")
info, err := os.Stat(filepath.Join(dataDir, "system.json"))
require.NoError(t, err)
assert.True(t, info.IsDir(), "system.json on disk must be left untouched")
}
func TestLoadSystemSettingsWithRetry(t *testing.T) {
prevDelay := systemSettingsRetryDelay
systemSettingsRetryDelay = 0
t.Cleanup(func() { systemSettingsRetryDelay = prevDelay })
t.Run("transient_failure_recovers_on_retry", func(t *testing.T) {
want := &SystemSettings{PVEPollingInterval: 42}
calls := 0
settings, err := loadSystemSettingsWithRetry(func() (*SystemSettings, error) {
calls++
if calls == 1 {
return nil, errors.New("transient read failure")
}
return want, nil
})
require.NoError(t, err)
assert.Equal(t, 2, calls)
assert.Same(t, want, settings)
})
t.Run("persistent_failure_surfaces_error", func(t *testing.T) {
calls := 0
settings, err := loadSystemSettingsWithRetry(func() (*SystemSettings, error) {
calls++
return nil, errors.New("still failing")
})
require.Error(t, err)
assert.Nil(t, settings)
assert.Equal(t, 2, calls, "exactly one retry")
})
t.Run("success_does_not_retry", func(t *testing.T) {
calls := 0
_, err := loadSystemSettingsWithRetry(func() (*SystemSettings, error) {
calls++
return nil, nil
})
require.NoError(t, err)
assert.Equal(t, 1, calls)
})
}