diff --git a/internal/config/config.go b/internal/config/config.go index 784e82b21..1c91a01eb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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") diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 5dc3b9f49..33ccf9f31 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -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) + }) +}