From 61df0814ea25cf4a63e5633aef4157e728b6e330 Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Sun, 26 Jul 2026 11:19:03 +0100 Subject: [PATCH] Push polling cadence settings into live monitors on save Saving backupPollingInterval, pmgPollingInterval or backupPollingEnabled only mutated the API handler's base config. Monitors poll against a detached DeepCopy of that config, and only pvePollingInterval set the reload flag, so the saved values never reached shouldRunBackupPoll or the PMG scheduler until an unrelated monitor reload happened. Add mutex-guarded runtime overrides on Monitor with live setters, fan them out to every tenant monitor from the settings handler (mirroring forEachNotificationManager), and clear the per-instance last-poll timestamps when lowering the backup interval or re-enabling backup polling so the next cycle runs an immediate catch-up poll. Fixes #1619 Co-Authored-By: Claude Fable 5 --- internal/api/system_settings.go | 48 +++++ internal/api/system_settings_handlers_test.go | 80 ++++++++ internal/monitoring/monitor.go | 180 +++++++++--------- internal/monitoring/monitor_backups.go | 2 +- internal/monitoring/monitor_pbs_pmg.go | 2 +- .../monitoring/monitor_runtime_settings.go | 126 ++++++++++++ .../monitor_runtime_settings_test.go | 112 +++++++++++ internal/monitoring/poll_providers.go | 9 +- 8 files changed, 464 insertions(+), 95 deletions(-) create mode 100644 internal/monitoring/monitor_runtime_settings.go create mode 100644 internal/monitoring/monitor_runtime_settings_test.go diff --git a/internal/api/system_settings.go b/internal/api/system_settings.go index ed4f29791..4daf68e9d 100644 --- a/internal/api/system_settings.go +++ b/internal/api/system_settings.go @@ -33,6 +33,9 @@ type SystemSettingsMonitor interface { EnableTemperatureMonitoring() DisableTemperatureMonitoring() GetNotificationManager() *notifications.NotificationManager + SetBackupPollingEnabled(enabled bool) + SetBackupPollingInterval(interval time.Duration) + SetPMGPollingInterval(interval time.Duration) } // SystemSettingsHandler handles system settings @@ -161,6 +164,30 @@ func (h *SystemSettingsHandler) forEachNotificationManager(r *http.Request, fn f } } +// forEachTenantMonitor applies fn to every live tenant monitor when +// multi-tenant iteration is available, falling back to the request's monitor +// otherwise. Instance-wide polling cadence settings must reach every org's +// monitor: each one polls against a detached copy of the base config, so +// mutating h.config alone never affects a running monitor. +func (h *SystemSettingsHandler) forEachTenantMonitor(r *http.Request, fn func(SystemSettingsMonitor)) { + h.stateMu.RLock() + mtMonitor := h.mtMonitor + h.stateMu.RUnlock() + + type monitorRanger interface { + ForEachMonitor(func(*monitoring.Monitor)) + } + if ranger, ok := mtMonitor.(monitorRanger); ok && ranger != nil { + ranger.ForEachMonitor(func(m *monitoring.Monitor) { + fn(m) + }) + return + } + if monitor := h.getMonitor(r.Context()); monitor != nil { + fn(monitor) + } +} + func (h *SystemSettingsHandler) getMonitor(ctx context.Context) SystemSettingsMonitor { h.stateMu.RLock() mtMonitor := h.mtMonitor @@ -1086,6 +1113,27 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter }) log.Info().Str("publicURL", settings.PublicURL).Msg("Updated notification public URL from settings") } + // Polling cadence settings are read every cycle by each tenant monitor + // from its own detached config copy, so push them into the live monitors + // directly; only pvePollingInterval triggers a full monitor reload. + if _, ok := rawRequest["backupPollingEnabled"]; ok && settings.BackupPollingEnabled != nil { + enabled := *settings.BackupPollingEnabled + h.forEachTenantMonitor(r, func(m SystemSettingsMonitor) { + m.SetBackupPollingEnabled(enabled) + }) + } + if _, ok := rawRequest["backupPollingInterval"]; ok { + interval := h.config.BackupPollingInterval + h.forEachTenantMonitor(r, func(m SystemSettingsMonitor) { + m.SetBackupPollingInterval(interval) + }) + } + if _, ok := rawRequest["pmgPollingInterval"]; ok && settings.PMGPollingInterval > 0 { + interval := time.Duration(settings.PMGPollingInterval) * time.Second + h.forEachTenantMonitor(r, func(m SystemSettingsMonitor) { + m.SetPMGPollingInterval(interval) + }) + } // Reload cached system settings after successful save if h.reloadSystemSettingsFunc != nil { diff --git a/internal/api/system_settings_handlers_test.go b/internal/api/system_settings_handlers_test.go index ef957584f..db9be5df6 100644 --- a/internal/api/system_settings_handlers_test.go +++ b/internal/api/system_settings_handlers_test.go @@ -22,6 +22,9 @@ import ( // MockMonitor implementation type mockMonitor struct { + backupPollingEnabledCalls []bool + backupPollingIntervalCalls []time.Duration + pmgPollingIntervalCalls []time.Duration } func (m *mockMonitor) GetDiscoveryService() *discovery.Service { return nil } @@ -31,6 +34,15 @@ func (m *mockMonitor) StopDiscoveryService() func (m *mockMonitor) EnableTemperatureMonitoring() {} func (m *mockMonitor) DisableTemperatureMonitoring() {} func (m *mockMonitor) GetNotificationManager() *notifications.NotificationManager { return nil } +func (m *mockMonitor) SetBackupPollingEnabled(enabled bool) { + m.backupPollingEnabledCalls = append(m.backupPollingEnabledCalls, enabled) +} +func (m *mockMonitor) SetBackupPollingInterval(interval time.Duration) { + m.backupPollingIntervalCalls = append(m.backupPollingIntervalCalls, interval) +} +func (m *mockMonitor) SetPMGPollingInterval(interval time.Duration) { + m.pmgPollingIntervalCalls = append(m.pmgPollingIntervalCalls, interval) +} type mockTenantMonitorProvider struct { orgID string @@ -263,6 +275,74 @@ func TestHandleUpdateSystemSettings_Basic(t *testing.T) { } } +// Regression test for #1619: backupPollingInterval, pmgPollingInterval and +// backupPollingEnabled must be pushed into live monitors on save. Mutating +// h.config alone never reaches a running monitor (each polls a detached +// config copy), and these settings do not trigger a monitor reload. +func TestHandleUpdateSystemSettings_PollingCadencePushedToLiveMonitors(t *testing.T) { + tempDir := t.TempDir() + cfg := &config.Config{ + DataPath: tempDir, + ConfigPath: tempDir, + EnableBackupPolling: true, + BackupPollingInterval: time.Hour, + PMGPollingInterval: time.Minute, + } + persistence := config.NewConfigPersistence(tempDir) + monitor := &mockMonitor{} + reloadCalled := false + handler := newTestSystemSettingsHandler(cfg, persistence, monitor, func() {}, func() error { + reloadCalled = true + return nil + }) + + tokenVal := "testtoken123" + cfg.APITokens = []config.APITokenRecord{ + {ID: "token1", Hash: internalauth.HashAPIToken(tokenVal), Name: "Test Token"}, + } + + updates := map[string]interface{}{ + "backupPollingInterval": 600, + "pmgPollingInterval": 120, + "backupPollingEnabled": false, + } + body, _ := json.Marshal(updates) + + req := httptest.NewRequest(http.MethodPost, "/api/system-settings", bytes.NewReader(body)) + req.Header.Set("X-API-Token", tokenVal) + rec := httptest.NewRecorder() + + handler.HandleUpdateSystemSettings(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d, body: %s", rec.Code, rec.Body.String()) + } + + if len(monitor.backupPollingIntervalCalls) != 1 || monitor.backupPollingIntervalCalls[0] != 600*time.Second { + t.Errorf("expected SetBackupPollingInterval(600s) on live monitor, got %v", monitor.backupPollingIntervalCalls) + } + if len(monitor.pmgPollingIntervalCalls) != 1 || monitor.pmgPollingIntervalCalls[0] != 120*time.Second { + t.Errorf("expected SetPMGPollingInterval(120s) on live monitor, got %v", monitor.pmgPollingIntervalCalls) + } + if len(monitor.backupPollingEnabledCalls) != 1 || monitor.backupPollingEnabledCalls[0] != false { + t.Errorf("expected SetBackupPollingEnabled(false) on live monitor, got %v", monitor.backupPollingEnabledCalls) + } + if reloadCalled { + t.Error("polling cadence settings should not trigger a full monitor reload") + } + + // The base config still updates so future tenant monitors inherit the values. + if cfg.BackupPollingInterval != 600*time.Second { + t.Errorf("expected base config BackupPollingInterval 600s, got %v", cfg.BackupPollingInterval) + } + if cfg.PMGPollingInterval != 120*time.Second { + t.Errorf("expected base config PMGPollingInterval 120s, got %v", cfg.PMGPollingInterval) + } + if cfg.EnableBackupPolling { + t.Error("expected base config EnableBackupPolling false") + } +} + func TestHandleUpdateSystemSettings_DisablesAutoUpdatesWhenRCSelected(t *testing.T) { tempDir := t.TempDir() cfg := &config.Config{ diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index bf95d88e3..698fc7be5 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -1053,91 +1053,95 @@ func (h InstanceHealth) NormalizeCollections() InstanceHealth { // Monitor handles all monitoring operations type Monitor struct { - config *config.Config - state *models.State - orgID string // Organization ID for tenant isolation (empty = default/legacy) - pveClients map[string]PVEClientInterface - pbsClients map[string]*pbs.Client - pmgClients map[string]*pmg.Client - availabilityStatuses map[string]AvailabilityProbeStatus - pollProviders map[InstanceType]PollProvider - pollMetrics *PollMetrics - scheduler *AdaptiveScheduler - stalenessTracker *StalenessTracker - taskQueue *TaskQueue - pollTimeout time.Duration - circuitBreakers map[string]*circuitBreaker - deadLetterQueue *TaskQueue - failureCounts map[string]int - lastOutcome map[string]taskOutcome - backoffCfg backoffConfig - rng *rand.Rand - maxRetryAttempts int - tempCollector *TemperatureCollector // SSH-based temperature collector - guestMetadataStore *config.GuestMetadataStore - dockerMetadataStore *config.DockerMetadataStore - hostMetadataStore *config.HostMetadataStore - hostContinuityStore *config.HostContinuityStore - hostAgentLifecycleMu sync.RWMutex - mu sync.RWMutex - startTime time.Time - rateTracker *RateTracker - metricsHistory *MetricsHistory - metricsStore *metrics.Store // Persistent SQLite metrics storage - alertManager *alerts.Manager - alertResolvedAICallback func(*alerts.Alert) - alertTriggeredAICallback func(*alerts.Alert) - connectionsSnapshotLister func() []alerts.ConnectionSnapshot // returns platform connection snapshots for the connection-degraded check - incidentStore *memory.IncidentStore - notificationMgr *notifications.NotificationManager - configPersist *config.ConfigPersistence - discoveryService *discovery.Service // Background discovery service - activePollCount int32 // Number of active polling operations - pollCounter int64 // Counter for polling cycles - authFailures map[string]int // Track consecutive auth failures per node - lastAuthAttempt map[string]time.Time // Track last auth attempt time - lastClusterCheck map[string]time.Time // Track last cluster check for standalone nodes - lastPhysicalDiskPoll map[string]time.Time // Track last physical disk poll time per instance - lastPVEBackupPoll map[string]time.Time // Track last PVE backup poll per instance - lastPBSBackupPoll map[string]time.Time // Track last PBS backup poll per instance - pveBackupInventoryReady map[string]map[string]bool // Track PVE guest inventory readiness for backup orphan detection - pveBackupTemplateSubjects map[string]map[string]struct{} // Track template VMIDs excluded from runtime workloads but valid for backups - backupPermissionWarnings map[string]string // Track backup permission issues per instance (instance -> warning message) - persistence *config.ConfigPersistence // Add persistence for saving updated configs - pbsBackupPollers map[string]bool // Track PBS backup polling goroutines per instance - pbsBackupCacheTime map[string]map[pbsBackupGroupKey]time.Time // Track when each PBS backup group was last fetched - runtimeCtx context.Context // Context used while monitor is running - wsHub *websocket.Hub // Hub used for broadcasting state - diagMu sync.RWMutex // Protects diagnostic snapshot maps - nodeSnapshots map[string]NodeMemorySnapshot - guestSnapshots map[string]GuestMemorySnapshot - rrdCacheMu sync.RWMutex // Protects short-lived guest memory caches. - nodeRRDMemCache map[string]rrdMemCacheEntry - vmRRDMemCache map[string]rrdMemCacheEntry - vmAgentMemCache map[string]agentMemCacheEntry - removedDockerHosts map[string]time.Time // Track deliberately removed Docker hosts (ID -> removal time) - dockerTokenBindings map[string]string // Track token ID -> Docker host identity bindings to enforce uniqueness - dockerIdentityFlaps map[string]*dockerIdentityFlapTracker // Track per-host identity flapping (cloned VMs sharing machine-id) - removedKubernetesClusters map[string]time.Time // Track deliberately removed Kubernetes clusters (ID -> removal time) - kubernetesTokenBindings map[string]string // Track token ID -> agent ID bindings to enforce uniqueness - removedHostAgents map[string]time.Time // Track deliberately removed host agents (ID -> removal time) - hostTokenBindings map[string]string // Track tokenID:hostname -> host identity bindings - hostReportApplyLocksMu sync.Mutex - hostReportApplyLocks map[string]*hostReportApplyLock - hostReportOrderMu sync.Mutex - hostReportOrders map[string]hostReportOrder - dockerCommands map[string]*dockerHostCommand - dockerCommandIndex map[string]string - guestMetadataMu sync.RWMutex - guestMetadataCache map[string]guestMetadataCacheEntry - guestMetadataLimiterMu sync.Mutex - guestMetadataLimiter map[string]time.Time - guestMetadataSlots chan struct{} - guestMetadataMinRefresh time.Duration - guestMetadataRefreshJitter time.Duration - guestMetadataRetryBackoff time.Duration - guestMetadataHoldDuration time.Duration - guestAgentWorkSlots chan struct{} + config *config.Config + state *models.State + orgID string // Organization ID for tenant isolation (empty = default/legacy) + pveClients map[string]PVEClientInterface + pbsClients map[string]*pbs.Client + pmgClients map[string]*pmg.Client + availabilityStatuses map[string]AvailabilityProbeStatus + pollProviders map[InstanceType]PollProvider + pollMetrics *PollMetrics + scheduler *AdaptiveScheduler + stalenessTracker *StalenessTracker + taskQueue *TaskQueue + pollTimeout time.Duration + circuitBreakers map[string]*circuitBreaker + deadLetterQueue *TaskQueue + failureCounts map[string]int + lastOutcome map[string]taskOutcome + backoffCfg backoffConfig + rng *rand.Rand + maxRetryAttempts int + tempCollector *TemperatureCollector // SSH-based temperature collector + guestMetadataStore *config.GuestMetadataStore + dockerMetadataStore *config.DockerMetadataStore + hostMetadataStore *config.HostMetadataStore + hostContinuityStore *config.HostContinuityStore + hostAgentLifecycleMu sync.RWMutex + mu sync.RWMutex + startTime time.Time + rateTracker *RateTracker + metricsHistory *MetricsHistory + metricsStore *metrics.Store // Persistent SQLite metrics storage + alertManager *alerts.Manager + alertResolvedAICallback func(*alerts.Alert) + alertTriggeredAICallback func(*alerts.Alert) + connectionsSnapshotLister func() []alerts.ConnectionSnapshot // returns platform connection snapshots for the connection-degraded check + incidentStore *memory.IncidentStore + notificationMgr *notifications.NotificationManager + configPersist *config.ConfigPersistence + discoveryService *discovery.Service // Background discovery service + activePollCount int32 // Number of active polling operations + pollCounter int64 // Counter for polling cycles + authFailures map[string]int // Track consecutive auth failures per node + lastAuthAttempt map[string]time.Time // Track last auth attempt time + lastClusterCheck map[string]time.Time // Track last cluster check for standalone nodes + lastPhysicalDiskPoll map[string]time.Time // Track last physical disk poll time per instance + lastPVEBackupPoll map[string]time.Time // Track last PVE backup poll per instance + lastPBSBackupPoll map[string]time.Time // Track last PBS backup poll per instance + pveBackupInventoryReady map[string]map[string]bool // Track PVE guest inventory readiness for backup orphan detection + pveBackupTemplateSubjects map[string]map[string]struct{} // Track template VMIDs excluded from runtime workloads but valid for backups + backupPermissionWarnings map[string]string // Track backup permission issues per instance (instance -> warning message) + persistence *config.ConfigPersistence // Add persistence for saving updated configs + pbsBackupPollers map[string]bool // Track PBS backup polling goroutines per instance + pbsBackupCacheTime map[string]map[pbsBackupGroupKey]time.Time // Track when each PBS backup group was last fetched + runtimePollingMu sync.RWMutex // Guards the polling-cadence overrides below; polling goroutines read them every cycle + backupPollingEnabledOverride *bool // Runtime override for config.EnableBackupPolling (nil = use config) + backupPollingIntervalOverride *time.Duration // Runtime override for config.BackupPollingInterval (nil = use config) + pmgPollingIntervalOverride *time.Duration // Runtime override for config.PMGPollingInterval (nil = use config) + runtimeCtx context.Context // Context used while monitor is running + wsHub *websocket.Hub // Hub used for broadcasting state + diagMu sync.RWMutex // Protects diagnostic snapshot maps + nodeSnapshots map[string]NodeMemorySnapshot + guestSnapshots map[string]GuestMemorySnapshot + rrdCacheMu sync.RWMutex // Protects short-lived guest memory caches. + nodeRRDMemCache map[string]rrdMemCacheEntry + vmRRDMemCache map[string]rrdMemCacheEntry + vmAgentMemCache map[string]agentMemCacheEntry + removedDockerHosts map[string]time.Time // Track deliberately removed Docker hosts (ID -> removal time) + dockerTokenBindings map[string]string // Track token ID -> Docker host identity bindings to enforce uniqueness + dockerIdentityFlaps map[string]*dockerIdentityFlapTracker // Track per-host identity flapping (cloned VMs sharing machine-id) + removedKubernetesClusters map[string]time.Time // Track deliberately removed Kubernetes clusters (ID -> removal time) + kubernetesTokenBindings map[string]string // Track token ID -> agent ID bindings to enforce uniqueness + removedHostAgents map[string]time.Time // Track deliberately removed host agents (ID -> removal time) + hostTokenBindings map[string]string // Track tokenID:hostname -> host identity bindings + hostReportApplyLocksMu sync.Mutex + hostReportApplyLocks map[string]*hostReportApplyLock + hostReportOrderMu sync.Mutex + hostReportOrders map[string]hostReportOrder + dockerCommands map[string]*dockerHostCommand + dockerCommandIndex map[string]string + guestMetadataMu sync.RWMutex + guestMetadataCache map[string]guestMetadataCacheEntry + guestMetadataLimiterMu sync.Mutex + guestMetadataLimiter map[string]time.Time + guestMetadataSlots chan struct{} + guestMetadataMinRefresh time.Duration + guestMetadataRefreshJitter time.Duration + guestMetadataRetryBackoff time.Duration + guestMetadataHoldDuration time.Duration + guestAgentWorkSlots chan struct{} // Configurable guest agent timeouts (refs #592) guestAgentFSInfoTimeout time.Duration guestAgentNetworkTimeout time.Duration @@ -1236,11 +1240,11 @@ func (m *Monitor) shouldRunBackupPoll(last time.Time, now time.Time) (bool, stri return false, "configuration unavailable", last } - if !m.config.EnableBackupPolling { + if !m.backupPollingEnabledSetting() { return false, "backup polling globally disabled", last } - interval := m.config.BackupPollingInterval + interval := m.backupPollingIntervalSetting() if interval > 0 { if !last.IsZero() && now.Sub(last) < interval { next := last.Add(interval) @@ -1954,7 +1958,7 @@ func (m *Monitor) baseIntervalForInstanceType(instanceType InstanceType) time.Du case InstanceTypePBS: return clampInterval(m.config.PBSPollingInterval, 10*time.Second, time.Hour) case InstanceTypePMG: - return clampInterval(m.config.PMGPollingInterval, 10*time.Second, time.Hour) + return clampInterval(m.pmgPollingIntervalSetting(), 10*time.Second, time.Hour) default: base := m.config.AdaptivePollingBaseInterval if base <= 0 { diff --git a/internal/monitoring/monitor_backups.go b/internal/monitoring/monitor_backups.go index 476770e55..21da106c0 100644 --- a/internal/monitoring/monitor_backups.go +++ b/internal/monitoring/monitor_backups.go @@ -2031,7 +2031,7 @@ func (m *Monitor) pollPVEBackupsAsync( return nil } - if !m.config.EnableBackupPolling { + if !m.backupPollingEnabledSetting() { log.Debug(). Str("instance", instanceName). Msg("Skipping backup polling - globally disabled") diff --git a/internal/monitoring/monitor_pbs_pmg.go b/internal/monitoring/monitor_pbs_pmg.go index c1a74a7c8..60479ae18 100644 --- a/internal/monitoring/monitor_pbs_pmg.go +++ b/internal/monitoring/monitor_pbs_pmg.go @@ -498,7 +498,7 @@ func (m *Monitor) pollPBSInstance(ctx context.Context, instanceName string, clie log.Debug(). Str("instance", instanceName). Msg("No PBS datastores available for backup polling") - } else if !m.config.EnableBackupPolling { + } else if !m.backupPollingEnabledSetting() { log.Debug(). Str("instance", instanceName). Msg("Skipping PBS backup polling - globally disabled") diff --git a/internal/monitoring/monitor_runtime_settings.go b/internal/monitoring/monitor_runtime_settings.go new file mode 100644 index 000000000..ec12c2e8c --- /dev/null +++ b/internal/monitoring/monitor_runtime_settings.go @@ -0,0 +1,126 @@ +package monitoring + +import "time" + +// Runtime-tunable polling cadence settings. +// +// Each tenant monitor polls against a detached DeepCopy of the base config, +// so mutating the base config after a system-settings save never reaches a +// running monitor (#1619). These setters push the saved values into live +// monitors directly, shadowing the config fields. The overrides live behind +// runtimePollingMu because polling goroutines read them every cycle while +// the settings API writes them. + +// backupPollingEnabledSetting returns the effective EnableBackupPolling value. +func (m *Monitor) backupPollingEnabledSetting() bool { + if m == nil || m.config == nil { + return false + } + m.runtimePollingMu.RLock() + override := m.backupPollingEnabledOverride + m.runtimePollingMu.RUnlock() + if override != nil { + return *override + } + return m.config.EnableBackupPolling +} + +// backupPollingIntervalSetting returns the effective BackupPollingInterval. +// Zero means cycle-based scheduling (BackupPollingCycles). +func (m *Monitor) backupPollingIntervalSetting() time.Duration { + if m == nil || m.config == nil { + return 0 + } + m.runtimePollingMu.RLock() + override := m.backupPollingIntervalOverride + m.runtimePollingMu.RUnlock() + if override != nil { + return *override + } + return m.config.BackupPollingInterval +} + +// pmgPollingIntervalSetting returns the effective PMGPollingInterval before +// clamping. +func (m *Monitor) pmgPollingIntervalSetting() time.Duration { + if m == nil || m.config == nil { + return 0 + } + m.runtimePollingMu.RLock() + override := m.pmgPollingIntervalOverride + m.runtimePollingMu.RUnlock() + if override != nil { + return *override + } + return m.config.PMGPollingInterval +} + +// SetBackupPollingEnabled toggles backup polling on the live monitor. +// Re-enabling clears the per-instance last-poll timestamps so the next +// polling cycle runs an immediate catch-up poll, matching the behavior of +// a full monitor reload. +func (m *Monitor) SetBackupPollingEnabled(enabled bool) { + if m == nil { + return + } + m.runtimePollingMu.Lock() + wasEnabled := m.config != nil && m.config.EnableBackupPolling + if m.backupPollingEnabledOverride != nil { + wasEnabled = *m.backupPollingEnabledOverride + } + m.backupPollingEnabledOverride = &enabled + m.runtimePollingMu.Unlock() + + if enabled && !wasEnabled { + m.resetBackupPollTimestamps() + } +} + +// SetBackupPollingInterval updates the backup polling cadence on the live +// monitor. Lowering the interval (or switching from cycle-based scheduling +// to an interval) clears the per-instance last-poll timestamps so the next +// polling cycle runs an immediate catch-up poll rather than waiting out the +// remainder of the old interval. +func (m *Monitor) SetBackupPollingInterval(interval time.Duration) { + if m == nil { + return + } + if interval < 0 { + interval = 0 + } + m.runtimePollingMu.Lock() + previous := time.Duration(0) + if m.config != nil { + previous = m.config.BackupPollingInterval + } + if m.backupPollingIntervalOverride != nil { + previous = *m.backupPollingIntervalOverride + } + m.backupPollingIntervalOverride = &interval + m.runtimePollingMu.Unlock() + + if interval > 0 && (previous <= 0 || interval < previous) { + m.resetBackupPollTimestamps() + } +} + +// SetPMGPollingInterval updates the PMG polling cadence on the live monitor. +// The scheduler re-reads the base interval every cycle, so no further action +// is needed. +func (m *Monitor) SetPMGPollingInterval(interval time.Duration) { + if m == nil || interval <= 0 { + return + } + m.runtimePollingMu.Lock() + m.pmgPollingIntervalOverride = &interval + m.runtimePollingMu.Unlock() +} + +// resetBackupPollTimestamps clears the per-instance backup poll timestamps +// so the next cycle polls immediately. +func (m *Monitor) resetBackupPollTimestamps() { + m.mu.Lock() + m.lastPVEBackupPoll = make(map[string]time.Time) + m.lastPBSBackupPoll = make(map[string]time.Time) + m.mu.Unlock() +} diff --git a/internal/monitoring/monitor_runtime_settings_test.go b/internal/monitoring/monitor_runtime_settings_test.go new file mode 100644 index 000000000..7aaafb7ee --- /dev/null +++ b/internal/monitoring/monitor_runtime_settings_test.go @@ -0,0 +1,112 @@ +package monitoring + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +// Regression test for #1619: monitors poll against a detached DeepCopy of the +// base config, so mutating the base config after a settings save must not be +// relied on — the live setters have to reach the running monitor directly. +func TestSetBackupPollingIntervalReachesLiveMonitor(t *testing.T) { + now := time.Now() + base := &config.Config{ + EnableBackupPolling: true, + BackupPollingInterval: time.Hour, + } + m := &Monitor{ + config: base.DeepCopy(), + lastPVEBackupPoll: map[string]time.Time{"pve1": now.Add(-30 * time.Minute)}, + lastPBSBackupPoll: map[string]time.Time{"pbs1": now.Add(-30 * time.Minute)}, + } + + last := m.lastPVEBackupPoll["pve1"] + if should, _, _ := m.shouldRunBackupPoll(last, now); should { + t.Fatal("expected no poll 30m into a 1h interval") + } + + // Mutating the base config (what the settings handler used to do) never + // reaches the monitor's detached copy. + base.BackupPollingInterval = 10 * time.Minute + if should, _, _ := m.shouldRunBackupPoll(last, now); should { + t.Fatal("base config mutation unexpectedly reached the detached monitor config") + } + + // The live setter must take effect without a monitor reload. + m.SetBackupPollingInterval(10 * time.Minute) + if len(m.lastPVEBackupPoll) != 0 || len(m.lastPBSBackupPoll) != 0 { + t.Fatal("lowering the interval should clear last-poll timestamps for an immediate catch-up poll") + } + last = m.lastPVEBackupPoll["pve1"] // zero after reset + if should, _, _ := m.shouldRunBackupPoll(last, now); !should { + t.Fatal("expected immediate catch-up poll after lowering the interval") + } +} + +func TestSetBackupPollingIntervalRaisingKeepsTimestamps(t *testing.T) { + now := time.Now() + last := now.Add(-5 * time.Minute) + m := &Monitor{ + config: &config.Config{ + EnableBackupPolling: true, + BackupPollingInterval: 10 * time.Minute, + }, + lastPVEBackupPoll: map[string]time.Time{"pve1": last}, + lastPBSBackupPoll: map[string]time.Time{}, + } + + m.SetBackupPollingInterval(time.Hour) + if got := m.lastPVEBackupPoll["pve1"]; !got.Equal(last) { + t.Fatal("raising the interval should keep last-poll timestamps") + } + if should, _, _ := m.shouldRunBackupPoll(last, now); should { + t.Fatal("expected no poll 5m into the raised 1h interval") + } +} + +func TestSetBackupPollingEnabledReachesLiveMonitor(t *testing.T) { + now := time.Now() + m := &Monitor{ + config: &config.Config{ + EnableBackupPolling: true, + BackupPollingInterval: time.Hour, + }, + lastPVEBackupPoll: map[string]time.Time{"pve1": now.Add(-30 * time.Minute)}, + lastPBSBackupPoll: map[string]time.Time{}, + } + + m.SetBackupPollingEnabled(false) + if should, reason, _ := m.shouldRunBackupPoll(time.Time{}, now); should || reason != "backup polling globally disabled" { + t.Fatalf("expected backup polling disabled, got should=%v reason=%q", should, reason) + } + + m.SetBackupPollingEnabled(true) + if len(m.lastPVEBackupPoll) != 0 { + t.Fatal("re-enabling backup polling should clear last-poll timestamps") + } + if should, _, _ := m.shouldRunBackupPoll(m.lastPVEBackupPoll["pve1"], now); !should { + t.Fatal("expected immediate poll after re-enabling backup polling") + } +} + +func TestSetPMGPollingIntervalReachesLiveMonitor(t *testing.T) { + base := &config.Config{PMGPollingInterval: 2 * time.Minute} + m := &Monitor{config: base.DeepCopy()} + + if got := m.baseIntervalForInstanceType(InstanceTypePMG); got != 2*time.Minute { + t.Fatalf("expected 2m from config, got %v", got) + } + + m.SetPMGPollingInterval(5 * time.Minute) + if got := m.baseIntervalForInstanceType(InstanceTypePMG); got != 5*time.Minute { + t.Fatalf("expected 5m after live update, got %v", got) + } + + // Clamping still applies to live updates. + m.SetPMGPollingInterval(2 * time.Hour) + if got := m.baseIntervalForInstanceType(InstanceTypePMG); got != time.Hour { + t.Fatalf("expected live update clamped to 1h, got %v", got) + } +} diff --git a/internal/monitoring/poll_providers.go b/internal/monitoring/poll_providers.go index 97eb74e9d..16809d6de 100644 --- a/internal/monitoring/poll_providers.go +++ b/internal/monitoring/poll_providers.go @@ -6,7 +6,6 @@ import ( "strings" "time" - "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" "github.com/rcourtman/pulse-go-rewrite/pkg/pmg" @@ -348,7 +347,7 @@ type prefixedPollProviderSpec[C comparable] struct { prefix string configInstances func(*Monitor) []pollProviderInstanceConfig clients func(*Monitor) map[string]C - pollingInterval func(*config.Config) time.Duration + pollingInterval func(*Monitor) time.Duration buildPollTask func(*Monitor, string) (PollTask, error) } @@ -375,7 +374,7 @@ func newPrefixedPollProvider[C comparable](spec prefixedPollProviderSpec[C]) Pol if m == nil || m.config == nil { return 0 } - return clampInterval(spec.pollingInterval(m.config), 10*time.Second, time.Hour) + return clampInterval(spec.pollingInterval(m), 10*time.Second, time.Hour) }, buildPollTask: spec.buildPollTask, } @@ -387,7 +386,7 @@ func newPBSPollProvider() PollProvider { prefix: "pbs-", configInstances: pbsInstanceConfigs, clients: pbsClientMap, - pollingInterval: func(cfg *config.Config) time.Duration { return cfg.PBSPollingInterval }, + pollingInterval: func(m *Monitor) time.Duration { return m.config.PBSPollingInterval }, buildPollTask: func(m *Monitor, instanceName string) (PollTask, error) { if m == nil { return PollTask{}, fmt.Errorf("monitor is nil") @@ -411,7 +410,7 @@ func newPMGPollProvider() PollProvider { prefix: "pmg-", configInstances: pmgInstanceConfigs, clients: pmgClientMap, - pollingInterval: func(cfg *config.Config) time.Duration { return cfg.PMGPollingInterval }, + pollingInterval: func(m *Monitor) time.Duration { return m.pmgPollingIntervalSetting() }, buildPollTask: func(m *Monitor, instanceName string) (PollTask, error) { if m == nil { return PollTask{}, fmt.Errorf("monitor is nil")